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

Showing only a child popover causes "beforetoggle" and "toggle" events but popover is invisble #9487

Open
mbrodesser-Igalia opened this issue Jul 4, 2023 · 10 comments
Labels
topic: popover The popover attribute and friends

Comments

@mbrodesser-Igalia
Copy link
Member

Example: https://jsfiddle.net/ux09nbsy/1/

Tested Chrome Dev edition and Firefox, both on Ubuntu 22.04.

It's seems desirable to actually show the popup. Otherwise, which is unlikely, the spec would need to be adapted.

CC @josepharhar

@mbrodesser-Igalia mbrodesser-Igalia added the topic: popover The popover attribute and friends label Jul 4, 2023
@josepharhar
Copy link
Contributor

Hmm yeah I see that the child popover can be showing while the parent is not showing. I suppose it would make sense to disallow this for popover=auto elements where the hierarchy means something.

It's seems desirable to actually show the popup.

In order for the child popover to be showing, the parent would need to also be showing right? Or do you think we should change the rendering somehow in a way that allows the child to be visible?

@mbrodesser-Igalia
Copy link
Member Author

Hmm yeah I see that the child popover can be showing while the parent is not showing. I suppose it would make sense to disallow this for popover=auto elements where the hierarchy means something.

Right. That would be aligned with https://open-ui.org/components/popover.research.explainer/#nested-popovers.
So the spec needs to be adapted to cover that.

In order for the child popover to be showing, the parent would need to also be showing right?

Yes.

Or do you think we should change the rendering somehow in a way that allows the child to be visible?

No, as long as there are no use-cases for it.

@mbrodesser-Igalia
Copy link
Member Author

I suppose it would make sense to disallow this for popover=auto elements where the hierarchy means something.

@nt1m what's Apple's view on this?

@josepharhar
Copy link
Contributor

In order to resolve this we could add a step to check popover validity which looks for the closest popover=auto ancestor and stops popovers from being shown if their popover=auto ancestor is not showing, right?

@nt1m
Copy link
Member

nt1m commented Jul 11, 2023

I'm tempted to say we don't really need to address this, it's like the case you're putting a popover inside <div hidden> IMO.

@mbrodesser-Igalia
Copy link
Member Author

In order to resolve this we could add a step to check popover validity which looks for the closest popover=auto ancestor and stops popovers from being shown if their popover=auto ancestor is not showing, right?

That should work, although it might make "check popover validity" harder to understand and too generic. Actually seeing the change would help. Adding specific checks to https://html.spec.whatwg.org/#show-popover only could lead to a clearer structure, since that's the only function requiring such checks.

@mbrodesser-Igalia
Copy link
Member Author

I'm tempted to say we don't really need to address this, it's like the case you're putting a popover inside <div hidden> IMO.

Hard to judge without actual use cases. But since the issue is pretty easy to reproduce, it might be worth fixing.

@nt1m
Copy link
Member

nt1m commented Jul 11, 2023

I don't really have a strong opinion here, but I guess the most simple solutions would be:

  • throw an exception
  • keep it as it is

@mbrodesser-Igalia
Copy link
Member Author

I don't really have a strong opinion here, but I guess the most simple solutions would be:

* throw an exception

* keep it as it is

+1 for throwing an exception.

@josepharhar
Copy link
Contributor

I'm tempted to say we don't really need to address this, it's like the case you're putting a popover inside <div hidden> IMO.

Agreed

In order to resolve this we could add a step to check popover validity which looks for the closest popover=auto ancestor and stops popovers from being shown if their popover=auto ancestor is not showing, right?

That should work, although it might make "check popover validity" harder to understand and too generic. Actually seeing the change would help. Adding specific checks to https://html.spec.whatwg.org/#show-popover only could lead to a clearer structure, since that's the only function requiring such checks.

What exactly should we add in either place if we do this anyway? I don't think we have an algorithm already which just finds the closest popover=auto ancestor or lets us easily iterate over all the ancestors as defined by the relationships with invokers etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: popover The popover attribute and friends
Development

No branches or pull requests

3 participants