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

Rename defaultopen for popovers #631

Closed
mfreed7 opened this issue Nov 1, 2022 · 3 comments
Closed

Rename defaultopen for popovers #631

mfreed7 opened this issue Nov 1, 2022 · 3 comments
Labels
popover The Popover API

Comments

@mfreed7
Copy link
Collaborator

mfreed7 commented Nov 1, 2022

See the conversation starting here from the WHATWG html spec PR for popover. The issue is that defaultopen sounds too generic, yet there are myriad questions that would be raised by a "generic" version of the defaultopen feature.

So the request is to rename this attribute to something popover-specific, e.g. popoverdefaultopen.

Let's bikeshed a popover-specific name for this attribute.

Recall that we already discussed this in #500, but the new input is that this needs to be less generic.

@mfreed7 mfreed7 added agenda+ Use this label if you'd like the topic to be added to the meeting agenda popover The Popover API labels Nov 1, 2022
@jh3y
Copy link
Collaborator

jh3y commented Nov 2, 2022

Thought this one might follow suit 👍

I do wonder if there is potentially the opportunity to line it up a little with dialog where the attribute is popoveropen and you could use that to drive the behavior too 🤔 Imagine that could potentially introduce some complications.

@css-meeting-bot
Copy link

The Open UI Community Group just discussed Rename defaultopen for popovers, and agreed to the following:

  • RESOLVED: remove the `defaultopen` attribute and its behavior entirely, for now. In the meantime, JS can be used for this behavior. Try to push forward (in WHATWG/html) a deprecation of the `open` attribute on `<details>` and `<dialog>` and a replacement with `defaultopen`.
