[Modal] Scroll changes when focus from browser URL bar

[mctep]

• This is not a v0.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

When any modal is open the window scroll position should not never change.

Current Behavior 😯

When any modal is open we could change scroll by focus through browser address bar.

Steps to Reproduce 🕹

Link: https://jp6q69qqkv.codesandbox.io/

1. Scroll to the bottom to hide the "Focusable Button"
2. Open Dialog through the "Super Secret Button"
3. Focus to the browser address bar
4. Press Tab

Context 🔦

When we focus to window (through Tab press in address bar) Chrome tries to find the first (or the last if we press Shift+Tab) focusable element on the page and focus it. If this element is not in viewport Chrome scrolls the page to this element.

Example source code: https://codesandbox.io/s/8n9wj3vo60

According to: #13702 (comment)

Your Environment 🌎

Tech	Version
Material-UI	v4.0.0-alpha.8
React
Browser	Chrome
TypeScript
etc.

[oliviertassinari]

@mctep Have you found any implementation that handles this case correctly? I can't so far:

• KO: https://www.w3.org/TR/wai-aria-practices/examples/dialog-modal/dialog.html
• KO: https://getbootstrap.com/docs/4.3/components/modal/
• KO: https://ant.design/components/modal/
• KO: https://github.com/reactjs/react-modal (https://codesandbox.io/s/6z9n46j34k)

[mctep]

No. Looks like all projects are affected by the issue.
I have made this trick for my company UI library.

[mctep]

Hm... If just tabidex="0" is added to body element the issue is gone for w3.org sample and Material UI.
Focus event listener also could be added to body in TrapFocus I think.

[oliviertassinari]

@theKashey has proposed a simple logic to change the behavior in #15450 (comment).
@mctep Do you have any resource on why it should behave this way?

[mctep]

@oliviertassinari Do you mean fixed hidden sentinels in the body? I have no resources (and have not found any), it is just an idea. @theKashey proposed the right thing. I think it is the simplest workaround to prevent page from scrolling. At least I do not see the easier solution.

[oliviertassinari]

I was wondering if the page scrolling to the top wasn't a good thing. It's what happens when no modal is open. But, once you close the Modal, the focus goes back to the previously activeElement, the page is gonna scroll a second time. I think that we should avoid it, changing the behavior is probably a good idea. You have suggested the usage of a tab index attribute on the body element. However, it will probably harm the Skip-nav link. I would rather avoid any global side effect like this.

[mctep]

I meant to add tabIndex to the body while any modal is open.
Looks like the skip nav link should not be accessible while any modal is open. So we should remove tabIndex from body on modal closing.

However it was a bad idea because by this way we could not detect focus direction. When we press Shift + Tab from the URL bar we should focus to the last element in the Modal. It is possible only when we have two sentinels in the body element as the first child and as the last child.

[mctep]

In the other hand detecting the last focusable element in the opened modal is non-trivial task. So case with Shift + Tab could be ignored. So I do not see big difference between body + tabindex and fixed hidden sentinel ways.