-
Notifications
You must be signed in to change notification settings - Fork 199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move light dismiss description to standalone page. #243
Move light dismiss description to standalone page. #243
Conversation
…nition a bit. Add some demo gifs. Add window blur as a light dismiss trigger.
the focus target is outside of the contents of the component. This includes | ||
the case where the user invokes a non-focusable element, which causes focus to | ||
switch to the `<body>`. | ||
- There is one exception to this: if a user invokes a non-focusable element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This intrigues me. The focus moves to the <body>
? Do you have an example as this seems odd to move the focus to the body and not dismiss the popup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It moves either to the closest focusable ancestor of the clicked element, or to the <body>
if there is no focusable ancestor. You can try this out with the rightmost <select>
here: https://codepen.io/daniec/pen/poyPOKY. Note that the focusout
event causes the popup to collapse when the nonfocusable text inside it is clicked, because that click causes the focus to jump to the <body>
.
This is a bit odd but it's just how focus works in the web platform.
That said, I talked about this with some Microsoft folks and we thought that it's probably better to pull the exception for this behavior outside of the core of the light dismiss definition, and instead include a note recommending that developers make the top level of the component with light dismiss focusable (e.g. with tabindex="-1"
). If this is done, then focus won't move outside of the light dismiss component and it won't be hidden.
I've made that change to this PR.
@dandclark sorry for the delayed review. I approved it but I'd like clarification on the focus moving to the body while not dismissing the popup as that seems rather odd to me from a UX perspective. |
…e of the definition and add recommendation that root of the light dismiss component should be focusable, to avoid this issue.
@gregwhitworth Now it's my turn to apologize for the delay. 😊 |
Hi @mfreed7 , @andrico1234 reminded me of the existence of this PR about light dismiss from a while back. What I've written here is a bit more thorough than what we've got in the Popup Explainer so far. Do you think any of this is worth moving into Popup after your PR lands? Also of note is that we apparently resolved in 2020 that window blur should trigger light dismiss, contrary to the way the discussion was going yesterday. #415 seems to be a partial dupe of #241, which I'd entirely forgotten about. |
Thanks for bringing this up! I do think popup is where the light dismiss description should live, since it's pretty core to what a popup is. I also agree that this description is more thorough, and should be folded into what's already in popup. Let's get #490 landed and then do that. Wow, I had forgotten about that meeting discussing window blur. But after reading the minutes, I remember it. One of the suggestions was to do some UXR and see what the "right" behavior is. The resolution from that meeting was essentially "Windows does this, so that's what we should keep doing". Perhaps it's worth re-opening the discussion. In any case, I have a to-do for today to post the summary of yesterday's meeting, and I'll be sure to include this link - thanks! |
I think this is stale at this point, closing this out - we can always re-open later. |
Move the light dismiss description from a section in the
<select>
research page to a standalone page. Expand the definition a bit and add some demo gifs. Add window blur as a light dismiss trigger per telecon resolution.The new page is added under a new section called "Behaviors":
Not sure if that's the organization we want; I'm happy to change it to something else.
Resolves #241.