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

[popup] defaultopen expectations #609

Closed
scottaohara opened this issue Sep 15, 2022 · 9 comments
Closed

[popup] defaultopen expectations #609

scottaohara opened this issue Sep 15, 2022 · 9 comments
Labels
popover The Popover API

Comments

@scottaohara
Copy link
Collaborator

scottaohara commented Sep 15, 2022

it was previously resolved that defaultopen wouldn't be supported for popup=hint, but was allowed for the other popup types.

I'm concerned that there was a miss on this resolution in regards to popup=auto, as similar to popup=hint, as auto popups will very easily and regularly light dismiss if not very carefully implemented.

see this example forked from Jhey's explainer. I've added a single link to the demo and when tabbing from the browser chrome into the demo, focus goes to the link, thus automatically dismissing the popup. If this popup contained interactive elements, then a keyboard user would not be able to get to it and would need to figure out what element in the page will re-invoke the popup. This content would be completely missed to someone using a screen reader.

I can recall that a use case for popup=auto being open by default was related to UI that i've seen where a search field's auto-suggestion popup was open on page load, and persisted until a user was able to navigate to the search field to interact with the UI. It would auto-dismiss after focus reached / left the search field, or if someone used esc key to dismiss, or mouse clicked / touched outside the popup to light dismiss it.

But, now I'm wondering if this resolution needs to be revised as per my forked example, there needs to be more nuance to how an auto popup can be open by default in reality (e.g., if it was autofocused, then a keyboard user / someone using AT could actually get to the popup when encountering a real web page where there will be other focusable elements besides those within the popup / the popup's trigger). Otherwise, defaultopen may be a feature more practical for popup=manual.

Thoughts?

cc @mfreed7 @jh3y @aleventhal

@scottaohara
Copy link
Collaborator Author

similarly, if an auto popup were opened on page load via a showPopUp(), then it should automatically be focused to mitigate what i outlined in my op

@tbondwilkinson
Copy link

In your example, why not use autofocus on the popup=auto that you are showing with defaultopen? It seems like if you are showing a popup on pageload like this, you would also want it to be focused, otherwise it would dismiss if someone tabs into the page to a link.

But the proposal of basically relegating more complicated use cases to popup="manual" seems reasonable, but I think then you might look at what the downsides of "manual" are, specifically that you then have to implement your auto-dismiss rules. Should manual be enhanced so that someone could opt-in to "auto" type dismiss rules, without actually being "auto"?

Concretely what happens if you rewrite your implementation using "manual", do you lose out on functionality you otherwise get with "auto"?

@scottaohara
Copy link
Collaborator Author

@tbondwilkinson I did mentioned that one resolution could be to ensure the defaultopen popup automatically receives focus. Requiring autofocus on a defaultopen popup is definitely a solution worth considering. But, if that's the solution then people will forget to do that. This is why I forked the example, to demonstrate how this could be a gap.

Concretely what happens if you rewrite your implementation using "manual", do you lose out on functionality you otherwise get with "auto"?

I would have implemented this example as a modal dialog due to its presentation and the above issues mentioned.

@tbondwilkinson
Copy link

The likelihood of people forgetting to add autofocus on a defaultopen popup is the same as people forgetting to add autofocus on popup in general, isn't it? I haven't verified, but the explainer I think says that no focus behavior is performed on show except when autofocus is used.

@scottaohara
Copy link
Collaborator Author

but the explainer I think says that no focus behavior is performed on show except when autofocus is used.

right. that's why i raised this issue as i think this may be a gap in what we wrote per this use case. again, autofocus may be the solution, maybe another.

@tbondwilkinson
Copy link

I think the proposal of having autofocus be the default for popup=auto (which would apply both for defaultopen and for other ways of opening a popup) is reasonable, I think autofocusing the root (when no other element is requested to be autofocused) is correct. It does complicated the behavior of autofocus though since this needs to differentiate between "I want to autofocus my root" and the desired behavior "I want to autofocus my root, in the absence of any contained element attributed with autofocus". Perhaps autofocus=auto (which would be the default) could clearly indicate the latter?

And this would I assume imply that people wanting to opt out would have autofocus=none?

@scottaohara
Copy link
Collaborator Author

scottaohara commented Sep 15, 2022

yeh, i hesitate to think that autofocus needs to happen 100% of the time (outside of if - as you mentioned - autofocus could be more of a true/false declaration), depending on how the auto popup was opened. but defaultopen is a clear signal that "i want people to look at this now" - so it is definitely worth considering in that instance.

@mfreed7 mfreed7 added the popover The Popover API label Oct 5, 2022
@mfreed7
Copy link
Collaborator

mfreed7 commented Oct 5, 2022

This issue is quite related to #415, and in particular, #415 (comment).

This entire issue would be moot if blur() were not a light dismiss trigger, correct? The keyboard user would be able to tab around at will, without "fear" of closing the open pop-up. If they discovered the pop-up, and wanted to close it, the Esc key will always do that. And as a side-effect, this would also solve #415 (comment).

We're going to re-discuss #415 soon, so perhaps this issue should wait until that resolution.

@mfreed7
Copy link
Collaborator

mfreed7 commented Oct 20, 2022

Since we just resolved to remove blur as a light dismiss trigger for pop-ups, at least by default, that should solve this issue. I'll close it, but feel free to re-open if I missed something.

@mfreed7 mfreed7 closed this as completed Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
popover The Popover API
Projects
None yet
Development

No branches or pull requests

3 participants