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

New Approach for Popup #455

Closed
mfreed7 opened this issue Feb 8, 2022 · 7 comments
Closed

New Approach for Popup #455

mfreed7 opened this issue Feb 8, 2022 · 7 comments
Labels
popover The Popover API

Comments

@mfreed7
Copy link
Collaborator

mfreed7 commented Feb 8, 2022

There has been extensive debate (e.g. #410 and #417, plus numerous OpenUI meeting discussions) about the <popup> element proposal. That proposal got "stuck" on what appear to be some rather fundamental questions about the semantics and associated AX roles for this new element.

As a result of these issues, we've put together a fresh proposal for a new way to achieve (roughly) the same list of goals. This new proposal introduces an HTML attribute syntax that allows any element to be made into a "popup" that gets rendered on top of all other page content:

<div popup=popup>I'm rendered on top!</div>

Here, the popup attribute is set to a value of popup (other values include hint and async) to indicate "popup" behaviors should be applied. That includes:

  • Rendering in the top layer
  • Automatically-provided light-dismiss and one-at-a-time behavior. (These depend on the value of the popup attribute.)
  • Accessible by default

Additionally, another HTML attribute, triggerpopup, can be used for declarative triggering, so that this API can be used without any JavaScript required:

<button id=picker-button triggerpopup=datepicker>Pick a date</button> 
<my-date-picker role=dialog id=datepicker popup=popup hidden>
  ...date picker implementation...
</my-date-picker>

In this case, the <my-date-picker> component starts out hidden, but when the <button> is clicked, the UA removes hidden from <my-date-picker> and the date picker is shown. As with all popups, once the user clicks outside, hits Esc, etc., the UA will light-dismiss it by adding back the hidden attribute.

Please take a look at the new proposal, and add your comments here to this issue. Pull requests for changes are also great.

@chrisdholt
Copy link
Collaborator

Only note I would have is "accessible by default" seems like something that is likely within reason. A point of clarity here would be that an element with a role of dialog with aria-modal="true" and popup="popup" likely isn't going to have focus-trapping behavior by default nor do I think that it should. With that said, focusgroup may provide that for us. Just adding a note to make sure expectations are set.

All that said, I really like this @mfreed7.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Feb 15, 2022

So this new approach was discussed last week at OpenUI. I generally heard supportive comments, and didn't hear any (at least major) objections to the new approach. I'd really like to kick the tires by re-implementing the Chromium <popup> element prototype into a new <div popup=popup> attribute prototype. Before I do that, however, I'd love to hear any input or feedback. I'd hate to implement something that has a fatal flaw. Let me know what you think!

@scottaohara
Copy link
Collaborator

@mfreed7 issues here or with your explainer doc?

from my memory, the topic of hidden vs open seemed good feedback, and making this change would help realign with the aspect of the element's initial spec that was calling out use of open

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Feb 16, 2022

@mfreed7 issues here or with your explainer doc?

Either way works for me! I'll try to listen to both and incorporate them back into the explainer.

from my memory, the topic of hidden vs open seemed good feedback, and making this change would help realign with the aspect of the element's initial spec that was calling out use of open

Yep, this is a great point, thanks for the reminder. I had indeed taken a note to possibly change to an open attribute from the hidden attribute. There were several good comments to that effect in the meeting. Another related one was that by mapping the open attribute to the visibility CSS property, animations could still work in both directions. I'll make this change.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Feb 16, 2022

Yep, this is a great point, thanks for the reminder. I had indeed taken a note to possibly change to an open attribute from the hidden attribute. There were several good comments to that effect in the meeting. Another related one was that by mapping the open attribute to the visibility CSS property, animations could still work in both directions. I'll make this change.

I've updated the explainer to use open instead of hidden. It begs the question we already discussed on #311 about whether we actually need the open attribute at all. In contrast to the decision/discussion related to the <popup> element, I do think we'd need a CSS pseudo class at least, to allow styling things differently depending on whether the element is in the top layer. (That's discussed here.) But it seems like perhaps we could get away with only something like an initiallyopen attribute that sets the initial state, and have the rest happen in the UA. Thoughts?

@css-meeting-bot
Copy link

The Open UI Community Group just discussed New Approach for Popup, and agreed to the following:

  • RESOLVED: The general shape of the `popup` content attribute proposal looks good, and we are ok to start prototyping.
The full IRC log of that discussion <gregwhitworth> Topic: New Approach for Popup
<gregwhitworth> github: https://github.com//issues/455
<hdv> masonf: the main thing I want to get out of this meeting… I presented a new approach to popup that is based on a content attribute as opposed to a new element
<hdv> masonf: the comments I heard were generally positive, so the main thing I want to discuss is… are there any major killer issue that cause it to be not a viable, valid proposal
<hdv> masonf: because the next thing I'd like to do is update the prototype in Chromium
<hdv> masonf: a lot of people have raised great issues, they are in my repo btw, will probably move to open ui repo
<hdv> masonf: would love to hear more comments if people have them
<masonf> Proposed resolution: The general shape of the `popup` content attribute proposal looks good, and we are ok to start prototyping.
<davidluhr> Our work is done here, Dave gave the thumbs up.
<hdv> dave: do it!
<una> LGTM
<hdv> chris: I'd like to second that
<hdv> masonf: we're trying to get to the point where we found most issues there are with it
<hdv> davatron5000: are you happy with it, Mason?
<gregwhitworth> RESOLVED: The general shape of the `popup` content attribute proposal looks good, and we are ok to start prototyping.
<hdv> masonf: yes, I'm happier with it than the element and it feels very implementable to me

@andrico1234
Copy link
Collaborator

@mfreed7 , I'm closing this issue on account of the Telecon resolution. Feel free to re-open if you'd like to continue the discussion here 😄

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

5 participants