Skip to content
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

Closed

Conversation

dandclark
Copy link
Collaborator

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":

Screenshot 2020-12-20 115008

Not sure if that's the organization we want; I'm happy to change it to something else.

Resolves #241.

…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
Copy link
Member

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.

Copy link
Collaborator Author

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.

@gregwhitworth
Copy link
Member

@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.
@dandclark
Copy link
Collaborator Author

@gregwhitworth Now it's my turn to apologize for the delay. 😊
I added a clarification above and changed the PR to pull this strange exception out of the light dismiss definition, replacing it with a note on a best practice that will mitigate the issue.

@yoavweiss yoavweiss changed the base branch from master to main March 5, 2021 21:45
@dandclark
Copy link
Collaborator Author

dandclark commented Mar 18, 2022

Hi @mfreed7 , @andrico1234 reminded me of the existence of this PR about light dismiss from a while back.
I think after #490 merges, the popup explainer should be where the definition of light dismiss lives.

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.

@mfreed7
Copy link
Collaborator

mfreed7 commented Mar 18, 2022

Hi @mfreed7 , @andrico1234 reminded me of the existence of this PR about light dismiss from a while back. I think after #490 merges, the popup explainer should be where the definition of light dismiss lives.

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!

@gregwhitworth
Copy link
Member

I think this is stale at this point, closing this out - we can always re-open later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Light dismiss on window blur?
3 participants