Prevent scrolling up when opening a dialog

[mmtr]

Changes proposed in this Pull Request

Add a ReactModal__Html--open class to the HTML document that unsets the default overflow-y: scroll; CSS declaration for solving a conflict between that declaration and the ReactModal__Body--open class which is defining a overflow: hidden; for the body.

When we scroll in Calypso, we're actually scrolling the body element, so when we disable the scrolling with overflow: hidden;, its ancestor, the HTML document, becomes scrollable which by default is scrolled to the top since we have never scrolled that element. Note that although the HTML document is scrollable now, we cannot scroll down when the Dialog is open because the body height is limited to the 100% of the window height.

With the ReactModal__Html--open we avoid this behavior since the HTML content is now visible and not scrollable. This indeed has side effect: the scrollbar is lost so there is a reflow caused by the new window size available. But I don't think is a big deal, since it's almost inappreciable due to the fact that most of the UI changes because of the dialog.

Testing instructions

• Create a new post with the Classic editor.
• Add enough content to scroll down.
• Insert an image at the end of the post.
• Check that you remain in the same scroll position you were before opening the Media Library.
• Add a link towards the end of the post (by highlighting some text and clicking on the hyperlink button).
• Make sure you're still in the same scroll position.
• Switch to Gutenberg.
• Insert and image using the Media Library.
• Check again that the page is not scrolled.

Fixes #16873.
Fixes #27226.
Fixes #29821.

[matticbot]

Test live: https://calypso.live/?branch=fix/modal-scroll-top

[Copons]

overflow-y: scroll forces displaying the scrollbar even if the content isn’t long enough to scroll.

The purpose is to prevent the reflow caused by the content obtaining or losing the scrollbar, which happens, for example, when the content length changes, or when you play around with modals.

I can confirm that the change in this PR fixes the modal issue, but I honestly don't know how to effectively test for regressions. 🤔

[mmtr]

Ah, you're right @Copons!

Didn't have that in consideration since I'm using the "auto" scrollbars in macOS, so they appear floating over the content and overflow: scroll doesn't affect the size available for the content.

But indeed that reflow is there if I change the setting to always display the scrollbar (and I guess that this is happening always in Windows).

I'd say that was the single purpose of overflow-y: scroll and it's causing a regression.

I guess we could solve it by doing what Bootstrap does: apply a padding when the modal is opened, so the size available doesn't change. Although this will make the code more complex and I honestly don't like that very much.

The alternative is to do what @jameskoster suggested on #16873, remove the overflow: hidden style defined by ReactModal__Body--open, but that will make scrollable the content behind the modal.

[Copons]

We could also keep track of the scroll position before opening the modal, and make sure to scroll back to it after closing it. 🤔

[gwwar]

I think it's probably much less risky for us to adopt a more targeted fix. I don't have any preferences for the approach but will note that JS fixes tend to a bit more finicky and prone to breakage.

[mmtr]

I included back the original overflow-y: scroll for the HTML document in d4e248e and created a new ReactModal__Html--open which unsets that CSS and is only applied when opening a dialog, so the change should be less risky now.

This indeed causes the effect described by @Copons: the scrollbar is gone when the modal is open so there is reflow caused by the new size available in the window, but I don't think this is a problem since the change is almost invisible due to the fact that most of the UI changes because the Dialog is covering most of the previous content.

[gwwar]

Thanks @mmtr this tests well for me! I think after we verify that we're not using those component refs this is good to 🚢

[jameskoster]

Did y'all see this? #28737

Seemed like an elegant fix to me.

[mmtr]

Sorry, I wasn't aware about that PR.

Although it's the same approach I took at the beginning (a317984) so, as @Copons noted (#29832 (comment)), that didn't prevent the reflow caused by the content obtaining or losing the scrollbar.

Which is why I decided to implement a more targeted fix, in order to lose the scrollbar only when the dialog is open.