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] Consider renaming triggerpopup #508

Closed
mfreed7 opened this issue Mar 31, 2022 · 15 comments
Closed

[popup] Consider renaming triggerpopup #508

mfreed7 opened this issue Mar 31, 2022 · 15 comments
Labels
needs edits This is ready for edits to be made popover The Popover API

Comments

@mfreed7
Copy link
Collaborator

mfreed7 commented Mar 31, 2022

To complete the trifecta of popup bikeshedding issues (along with #491 for popup and #500 for initiallyopen), let's consider renaming triggerpopup.

The purpose of this attribute is to link a button (or similar) element to a popup element, and enable the button to declaratively trigger (show) the popup. The functionality is described here in the explainer.

Suggestions appreciated.

@scottaohara
Copy link
Collaborator

This may be best decided once popup has been shed or shedded… whichever word is best for past tensing that.

but, I personally like the terms trigger or controls which could be appended to whatever other word is chosen. though the latter is due to my familiarity with the aria-controls attribute

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Apr 7, 2022

One more comment here, which directly relates to #486. If that issue is resolved that clicking the trigger button a second time closes the popup, then perhaps a better name for the attribute would be something like togglepopup.

@una
Copy link
Collaborator

una commented Apr 7, 2022

+1 togglepopup seems like a better idea here —

  1. This makes it possible to add this attribute to the "close" button within a modal popup.
  2. It also enables open/close in a trigger button such as a select menu instead of just allowing that button to open it

@css-meeting-bot
Copy link

The Open UI Community Group just discussed 508, and agreed to the following:

  • RESOLVED: Change the name and behavior to 'togglepopup' and keep discussing the values and behaviors for only-open and only-close.
The full IRC log of that discussion <hdv> Topic: 508
<hdv> github: https://github.com//issues/508
<hdv> una: so when you have a menu that you want to close, you may have like an X on it to close it… when masonf and I talked about it we realised togglepopup may be better
<hdv> una: because you usually have the same button to open and close it
<hdv> una: so it could make sense to have multiple attribute, but probably woudl be too many
<Travis> https://wicg.github.io/close-watcher/ as FYI
<masonf> q?
<dandclark> q+
<JonathanNeal> ack JonathanNeal
<hdv> una: majority of reasons seems to be opening or closing, which could be same button or two, but you could use the same attr on both
<hdv> dandclark: I like the idea of a single toggle attribute that can handle both
<hdv> dandclark: are there no use cases where you want to separate the behavior?
<hdv> una: because you set the ID of the popup you can have as many buttons that you could use the attribute on
<hdv> dandclark: what if you want two separate buttons, one is for opening one for closing
<hdv> ack dandclark
<hdv> q+
<JonathanNeal> q+
<hdv> ack hdv
<scotto> q+
<hdv> hdv: thinking of accessible names, would there be cases where you would want to have specific butons for 'open thing' and 'close thing', for that you would need mutiple buttons
<hdv> una: yes but thinking of use cases like tooltips or popovers you would usually use the same button
<JonathanNeal> ack JonathanNeal
<hdv> JonathanNeal: does this open the possibility to @@@
<hdv> masonf: great idea!
<hdv> scotto: there probably wouldn't be an implicit ARIA role or state associated to it, it would behave as a button with a single function, if there was a single element that opened and closed a popup, we would probably need to find another way to set expanded/collapsed states, this could maybe be inferred
<hdv> scotto: probably could decide on something on the platform level that could be baked into the control like the way that works in a selectg
<hdv> s/selectg/select
<JonathanNeal> The "possibility" hdv mentioned was something like `togglepopup="+id"` or `togglepopup="open:id"`.
<masonf> Proposed resolution: Change the name to 'togglepopup' and keep discussing the values.
<hdv> Travis: seems like for 508 we almost have a proposed resolution, for 500, folks can go there and continue the conversation
<JonathanNeal> +1 to that proposed resolution, masonf, and that the default behavior is toggle
<masonf> Proposed resolution: Change the name and behavior to 'togglepopup' and keep discussing the values and behaviors for only-open and only-close.
<hdv> una: I would like to capture which values in the proposed resolution, the default behavior is toggle
<JonathanNeal> +1 to that, masonf
<hdv> +1
<masonf> RESOLVED: Change the name and behavior to 'togglepopup' and keep discussing the values and behaviors for only-open and only-close.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Apr 7, 2022

One proposal to highlight from the meeting just now:

<button togglepopup="id">This toggles the popup</button>
<button togglepopup="+id">This can only open the popup</button>
<button togglepopup="-id">This can only close the popup</button>

There are issues with ids that contain +/-, and the syntax is funny. But perhaps something like this as an approach?

@jonathantneal
Copy link
Contributor

I did initially suggest symbols. A symbol would prevent conflicts with multiple IDs. This is not a direct concern of togglepopup, which is limited to a single ID. Avoiding ID conflicts may still be worth considering, tho, largely as a general practice to be leveraged in future APIs, and lesser as a matter of future-proofing.

However, I would not prefer a pattern using HYPHEN MINUS (-), as that character overlaps with working IDs.

Example:

<style>[-id] { color: blue }</style>
<span -id>this text will be blue</span>

I would recommend a SOLIDUS (/) to separate an ID (or IDs) from additional arguments. In the case of togglepopup, a “force” argument.

Example:

<button togglepopup="id">makes the popup open or close</button>
<button togglepopup="id / open">makes the popup open</button>
<button togglepopup="id / close">makes the popup close</button>

If the separator is acceptable, the second argument can still be bikeshed. In the example above, I set a second argument of open or close. Perhaps it could also be toggle. Perhaps it could be true or false, mirroring DOMTokenList. Perhaps it could be 0 or 1, mirroring the CSS Toggles proposal. I only mention this to separate the open and close keywords from the preference for a SOLIDUS (/) separator.

Applying this pattern to some future concept:

<future-concept check="id">makes the thing checked or unchecked</future-concept>
<future-concept check="id / checked">makes the thing checked</future-concept>
<future-concept check="id / unchecked">makes the thing unchecked</future-concept>
<future-concept check="id / indeterminate">makes the thing indeterminate</future-concept>

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Apr 8, 2022

+1 to using SOLIDUS as a separator, or at the very least not using something that can appear in IDs. I added the +/- to my example just because that's what I heard at the meeting. Thanks for pointing out the issue.

I kind of like your proposed syntax. Is there any equivalent API on the platform, or would this be a brand new pattern? (I think it's new.)

<button togglepopup="id">makes the popup open or close (toggle)</button>
<button togglepopup="id / toggle">makes the popup open or close (toggle)</button>
<button togglepopup="id / open">makes the popup open but not close</button>
<button togglepopup="id / close">makes the popup close but not open</button>

Personally, I don't like true/false or 0/1 because it's confusing. What does togglepopup="id / false" mean? Don't toggle? Only close? I think your original open/close/toggle naming is the most clear and readable.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 14, 2022
Per the [1] resolution, OpenUI has decided to change from
'triggerpopup' to 'togglepopup', with the corresponding behavior
change that when the popup is already open, clicking the invoking
button again will cause the popup to be hidden.

[1] openui/open-ui#508 (comment)

Bug: 1307772
Change-Id: I9a9720a2acb0c952173bed47424ffbbfc8144714
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 14, 2022
Per the [1] resolution, OpenUI has decided to change from
'triggerpopup' to 'togglepopup', with the corresponding behavior
change that when the popup is already open, clicking the invoking
button again will cause the popup to be hidden.

[1] openui/open-ui#508 (comment)

Bug: 1307772
Change-Id: I9a9720a2acb0c952173bed47424ffbbfc8144714
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584778
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#992556}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 14, 2022
Per the [1] resolution, OpenUI has decided to change from
'triggerpopup' to 'togglepopup', with the corresponding behavior
change that when the popup is already open, clicking the invoking
button again will cause the popup to be hidden.

[1] openui/open-ui#508 (comment)

Bug: 1307772
Change-Id: I9a9720a2acb0c952173bed47424ffbbfc8144714
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584778
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#992556}
@mfreed7 mfreed7 added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Apr 18, 2022
@mfreed7
Copy link
Collaborator Author

mfreed7 commented Apr 21, 2022

In working on the Chromium prototype, I think I found an issue with the concept of allowing multiple buttons to open/close a popup. In this example:

<div popup=popup id=popup>Popup</div>
<button togglepopup=popup>Click me to *open* the popup</button>
<button togglepopup=popup>Click me to *close* the popup</button>

Clicking the "open" button properly opens the popup, and in doing so, registers the first button as the "invoking element" for the popup. Subsequent clicks on that same button are therefore ignored in the light dismiss logic, meaning a mouse-down on the first button doesn't immediately close the popup. However, if instead, the user clicks on the second button intending to close the popup, that mouse-down is a light dismiss trigger, since it isn't a descendent of a) the popup, b) the anchor element, or c) the invoking element. So that mouse-down closes the popup. Then the togglepopup behavior happens, which sees the now-hidden popup and "toggles" it back open. So the close button just causes the popup to flicker but stay open.

I suppose this could be fixed by adding more magic to the light dismiss algorithm, to check whether the clicked element is a descendent of a button that also points (via togglepopup) to the popup element, and if so, avoid light dismissing the popup. That seems like it'll work, but I wanted to bring this up in case there are better suggestions.

One bonus, I think, is that if we go this route, we don't actually have to store the invoking element on Element. We can just check for togglepopup on ancestors as we walk up the tree, and if they point to an open popup, count them as ancestral popups.

@css-meeting-bot
Copy link

The Open UI Community Group just discussed Consider renaming triggerpopup.

The full IRC log of that discussion <gregwhitworth> Topic: Consider renaming triggerpopup
<gregwhitworth> github: https://github.com//issues/508
<gregwhitworth> q?
<flackr> masonf: Discussed before, we resolved to rename triggerpopup to togglepopup. One issue still open - since it would have an implied toggle behavior open -> close -> open, you may want a way to have a one way toggle behavior to have two buttons
<flackr> masonf: There's a proposed syntax, togglepopup=<id>/(open|close)
<hdv> q+
<una> q+
<gregwhitworth> ack hdv
<flackr> hdv: Is there any precedent for this?
<flackr> masonf: Not aware of one
<gregwhitworth> ack una
<JonathanNeal> In CSS, there is a similar precedent.
<gregwhitworth> q+
<flackr> una: I'm in support of having it be togglepopup by default. Not sure about the proposed syntax where it's part of the string. I think it would make more sense if we had togglepopup=id and had openpopup and closepopup as separate values
<flackr> una: I think this is more clear from the developer perspective
<jh3y> openpop="id" closepopup="id" more declarative for people. String has some room for error. People writing "closer", etc.
<hdv> ack greg
<jh3y> +1
<JonathanNeal> The precedence of a second argument being a "force" is in classList / DOMTokenList.
<flackr> gregwhitworth: +1 to una's suggestion, we just need to figure out how to handle the collision if you put multiple on there
<flackr> gregwhitworth: we also need to look to the implementers for the overhead cost of this
<jh3y> Yeah, what takes precedence?
<gregwhitworth> q?
<gregwhitworth> ack dbaron
<Zakim> dbaron, you wanted to comment on collision handling
<flackr> una: it's hopefully relatively similar to other things, just syntactic sugar
<una> q+
<flackr> dbaron: for collision handling, we can take the union of the behaviors, i.e. openpopup + closepopup is equivalent to togglepopup. openpopup + togglepopup is equivalent to toggle
<gregwhitworth> ack una
<jh3y> I'd see open/close as taking precedence over toggle.
<flackr> una: I think this would be a bit confusing, and only one should win
<flackr> masonf: It's only confusing if you also have togglepopup, but if you only have openpopup or closepopup it makes sense
<JonathanNeal> what if I put different ids to open & close?
<gregwhitworth> yeah, we're discussing an error case
<jh3y> If you have open and toggle, it should be open IMO.
<gregwhitworth> flackr: it's like onmousedown and onmouseup onmouseclick
<JonathanNeal> openpopup="one_id" closepopup="alt_id"
<flackr> masonf: if we remove toggle and just have open and close it's pretty clear right?
<JonathanNeal> q+
<jh3y> Oof – the button that controls multiple popups. That would flag an a11y thing perhaps?
<flackr> una: I don't think it's confusing, but it's a better experience to just write toggle
<gregwhitworth> q?
<flackr> masonf: what if you write all three?
<flackr> una: I'm coming around to it should toggle, what if there are more ids?
<flackr> masonf: that's less ambiguous, specific behavior per id
<flackr> una: could you have opentoggle=id1 opentoggle=id2, etc
<hdv> or opentoggle="id1 id2 id3" like with class?
<flackr> masonf: I think you get the value of the last one provided?
<JonathanNeal> They are only supposed to support one, I asked about this.
<flackr> emilio: making it depend on order feels bad
<JonathanNeal> support one id
<flackr> emilio: we do have precedents with longhand and shorthand ordering in css, but it's not great
<gregwhitworth> q?
<JonathanNeal> my issue with multiple attributes are that the id must be duplicated, which means they can be different.
<flackr> emilio: if we want to have all three attributes, we should just define the order
<flackr> una: this feels like checked=false checked=true, last one wins
<flackr> emilio: this is a parser thing though, the html parser chooses one to win
<JonathanNeal> still q+
<flackr> emilio: having two different attributes is a perfectly syntactically valid situation, so it seems simplest to define the order in cases where the order could matter
<miriam> q+
<flackr> emilio: does the order matter? if both or all three are present, we just need to define the behavior to not do two different things
<masonf> q?
<flackr> una: if open and close are both present, does it open it after closing?
<flackr> emilio: right, this is what we should define
<JonathanNeal> Sorry, I lack the assertiveness to just jump into the conversation on Zoom. :)
<flackr> dbaron: I agree, you should only get one state change out of the same thing
<gregwhitworth> ack JonathanNeal
<flackr> JonathanNeal: I share these concerns. I had asked previously - there should only ever be one item an element with popup can control. This was the question that lead to toggleattribute being the one attribute which is why I ultimately prefer this syntax. If we duplicate the ids then we must resolve the possible difference of different ids
<flackr> JonathanNeal: If there was a single attribute syntax it removes this possible issue
<flackr> masonf: So the question is should it be possible to have one button open more than one popup?
<flackr> JonathanNeal: Oh, I thought it's not supposed to be possible. If it was, then a single attr is easier, but if not, then this isn't a concern
<gregwhitworth> ack miriam
<flackr> miriam: if we had 3 attrs, open|close|toggle, it makes sense with all three or any combination we get the toggle behavior because any comination adds up to this
<flackr> miriam: this gets more complicated with different ids, where it really shouldn't control different ids
<flackr> masonf: if you have togglepopup=id1 and openpopup=id2, which one opens?
<flackr> masonf: i realize we can define this, but it feels odd to have to reason about this
<gregwhitworth> that's a great point
<JonathanNeal> This is why I am hoping we can land on something like `toggle="id"`, `toggle="id open"`, `toggle="id close"`.
<flackr> masonf: this is why the single attribute suggestion is nice - in avoiding this problem
<masonf> q?
<JonathanNeal> erm, but `togglepopup`:)
<jh3y> Is it not the case that we might want multiple popups in some scenarios?
<una> q?
<JonathanNeal> q?
<JonathanNeal> q+
<masonf> q+
<flackr> gregwhitworth: based on this, I can't stand the syntax that was proposed earlier, but i see the need to force choosing a single id
<una> q+
<gregwhitworth> ack JonathanNeal
<flackr> JonathanNeal: looking at css toggles proposal, it seems that a toggle and its control are not separated by a /, which makes me think the / might separate for controlling multiple items. I see a precedent there for not needing any symbol. Does this seem better? e.g. togglepopup="id (open|close)" this acts as the one way forcing direction
<gregwhitworth> ack masonf
<flackr> masonf: one issue there is ids can contain whitespace
<hdv> white space separated would make me thing there is an element with the id 'open' that would also open
<hdv> *think
<flackr> JonathanNeal: can ids support whitespace?
<gregwhitworth> q++ masonf
<JonathanNeal> We do have force on `toggle` in https://developer.mozilla.org/en-US/docs/Web/API/DOMTokenList/toggle
<flackr> una: with this syntax, the keywords open and close could be ids. It's weird that we're setting the attribute to toggle but the open and close don't mean toggle. It feels unusual and less normative. I still prefer multiple attributes
<gregwhitworth> ack +
<Futekov> +1 for having several attributes
<flackr> masonf: what if we allow all three, but define it such that one always takes precedence
<dbaron> +1 to just defining the precedence
<gregwhitworth> ack una
<gregwhitworth> ack masonf
<hdv> +1 to defining precedence, that way it is only complex for authors who make it complex by adding multiple attributes
<JonathanNeal> if we define precedence, I would expect `toggle` wins.
<flackr> una: I think masonf and i in agreement we like multiple attributes and we just need to define the precedence
<masonf> Proposed resolution: add two more attributes, 'openpopup' and 'hidepopup' which only open or close the popup. These, plus 'togglepopup' would only take an idref as a value. We would define a priority such that only one of the three takes effect.
<una> LGTM
<hdv> +1
<JonathanNeal> q+
<gregwhitworth> ack JonathanNeal
<flackr> JonathanNeal: I like this mostly. Can we punt on the last part about defining a priority, does adding one invalidate it?
<masonf> Proposed resolution: add two more attributes, 'openpopup' and 'hidepopup' which only open or close the popup. These, plus 'togglepopup' would only take an idref as a value. We would define how precedence works when more than one of these attributes is used.
<gregwhitworth> flackr: the ordering is just for the IDRef, not the behavior
<JonathanNeal> +1
<dbaron> (in hindsight, I realize I was a bit confused earlier and making suggestions that weren't helpful)
<Futekov> q+
<jh3y> ( open || close ) > toggle, open && close == do nothing
<gregwhitworth> ack Futekov
<flackr> una: we could still define it being the order things appear in the dom. The resolution is just to add these attributes and figure out how to resolve these conflicts
<JonathanNeal> thank you for the rephrasing, masonf.
<emilio> q+
<masonf> q+
<flackr> Futekov: we don't necessarily need precedence. We can just apply the same logic as when the parser sees duplicate attributes in one tag.
<gregwhitworth> ack emilio
<flackr> emilio: it's a pretty different case from having multiple classes in the class attribute or multiple class attributes?
<flackr> Futekov: multiple class attributes
<flackr> emilio: You only need to deal with that during parsing but not anywhere else, where having different attributes is a valid state that can happen from javascript. This makes it complicated to define last when it can occur later.
<flackr> emilio: I'd prefer not relying on attribute order
<gregwhitworth> ack masonf
<flackr> masonf: +1, this makes me nervous. We do keep track of the order, but it would be difficult to track over updates and could be complicated
<JonathanNeal> This is why I am glad we have rephrased the proposal.
<flackr> gregwhitworth: I think we're safe to resolve on modified proposed resolution, the further discussion is just about how to handle the discrepancies
<una> +1 thanks JonathanNeal
<flackr> Resolved: add two more attributes, 'openpopup' and 'hidepopup' which only open or close the popup. These, plus 'togglepopup' would only take an idref as a value. We would define how precedence works when more than one of these attributes is used.

@gregwhitworth gregwhitworth added needs edits This is ready for edits to be made and removed agenda+ Use this label if you'd like the topic to be added to the meeting agenda labels Apr 21, 2022
@mfreed7
Copy link
Collaborator Author

mfreed7 commented Apr 21, 2022

Note the resolution from the meeting just now:

<flackr> Resolved: add two more attributes, 'openpopup' and 'hidepopup' which only
open or close the popup. These, plus 'togglepopup' would only take an idref as a
value. We would define how precedence works when more than one of these
attributes is used.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Apr 21, 2022

Just to clarify what I assume to be everyone's preference: the resolution above should have said showpopup and not openpopup. If there are objections to that, please open a new issue to discuss, and sorry about that!

I've opened #523 to discuss the rules for precedence. Given that, I'm going to close this issue as resolved.

@mfreed7 mfreed7 closed this as completed Apr 21, 2022
@mfreed7
Copy link
Collaborator Author

mfreed7 commented Apr 21, 2022

I suppose this could be fixed by adding more magic to the light dismiss algorithm, to check whether the clicked element is a descendent of a button that also points (via togglepopup) to the popup element, and if so, avoid light dismissing the popup. That seems like it'll work, but I wanted to bring this up in case there are better suggestions.

One bonus, I think, is that if we go this route, we don't actually have to store the invoking element on Element. We can just check for togglepopup on ancestors as we walk up the tree, and if they point to an open popup, count them as ancestral popups.

I'm treating this issue like an implementation/spec issue at this point. I think it should be possible to get the behavior we want, and as I implement it in Chromium, I'll raise an issue if the above approach doesn't seem to be working.

@jh3y
Copy link
Collaborator

jh3y commented Apr 21, 2022

Thoughts from the call:

  • Is there not a use case whereby a developer could want multiple popups on the top level?
  • I like the idea of "toggle", "show", and "hide". For precedence, "show" or "hide" trump "toggle" IMO as they are more specific about what they are doing. If all three exist, it's a toggle. If "show" and "hide" exist, it's a non-issue too? Only issue is with varying IDs 🤔 If a button opens one and closes another, where does a11y come into that? Does it?
  • +1 for ::backdrop It's one less thing for developers to worry about creating. If they want to create custom, they can.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Apr 22, 2022

Thanks for the comments!

  • Is there not a use case whereby a developer could want multiple popups on the top level?

I view that as the primary difference between popups and dialogs. Popups are "one at a time", which allows them to be light-dismissed. If you want more, <dialog> is your friend. If you think there's a use cases we're missing here, please open an issue!

  • I like the idea of "toggle", "show", and "hide". For precedence, "show" or "hide" trump "toggle" IMO as they are more specific about what they are doing. If all three exist, it's a toggle. If "show" and "hide" exist, it's a non-issue too? Only issue is with varying IDs 🤔 If a button opens one and closes another, where does a11y come into that? Does it?

I posted this on the issue for you.

  • +1 for ::backdrop It's one less thing for developers to worry about creating. If they want to create custom, they can.

👍

@jh3y
Copy link
Collaborator

jh3y commented Apr 22, 2022

Awesome! Thanks @mfreed7 🙏

Yeah, after discussing it some more, I think you could do a popover as an overlay to tackle that scenario of wanting more than one popover displayed at a time. The idea was like "Help" screens/tutorials where you have multiple call outs. I've yet to dig into dialog demos in-depth. Can you display multiple at the same time side by side then? I know you can nest them which is interesting.

Thanks for forwarding on my comments 🙏

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 22, 2022
Prior to this change, there were several corner cases that would not
be properly handled, such as an anchor element that contains the
invoker of a lower-level popup. In addition, with this change, *any*
element that has an invoking attribute* that points to a popup, even
if it was not *used* to open that popup, will still act as an ancestral
popup relationship, and will not cause that popup to be light dismissed
if clicked.

See:
  openui/open-ui#508

* `togglepopup` for now, soon to also be `showpopup` and `hidepopup`

Bug: 1307772
Change-Id: I443ffd1884abfdb2856dc5d946323de4b83e20e4
aarongable pushed a commit to chromium/chromium that referenced this issue Apr 26, 2022
Prior to this change, there were several corner cases that would not
be properly handled, such as an anchor element that contains the
invoker of a lower-level popup. In addition, with this change, *any*
element that has an invoking attribute* that points to a popup, even
if it was not *used* to open that popup, will still act as an ancestral
popup relationship, and will not cause that popup to be light dismissed
if clicked.

See:
  openui/open-ui#508

* `togglepopup` for now, soon to also be `showpopup` and `hidepopup`

Bug: 1307772
Change-Id: I443ffd1884abfdb2856dc5d946323de4b83e20e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3601261
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#995882}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 26, 2022
Prior to this change, there were several corner cases that would not
be properly handled, such as an anchor element that contains the
invoker of a lower-level popup. In addition, with this change, *any*
element that has an invoking attribute* that points to a popup, even
if it was not *used* to open that popup, will still act as an ancestral
popup relationship, and will not cause that popup to be light dismissed
if clicked.

See:
  openui/open-ui#508

* `togglepopup` for now, soon to also be `showpopup` and `hidepopup`

Bug: 1307772
Change-Id: I443ffd1884abfdb2856dc5d946323de4b83e20e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3601261
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#995882}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 26, 2022
Prior to this change, there were several corner cases that would not
be properly handled, such as an anchor element that contains the
invoker of a lower-level popup. In addition, with this change, *any*
element that has an invoking attribute* that points to a popup, even
if it was not *used* to open that popup, will still act as an ancestral
popup relationship, and will not cause that popup to be light dismissed
if clicked.

See:
  openui/open-ui#508

* `togglepopup` for now, soon to also be `showpopup` and `hidepopup`

Bug: 1307772
Change-Id: I443ffd1884abfdb2856dc5d946323de4b83e20e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3601261
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#995882}
DanielRyanSmith pushed a commit to DanielRyanSmith/wpt that referenced this issue Apr 27, 2022
Per the [1] resolution, OpenUI has decided to change from
'triggerpopup' to 'togglepopup', with the corresponding behavior
change that when the popup is already open, clicking the invoking
button again will cause the popup to be hidden.

[1] openui/open-ui#508 (comment)

Bug: 1307772
Change-Id: I9a9720a2acb0c952173bed47424ffbbfc8144714
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584778
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#992556}
DanielRyanSmith pushed a commit to DanielRyanSmith/wpt that referenced this issue Apr 27, 2022
Prior to this change, there were several corner cases that would not
be properly handled, such as an anchor element that contains the
invoker of a lower-level popup. In addition, with this change, *any*
element that has an invoking attribute* that points to a popup, even
if it was not *used* to open that popup, will still act as an ancestral
popup relationship, and will not cause that popup to be light dismissed
if clicked.

See:
  openui/open-ui#508

* `togglepopup` for now, soon to also be `showpopup` and `hidepopup`

Bug: 1307772
Change-Id: I443ffd1884abfdb2856dc5d946323de4b83e20e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3601261
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#995882}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 16, 2022
…glepopup', a=testonly

Automatic update from web-platform-tests
Convert 'triggerpopup' attribute to 'togglepopup'

Per the [1] resolution, OpenUI has decided to change from
'triggerpopup' to 'togglepopup', with the corresponding behavior
change that when the popup is already open, clicking the invoking
button again will cause the popup to be hidden.

[1] openui/open-ui#508 (comment)

Bug: 1307772
Change-Id: I9a9720a2acb0c952173bed47424ffbbfc8144714
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584778
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#992556}

--

wpt-commits: 27622a853844ef5d16b1e53f83a2db5cb46d2596
wpt-pr: 33635
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 16, 2022
…pup algorithm, a=testonly

Automatic update from web-platform-tests
Fix a number of bugs in the ancestral popup algorithm

Prior to this change, there were several corner cases that would not
be properly handled, such as an anchor element that contains the
invoker of a lower-level popup. In addition, with this change, *any*
element that has an invoking attribute* that points to a popup, even
if it was not *used* to open that popup, will still act as an ancestral
popup relationship, and will not cause that popup to be light dismissed
if clicked.

See:
  openui/open-ui#508

* `togglepopup` for now, soon to also be `showpopup` and `hidepopup`

Bug: 1307772
Change-Id: I443ffd1884abfdb2856dc5d946323de4b83e20e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3601261
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#995882}

--

wpt-commits: df77a8e578af99c943f4f997b60ad9d0e753e57b
wpt-pr: 33759
jamienicol pushed a commit to jamienicol/gecko that referenced this issue May 25, 2022
…glepopup', a=testonly

Automatic update from web-platform-tests
Convert 'triggerpopup' attribute to 'togglepopup'

Per the [1] resolution, OpenUI has decided to change from
'triggerpopup' to 'togglepopup', with the corresponding behavior
change that when the popup is already open, clicking the invoking
button again will cause the popup to be hidden.

[1] openui/open-ui#508 (comment)

Bug: 1307772
Change-Id: I9a9720a2acb0c952173bed47424ffbbfc8144714
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584778
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#992556}

--

wpt-commits: 27622a853844ef5d16b1e53f83a2db5cb46d2596
wpt-pr: 33635
jamienicol pushed a commit to jamienicol/gecko that referenced this issue May 25, 2022
…pup algorithm, a=testonly

Automatic update from web-platform-tests
Fix a number of bugs in the ancestral popup algorithm

Prior to this change, there were several corner cases that would not
be properly handled, such as an anchor element that contains the
invoker of a lower-level popup. In addition, with this change, *any*
element that has an invoking attribute* that points to a popup, even
if it was not *used* to open that popup, will still act as an ancestral
popup relationship, and will not cause that popup to be light dismissed
if clicked.

See:
  openui/open-ui#508

* `togglepopup` for now, soon to also be `showpopup` and `hidepopup`

Bug: 1307772
Change-Id: I443ffd1884abfdb2856dc5d946323de4b83e20e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3601261
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#995882}

--

wpt-commits: df77a8e578af99c943f4f997b60ad9d0e753e57b
wpt-pr: 33759
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Per the [1] resolution, OpenUI has decided to change from
'triggerpopup' to 'togglepopup', with the corresponding behavior
change that when the popup is already open, clicking the invoking
button again will cause the popup to be hidden.

[1] openui/open-ui#508 (comment)

Bug: 1307772
Change-Id: I9a9720a2acb0c952173bed47424ffbbfc8144714
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584778
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#992556}
NOKEYCHECK=True
GitOrigin-RevId: e87c5eee0640e8e4848dec24ee3ea282cb016432
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Prior to this change, there were several corner cases that would not
be properly handled, such as an anchor element that contains the
invoker of a lower-level popup. In addition, with this change, *any*
element that has an invoking attribute* that points to a popup, even
if it was not *used* to open that popup, will still act as an ancestral
popup relationship, and will not cause that popup to be light dismissed
if clicked.

See:
  openui/open-ui#508

* `togglepopup` for now, soon to also be `showpopup` and `hidepopup`

Bug: 1307772
Change-Id: I443ffd1884abfdb2856dc5d946323de4b83e20e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3601261
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#995882}
NOKEYCHECK=True
GitOrigin-RevId: 8999b13d454b5fdabba3c21791e2fc7c5da2ba5c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs edits This is ready for edits to be made popover The Popover API
Projects
None yet
Development

No branches or pull requests

7 participants