Dialog contents focusable when dialog is hidden

[GuiRitter]

Hi everyone.

With the dialog visible or hidden, it's possible to press the Tab key until the dialog contents have focus, enabling them to be interacted with.

I believe this should be prevented, since it enables the user to interact with systems in ways the programmers might not be expecting, leading into, in a best case scenario, bad user experience and, in a worst case scenario, actions that violate security, privacy, etc.

Here's a CodeSandBox that reproduces the issue. To reproduce it, press the Tab key until the open modal button is selected. This might require that the text be focused or selected beforehand. Then press the Tab key once to select the button inside the modal. Then press Enter or Space Bar to execute the action that should be unavailable.

Thanks in advance.

[marcio]

Hi @GuiRitter

Thanks for report the issue, at the end, skylight works with display: block | none, then, the modal dom is always in you context, visible or hidden.

I'm not sure that prevent all fields/buttons inside a modal is a feature, maybe the devs can avoid this "problem" (when happen), just putting a tabindex="-1" in your fields/buttons when is hidden.

But I'll think about this, maybe with an optional prop, I don't now.

I'll keep the issue open to analyze.

:metal:

[NicoleRauch]

Today I was wondering about something similar: Why don't you simply render nothing when the dialog is invisible, instead of rendering it with display: none?

What I mean is something like:

render() {
     return dialogIsNotVisible ? null : 
     <ContentsOfReactSkylightAsBefore>...</ContentsOfReactSkylightAsBefore>;
}

That would also reduce the amount of generated HTML in cases where many dialogs are rendered but only one is visible at a time.

[GuiRitter]

Today I was wondering about something similar: Why don't you simply render nothing when the dialog is invisible, instead of rendering it with display: none?

What I mean is something like:

render() {
     return dialogIsNotVisible ? null : 
     <ContentsOfReactSkylightAsBefore>...</ContentsOfReactSkylightAsBefore>;
}

That would also reduce the amount of generated HTML in cases where many dialogs are rendered but only one is visible at a time.

Sure, that works, but it seems to me like this is something that skylight should implement, not us, even if only as courtesy,

[NicoleRauch]

Oh @GuiRitter , I'm super-sorry! This was badly worded and I did not intend to address you but instead React-Skylight... Let me try again, please.

[NicoleRauch]

Today I was wondering about something similar: Why doesn't react-skylight simply render nothing when the dialog is invisible, instead of rendering it with display: none?

What I mean is something like:

render() {
     return dialogIsNotVisible ? null : 
     <ContentsOfReactSkylightAsBefore>...</ContentsOfReactSkylightAsBefore>;
}

That would also reduce the amount of generated HTML in cases where many dialogs are rendered but only one is visible at a time. What do you think @marcio ?

[GuiRitter]

@NicoleRauch Don't worry, I didn't took it negatively nor nothing like that.

[OZZlE]

It's not great for Screen Readers such as VoiceOver, if you have a website in the US then by law it needs to work for blind people as well.

Currently elements inside the popup are focusable when it's closed, Voice Over on Mac reads the whole popup when I tested.

Another issue is that when the popup is opened then Voice Over can focus on elements outside the popup as well, so it's not clear to a blind person that they have opened a popup and it doesn't read it first. I made workarounds in my own wrapper component for popups but it would be great if this module could handle it instead :)

Also since the close button is a charecter it reads "button middle cross" which doesn't make sense, it would probably be better if it was an image with a hidden text (read by SR) or a link with title text. Where the text is "Close dialog" or so, should be translatable as well :)