The full IRC log of that discussion <gregwhitworth> Topic: Rename defaultopen for popovers
<gregwhitworth> github: https://github.com//issues/631
<JonathanNeal> masonf: we have this attribute — defaultopen — which will show the popup when the page is loaded. We had hoped it could become a more generic attribute that could be used on other things; similar to how we have this attribute on details/summary and dialog.
<JonathanNeal> masonf: the feedback has been that we should have it scoped to popover.
<JonathanNeal> masonf: details/summary has an open attribute which is live
<JonathanNeal> masonf: dialog has an open attribute which is also live
<JonathanNeal> masonf: we decided for popovers that a live attribute would not work
<JonathanNeal> masonf: funny things happen (especially while it’s out of the document)
<flackr> Remove all the live open attributes! \o/
<gregwhitworth> q?
<JonathanNeal> masonf: the alternative is to rename it to something possibly very long.
<jhey> q!
<JonathanNeal> q+
<JonathanNeal> q?!
<JonathanNeal> ack JonathanNeal
<jhey> q++
<JonathanNeal> ack +
<una> q+jhey
<gregwhitworth> ack jhey
<JonathanNeal> jhey: what I was thinking is: if we did something like `popoveropen` then could it be multipurpose?
<scotto_> q+
<gregwhitworth> that's akin to checked
<JonathanNeal> jhey: declare that something is open by default? and strike default off, so you could still dismiss it by removing the attribute?
<JonathanNeal> q+
<JonathanNeal> masonf: one issue is that you cannot change attributes once elements are removed from the document
<JonathanNeal> masonf: the act of removing it from the document technically can’t remove the attribute.
<JonathanNeal> jhey: what are the implications of that?
<JonathanNeal> masonf: it’s very confusing. the element will have the open attribute.
<JonathanNeal> jhey: when you say remove it, do you mean from the DOM or completely?
<JonathanNeal> masonf: completely from the DOM
<JonathanNeal> masonf: but if you stick it back into the DOM, it will be like you showed it again, because it still has the attribute.
<JonathanNeal> masonf: it ends up with a myriad of issues, like a thread, where you pull and little and everything untangles.
<JonathanNeal> jhey: so if I remove something, and then bring it back, is that a common way to do it?
<JonathanNeal> masonf: Virtual Dom does this all day long.
<flackr> virtual infinite list maybe?
<JonathanNeal> masonf: we had this discussion when it was called `initiallyopen`.
<JonathanNeal> jhey: two things I’ve learned; we like to rename things, and when we rename them we have to talk about them all over again.
<JonathanNeal> jhey: how does a dialog do it, then?
<JonathanNeal> masonf: there’s a note in the spec that says “please don’t use manipulate this attribute”.
<JonathanNeal> masonf: and then some events don’t fire in the right way.
<JonathanNeal> jhey: so no way to control it via attribute, then?
<JonathanNeal> masonf: my proposal is that it is only on page load as a behavior.
<scottkellum> q+
<gregwhitworth> ack scottkellum
<gregwhitworth> ack scotto_
<JonathanNeal> ack scotto_
<gregwhitworth> q+scottkellum
<JonathanNeal> scotto_: I understand why there’s no appetite to do this. but it opens up that anything else could need this attribute. it becomes a real slippery slope. Like “well, we didn’t want to use it any more for X reasons”. That’s a little weird, especially in cases where we could have the default open and the open have different values.
<JonathanNeal> scotto_: so, why not just do it with javascript?
<JonathanNeal> masonf: and no attribute?
<JonathanNeal> scotto_: yes, or have the attribute ,at least for parity. otherwise, none at all.
<JonathanNeal> scotto_: or do the hard work of designing the attribute that could be used on other elements.
<una> q+
<JonathanNeal> scotto_: those are the 3 options I see
<gregwhitworth> ack JonathanNeal
<gregwhitworth> JonathanNeal: I want to strongly +1 what scotto_ just said
<gregwhitworth> JonathanNeal: based on my experience with that attribute and what you mentioned in the spec masonf
<gregwhitworth> JonathanNeal: I would say the option of parity seems painful because it will happen all the moreso with popover
<gregwhitworth> JonathanNeal: the open attribute in a way shouldn't have ever happened, and that is how I also feel
<una> q-
<gregwhitworth> JonathanNeal: for those 2 reasons I don't want that parity and the 2 remaining options to split them off to handle all of them and the JS is the shippable option now
<gregwhitworth> JonathanNeal: that is why I like those options while not wanting to the JS options because parity makes the problem worse
<gregwhitworth> JonathanNeal: I think popover would suffer more from it, referring to open
<gregwhitworth> JonathanNeal: if the option is just to rename it to refer to JS option or the long hard road for generic
<JonathanNeal> back to scribing for me
<gregwhitworth> ack dbaron
<JonathanNeal> dbaron: two thoughts
<JonathanNeal> dbaron: to respond to scotto_, some of what is unusual here is that it is a feature in an attribute.
<JonathanNeal> dbaron: in some sense, the other features are not tied to an attribute.
<scotto_> q+
<JonathanNeal> dbaron: masonf said a while ago that defaultopen was only for document load, or does it work when the element is put into the document?
<gregwhitworth> q+
<jhey> You have to explicitly call showPopover() at the moment to make it work.
<JonathanNeal> masonf: as it stands now, it is document load, it is parser-only.
<JonathanNeal> masonf: while I have the microphone, I support the JS only solution.
<jhey> q+
<JonathanNeal> masonf: we never really talked about use-cases. there are a lot of testing use-cases, but I wonder if maybe JavaScript is enough.
<gregwhitworth> ack scottkellum
<JonathanNeal> scottkellum: scotto_ and masonf, I think the issues with `open` are clear. I’m trying to read back in the issues to find what the aversion to something to replace regular `open`.
<JonathanNeal> scottkellum: I know that Domenic is saying that `defaultopen` sounds too generic, but I’d love to know a little bit more about what is blocking us from having a more generic open.
<JonathanNeal> masonf: I think the viewpoint is that “the other similar elements have `open`” so why would this work differently? And if they both exist, what would happen when they are both used?
<JonathanNeal> masonf: I suppose we could try to deprecate the `open` attribute, but having deprecated things; that’s hard to do.
<JonathanNeal> scottkellum: I feel that. The longer attribute name feels very verbose.
<gregwhitworth> ack scotto_
<JonathanNeal> masonf: I think the best answer is to go back and add `defaultopen` to all the other elements; that’s a pretty good world.
<JonathanNeal> scotto_: my thought is: where would the appetite be for adding `defaultopen` to details and dialog?
<JonathanNeal> scotto_: or would it be `defaultdetailsopen` and `defaultdialogopen`?
<JonathanNeal> scotto_: the other thing that I wanted to make mention of; regarding the use-cases — I remember that we did talk about some of them, like the teaching UI where you might want to have a kind of popup to describe some thing.
<JonathanNeal> scotto_: but really, that is a dialog element that is given popover behaviors.
<JonathanNeal> scotto_: it could be opened by default with the open attribute.
<JonathanNeal> scotto_: and then there's autofocus. so if something with an open attribute has something inside it with autofocus, would that be a signal? could that be some way to do it without introducing more javascript?
<JonathanNeal> q+
<masonf> q+
<JonathanNeal> gregwhitworth: starting with the first issue; we’re compounding problems. It reminds me of working on flex and grids and floats.
<JonathanNeal> gregwhitworth: when you were talking about your utopia, I think we do that, and then we do the hard slog to figure that out, and we ship the JS solution for now.
<JonathanNeal> gregwhitworth: I think ‘foodefaultopen’ is silly.
<JonathanNeal> gregwhitworth: I agree with the Scotts.
<JonathanNeal> gregwhitworth: when I was playing with dialog it broke my brain that I had an HTML element but I needed JavaScript to use it.
<JonathanNeal> gregwhitworth: maybe getting it out there, we can see what the feedback is like. I’m totally cool from a product perspective to meet the needs in JS, and then see if the needs exist to have the generic attribute, to then go and deal with the <details> and <dialog> discussions.
<gregwhitworth> ack gregwhitworth
<JonathanNeal> gregwhitworth: I’d love to resolve on just +1 the JS API and defer the attribute until we can solve it correctly.
<JonathanNeal> jhey: I think a `defaultshow` attribute or whatever we call it needs to exist.
<JonathanNeal> jhey: If we show them the one JS line, how many people are gonna do it?
<JonathanNeal> jhey: or will they say “I’ll stick my library that does it for me”.
<gregwhitworth> that's a stronger argument IMO
<JonathanNeal> jhey: and what if I’m hydrating; do I want to load JS to make it appear right?
<JonathanNeal> jhey: we need a stat on how many welcome screens there are on the internet.
<gregwhitworth> ack jhey
<gregwhitworth> ack JonathanNeal
<gregwhitworth> JonathanNeal: I'm saying what gregwhitworth said
<gregwhitworth> JonathanNeal: +1 to JS API and also agree with jhey and we should solve this issue
<gregwhitworth> JonathanNeal: I hadn't thought about PE and SSR story
<gregwhitworth> JonathanNeal: but I want to put light on that too
<gregwhitworth> JonathanNeal: as I'm talking to folks building build tools they're is a lot of ambiguity about how one can have a good loading experience and good PE experience
<gregwhitworth> JonathanNeal: to jhey point if you don't use a standard you could still provide that
<gregwhitworth> JonathanNeal: I know popover isn't going to impact layout but it can still have an effect
<gregwhitworth> JonathanNeal: I can see usage going down if libs are doing it better
<gregwhitworth> JonathanNeal: it would cost something to get the PE
<gregwhitworth> ack masonf
<JonathanNeal> masonf: my takeaways have been that nobody likes `defaultpopupopen` because it’s really long and it’s not a pattern we want to continue.
<masonf> - Proposed resolution: remove the `defaultopen` attribute and its behavior entirely, for now. In the meantime, JS can be used for this behavior. Try to push forward (in WHATWG/html) a deprecation of the `open` attribute on `<details>` and `<dialog>` and a replacement with `defaultopen`.
<JonathanNeal> masonf: what I’m wondering is if, for now, we could go with this proposal (written above).
<JonathanNeal> masonf: in the meantime, it feels like this is the right way to go: deprecating the `open` attribute down the road. It would be possible now, since we have an `:open` pseudo-class.
<JonathanNeal> masonf: We could try to solve that in the right way, and avoid the long attribute here for now.
<JonathanNeal> +1
<JonathanNeal> q?
<JonathanNeal> +1 to the proposed resolution
<scotto_> +1. agree we eventually need a way to do this, but let's not rush it
<masonf> RESOLVED: remove the `defaultopen` attribute and its behavior entirely, for now. In the meantime, JS can be used for this behavior. Try to push forward (in WHATWG/html) a deprecation of the `open` attribute on `<details>` and `<dialog>` and a replacement with `defaultopen`.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Nov 10, 2022

