Skip to content

fix: remove double scroll bar in studio edit modal#53

Merged
navinkarkera merged 1 commit into
opencraft/ulmofrom
navin/FAL-4368/fix-double-scroll-bar
Jul 15, 2026
Merged

fix: remove double scroll bar in studio edit modal#53
navinkarkera merged 1 commit into
opencraft/ulmofrom
navin/FAL-4368/fix-double-scroll-bar

Conversation

@navinkarkera

@navinkarkera navinkarkera commented Jul 7, 2026

Copy link
Copy Markdown
Member

Description

Bug description:

Currently, when editing the AI block in the studio, the editing area has its own scroll bar independent of the one containing the block's edit view. This can create a scenario where someone scrolls down with the scroll wheel and hits the apparent bottom even though there are additional settings available.

The addition of .ai-eval-studio-scroll class to inner content caused double scroll when modal already has scrolling support.

But the modal for some reason doesn't scroll in fullscreen mode, so I had to add the class conditionally based on modal- fullscreen class.

However, we cannot use css directly due to shadow-dom which doesn't detect parent dom's classes, so we check the class using javascript and toggle a data attribute using mutation observer and apply the class depending on the data attribute.

Note: This could be an upstream issue which I did not test yet.

Should be fixed in openedx/openedx-platform#38859

Supporting information

Testing instructions

Verify the bug is fixed. Test in both normal and fullscreen mode.

Other information

Include anything else that will help reviewers and consumers understand the change.

  • Does this change depend on other changes elsewhere?
  • Any special concerns or limitations? For example: deprecations, migrations, security, or accessibility.

@navinkarkera navinkarkera requested a review from Kelketek July 7, 2026 14:40
@Kelketek

Kelketek commented Jul 7, 2026

Copy link
Copy Markdown
Member

@navinkarkera I think your assessment that this is an upstream bug may be correct. It seems especially weird that we need to worry about it like this, and hack around to solve it in this manner.

The scroll is now implemented in upstream and we don't it here. Depends
on openedx/openedx-platform#38859 to handle
fullscreen scrolling
@navinkarkera navinkarkera force-pushed the navin/FAL-4368/fix-double-scroll-bar branch from 355bcf5 to 75144b8 Compare July 8, 2026 07:36
@Kelketek

Kelketek commented Jul 8, 2026

Copy link
Copy Markdown
Member

@navinkarkera The client is currently on Ulmo. Since the bug is on Ulmo, does this fix need to be deferred until they upgrade to whatever is after Verawood, lest it break things for them today?

@Kelketek

Kelketek commented Jul 8, 2026

Copy link
Copy Markdown
Member

Actually, just realized we could put it in our fork, and solve it that way. Duh.

@Kelketek

Kelketek commented Jul 9, 2026

Copy link
Copy Markdown
Member

@navinkarkera It does look like the second scroll wheel is gone. Thank you!

I did notice, however, that the footer has a bunch of extra space. Here's what it looks like for me:

image

By comparison, here's the HTML block:

image

Could you take a look at this?

@navinkarkera

Copy link
Copy Markdown
Member Author

@Kelketek The html block opens up native react modal and doesn't need iframe for the editor to work. But in this case, the modal has its own default actions bar but the xblock implements its own action bar inside modal body. I'll investigate why it is so and if we can remove the empty space occupied by modal-actions.

@navinkarkera

Copy link
Copy Markdown
Member Author

@Kelketek I just pushed a change in edx-platform which fixes the empty space while still showing modal-actions if required.

@Kelketek

Copy link
Copy Markdown
Member

@navinkarkera This is working for me now, thanks! Please don't merge it yet, though-- I'll do so before coordinating with EduNext on their next deployment so it goes smoothly with the transition to our branch (they're currently using upstream) and then we can get the backports to there.

@Kelketek

Copy link
Copy Markdown
Member

@navinkarkera This should be safe to merge after all since we don't need to backport to Ulmo, right?

@navinkarkera

Copy link
Copy Markdown
Member Author

@Kelketek Yes. We don't need to backport edx-platform PR.

@navinkarkera navinkarkera merged commit 86e10d4 into opencraft/ulmo Jul 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants