[popup] Boolean property for show state

[0kku]

Is there a reason why there's no mention (that I can find) of a writable property for the "shown" state of a popup?

For example, it could look something like this:

popup.popUpShown // false
popup.showPopUp();
popup.popUpShown // true
popup.popUpShown = false;
popup.popUpShown // false

Or alternatively, it could be called popUpOpen.

I realize that one can check if a popup is being shown using popUp.matches(":open") as described in the explainer. However, this does not allow assigning a boolean value for the state. This would be helpful in a framework setting:

function MyComponent () {
  const shown = state(false);
  
  return html`
    <div popUp="auto" popUpShown=${shown} />
  `;
}

While, from what I understand, the current API would require one to write cumbersome code like this:

function MyComponent () {
  const shown = state(false);
  const popup = ref();
  
  if (shown && !popUp.matches(":open")) popup.showPopUp();
  else if (!shown && popUp.matches(":open")) popup.hidePopUp();
  
  return html`
    <div popUp="auto" ref=${popup} />
  `;
}

This would be less cumbersome if there was a toggle method that takes an argument for forcing the state similar to APIs like element.toggleAttribute(), classList.toggle(), etc. (which also doesn't seem to be present), though it would still be less ergonomic than having the proposed property:

function MyComponent () {
  const shown = state(false);
  const popup = ref();
  
  popup.togglePopUp(shown);
  
  return html`
    <div popUp="auto" ref=${popup} />
  `;
}

[tbondwilkinson]

I think the "toggle" API isn't a bad idea, perhaps you should open a separate issue for that case specifically?

I don't think I like the idea of a shown attribute controlling whether a popup is shown or not, and if it did exist it should be readonly rather than controlling behavior. I like having things that "do" things as methods, and things that are "observable" as properties.

[0kku]

Can you elaborate on why a writable property would be undesirable? It seems like it would be idiomatic and in-line with other DOM API properties like disabled, checked, indeterminate, required, etc. on input elements for example.

It seems like a settable property controlling its state would be vital for its adoption in contexts where a frontend UI framework that relies on setting properties is used. Without it, people would be left with having to create their own abstractions on top of the native element for each framework; as with the property missing, people would be unable to write templates declaratively, and would instead be forced to figure out a way to call a function in an imperative way. As demonstrated above, that makes for considerably less legible code.

[tbondwilkinson]

But it's inconsistent with other UI elements, like showModal, read the note in the spec about removing the "open" attribute, since the attribute is now readonly.

Because events, animations, etc. might all be triggered by a close or open mutation, those mutations should in my opinion be best represented as a method, rather than a property.

The difference between a browser native that does show/hide and something like a web framework is that (taking React as an example), setState is not a synchronous call to mutate an observable DOM node. setState schedules a data mutation and performs a render() using that new data. React, and other web frameworks, use data mutations as the basis for rendering.

The vanilla web as currently written does no such thing, there is no "data" model that you can mutate to cause the browser to do a (batched) re-rendering of the DOM. When in React you're reading the "isOpen" property of your data model, that's different than reading properties directly from the underlying DOM node itself.

If there were a way to link data Objects with DOM Objects, I agree that it would be pretty reasonable to have a data property map to "open."

[mfreed7]

+100 to avoiding a writable attribute that "does" things, not just because of the arguments above, but because there are myriad corner cases. E.g. what happens when the element is not connected to a document (and so, can't be shown), but you do popUp.shown=true?

Assuming we don't do that, and just have a readable value, then I suppose this is just sugar over popUp.matches(':open'). I agree that's a bit cumbersome to type, but you could always write a helper function or something. But I'm not against adding it, if there's strong desire. We already have pushback on the number of attributes and properties being added for this API.

The suggestion to add popUp.togglePopUp() is interesting. Along the same lines as above, it's an ergonomic helper. However, I suppose I'd hope most such usage instead uses the popuptoggletarget attribute, to make sure e.g. a11y is correctly set up.

I'll agenda+ this to discuss.

[css-meeting-bot]

The Open UI Community Group just discussed [popup] Boolean property for show state.

The full IRC log of that discussion

<gregwhitworth> Topic: [popup] Boolean property for show state
<JonathanNeal> masonf: this is #610. it’s a request for 3 different things.
<JonathanNeal> masonf: one of the suggestions is to add a popup shown attribute.
<JonathanNeal> masonf: it might be convenient
<JonathanNeal> masonf: request 2 is to make the property (I missed this part)
<scotto_> q+
<JonathanNeal> request 3 is to add a toggle popup
<gregwhitworth> github: https://github.com//issues/610
<JonathanNeal> masonf: request 3 is to add a toggle popup
<JonathanNeal> masonf: and if you want, I have proposed resolution.
<gregwhitworth> ack scotto_
<masonf> Proposed resolution: add `togglePopUp()` method which toggles a pop-up. Hold off for now on adding a `popUpShown` attribute.
<bkardell_> q+
<JonathanNeal> scotto_: I know this is something people would want to try to do. For the button that shows the popup, how much one style that to indicate that the popup is in the shown state?
<flackr> q+ to align attribute on name - e.g. open
<tantek> only feedback I have is I'd prefer "shown" rather than "true", per the pattern of avoiding literal boolean attributes
<JonathanNeal> scotto_: it sounds like we’ve been talking about the pseudo class for the popup itself, but what about for the triggering element?
<JonathanNeal> scotto_: an html attribute, that could be styled. right now we do that with aria-expanded="true"/"false" but
<JonathanNeal> scotto_: we don’t have that for this.
<tantek> also what's the value of such an attribute *while* it is animating from one state to the other?
<JonathanNeal> masonf: yes, right now there is no way to style the element that triggered the popup.
<tantek> (i.e. neither true nor false is correct)
<JonathanNeal> masonf: on the specific select menu use case, you can style it based on open and closed, but the general case doesn’t have anything.
<JonathanNeal> gregwhitworth: but on the general use case on the button being tied, I agree.
<gregwhitworth> ack bkardell_
<JonathanNeal> bkardell_: so, "open" is conceptually kind of a state, but "show" and "hide", they seem
<JonathanNeal> bkardell_: they seem to be about the visual presentation
<JonathanNeal> bkardell_: I don’t know in the use cases that remain, at least for hint, there are use cases where you may want to print them
<JonathanNeal> bkardell_: and you can totally show the popup all the time instead of waiting for it to require activation
<JonathanNeal> bkardell_: so I’m curious if we’ve made this by a conceptual thing like "active" rather than "show" or "hide".
<JonathanNeal> masonf: is the suggestion to rename "show" and "hide" for popup?
<JonathanNeal> bkardell_: yea. as far as I understand, we are talking about renaming them.
<JonathanNeal> bkardell_: so, if we change the display to block, is it shown?
<JonathanNeal> masonf: no, if you manually change it, it will now match "open".
<JonathanNeal> bkardell_: but in your author styles, if you put style block, it will be showing.
<JonathanNeal> masonf: it will be shown, but it won’t be "shown"
<JonathanNeal> bkardell_: these things have bitten us in the past
<JonathanNeal> bkardell_: part of me kind of like discussing this in terms of a state
<JonathanNeal> bkardell_: and that it might be tied to visibility, but it might not.
<gregwhitworth> ack flackr
<Zakim> flackr, you wanted to align attribute on name - e.g. open
<JonathanNeal> masonf: and I think we want that which is why we picked open and closed
<JonathanNeal> gregwhitworth: this was a great summary, and let’s pick this up next week
<JonathanNeal> gregwhitworth: please weigh in the proposed solution.
<gregwhitworth> Zakim, end meeting
<Zakim> As of this point the attendees have been miriam, una, masonf, JonathanNeal, dandclark, scotto_, flackr, dbaron, emilio, +q, tantek
<Zakim> RRSAgent, please draft minutes
<RRSAgent> I have made the request to generate https://www.w3.org/2022/10/13-openui-minutes.html Zakim
<Zakim> I am happy to have been of service, gregwhitworth; please remember to excuse RRSAgent. Goodbye

[bkardell]

without commenting on more, I like the idea of adding a toggle for parity with some similar things in the platform already even

[css-meeting-bot]

The Open UI Community Group just discussed [popup] Boolean property for show state, and agreed to the following:

• RESOLVED: add `togglePopUp()` method which toggles a pop-up.

The full IRC log of that discussion

<gregwhitworth> Topic: [popup] Boolean property for show state
<gregwhitworth> github: https://github.com//issues/610
<JonathanNeal> masonf: we talked about this last time, briefly. The issue brakes down into 3 things:
<JonathanNeal> masonf: request for a boolean attribute called something like popup shown
<JonathanNeal> masonf: it would give the same output as open.
<JonathanNeal> masonf: second request, make that attribute writable; so it can open and close it.
<JonathanNeal> masonf: third request, add another method, toggle popup, that only opens or closes popups
<gregwhitworth> I'm supportive of 1 & 3, not 2
<JonathanNeal> masonf: These are all things you can already do. The one that seems reasonable right now is the toggle popup method.
<JonathanNeal> masonf: This would make the JS methods align very nicely.
<gregwhitworth> q+
<JonathanNeal> masonf: the code you have to write otherwise is a lot more.
<masonf> - Proposed resolution: add `togglePopUp()` method which toggles a pop-up. Hold off for now on adding a `popUpShown` attribute.
<JonathanNeal> masonf: the code you have to write otherwise for toggle popup is a lot more.
<JonathanNeal> gregwhitworth: I'm not supportive of making it writable, because it would pair to other existing APIs.
<tantek> I'm ok with that. Same comments as before about popUpShown / boolean attributes, preferring not just true/false values
<JonathanNeal> gregwhitworth: when you say “hold off”, do you mean like V2?
<JonathanNeal> gregwhitworth: are you saying this isn’t something we need to ship right now, and that’s why we want to pause it?
<JonathanNeal> gregwhitworth: I would rather resolve they both exist, separately from whether it is shipped.
<gregwhitworth> ack gregwhitworth
<JonathanNeal> masonf: my motivation for punting that part for now is to de-risk the spec.
<masonf> Proposed resolution: add `togglePopUp()` method which toggles a pop-up.
<JonathanNeal> gregwhitworth: I'm supportive, so I would adjust the proposal so we can pick it up some time later down the road.
<masonf> RESOLVED: add `togglePopUp()` method which toggles a pop-up.