Disable ability to scroll background while in modal

[zolrath]

Problem

Currently while the modal is open a user can still scroll the primary page in the background.
This is usually unexpected and also leads to two scrollbars being rendered by Chrome.

Solution

In order to prevent the user from scrolling the background, one common practice is to set the body element to overflow: hidden while the modal is open.

One issue with this method is that it causes the sidebar to disappear, triggering a layout shift of the pages content.

Per CanIUse, all browsers (aside from Safari which uses floating scrollbars which do not take up space) have implemented support for scrollbar-gutter so we can use scrollbar-gutter: stable to prevent the shift.

Issues

scrollbar-gutter works perfectly well in all browsers tested, aside from a known issue in Chrome which prevents the scrollbar for the modal from rendering. The temporary hidden gutter is drawn above any fixed position element, causing the modals scrollbar to be hidden.

The scrollbar-gutter portion could be removed, keeping the layout shift from the sidebar disappearing but resolving the background scrolling/double scrollbar issue.

Alternatives

The HTML5 Dialog Element allows modals to be shown with a showModal command. Per CanIUse this has support in all browsers aside from IE11. It performs proper focus trapping, backdrop colors, prevents scrolling of backgrounds etc.

[rktjmp]

Just asking, is there a reason to not toggle the scrollbar-gutter in the JS event vs always in the template? Probably kind of race-y with the layout change?

[zolrath]

In my testing setting scrollbar-gutter in the JS event causes a flash of multiple scrollbars when closing the modal.

Having scrollbar-gutter: stable set also prevents things like opening/closing accordions or dynamically added content that causes the scrollbar to appear/disappear from creating a layout shift as well, so it's quite useful even outside of the context of the modal.

[shamanime]

I found this issue in our production app and implemented this exact same code to add and remove the overflow to the body, the issue is there's some circumstances where the modal may close without firing the close event (a redirect or patch that removes it from the HTML for instance) and the body stays locked.

To prevent this I created a hook that ensures the overflow is removed on the destroyed() event.

[zolrath]

Hmm, I suppose we have two directions we could go with this then.

1. Add a phx-removed callback on the modal. Would you expect this to be a function that only removes the overflow class from the body? Currently when a modal is closed it performs the @on_cancel callback passed to it, which in autogenerated code is a live_patch, but if the modal is getting removed due to an external patch it seems like it would not trigger its normal callback.
2. Go more minimal with this and add overscroll-none to the role="dialog" div. This keeps the second scrollbar, but the browser won't automatically scroll the background page when the user hits the bottom of the modal.

[cw789]

With the alternative of using the HTML dialog tag I personally currently just use:

body:has(dialog[open]) {
  overflow: hidden;
}

But that's not a solution if the Phoenix modal does not have any state tag or class.
And if this tag does not get removed on any patch, whatever.

[zolrath]

I love the has option conceptually as it's not something we need to toggle, though it becomes a convention the user needs to understand since it happens without any obvious cause.

I know that it's available everywhere but Firefox (and IE11) but I've been waiting for that selector for years. Really happy to see it's made so much progress recently!

Even in situations where the selector isn't supported, it would just mean the current behavior of allowing scrolling returns.

[cw789]

Even in situations where the selector isn't supported, it would just mean the current behavior of allowing scrolling returns.

Exactly it's a benefit for 80% of the users at the moment.

[chrismccord]

The modal component should have had phx-remove, so that takes care of the overflow failing to be reset on things like navigation. I went ahead and fixed that and applied @zolrath's changes in my own commit, (but added you as a co-author :) .Thanks!

❤️❤️❤️🐥🔥