I commented on the WHATWG PR with a summary of our discussion today.

@mfreed7 mfreed7 closed this as completed Nov 10, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 10, 2022
Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 11, 2022
Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 12, 2022
Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 15, 2022
Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 15, 2022
Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 15, 2022
Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 15, 2022
Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4021706
Reviewed-by: Nate Fischer <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1071760}
aarongable pushed a commit to chromium/chromium that referenced this issue Nov 15, 2022
Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4021706
Reviewed-by: Nate Fischer <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1071760}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 15, 2022
Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4021706
Reviewed-by: Nate Fischer <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1071760}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 21, 2022
…te, a=testonly

Automatic update from web-platform-tests
Remove the popover `defaultopen` attribute

Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4021706
Reviewed-by: Nate Fischer <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1071760}

--

wpt-commits: 3c338f5bbfd6a5b7ea8912a8c58e910a91a95e6c
wpt-pr: 36924
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Nov 21, 2022
…te, a=testonly

Automatic update from web-platform-tests
Remove the popover `defaultopen` attribute

Per the [1] resolution, we have decided to remove this feature
for now, while we work on ways to make it more generic and
applicable to other elements such as `<details>` and `<dialog>`.

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

Bug: 1307772
Change-Id: I7bc385fcbe1fadfcdf83d2d65c2e46abc07cb8a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4021706
Reviewed-by: Nate Fischer <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1071760}

--

wpt-commits: 3c338f5bbfd6a5b7ea8912a8c58e910a91a95e6c
wpt-pr: 36924
@mfreed7 mfreed7 removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Nov 30, 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