Skip to content

Invalid use of "find the topmost popover ancestor" #245

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.

Sign up for GitHub

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

Jump to bottom
Open
theIDinside opened this issue Mar 17, 2025 · 2 comments
Open

Invalid use of "find the topmost popover ancestor" #245

theIDinside opened this issue Mar 17, 2025 · 2 comments

Comments

@theIDinside
Copy link

theIDinside commented Mar 17, 2025 •
edited
Loading

Recently a new PR was pulled in: Add popover=hint #9778

It adds another argument to the find the topmost popover ancestor:

  1. a Node newPopoverOrTopLayerElement
  2. a list popoverList <- this one
  3. an HTML element or null invoker
  4. a boolean isPopover

However, to fullscreen an element we are still only using the old three argument-version.

@josepharhar wrote the pull request for the new popover attribute value so I guess my question is what would be the parameter for the list in the Fullscreen API scenario? I'm not well versed in how popover works or is intended to work, but I take that the specific change for the PR is to add tooltip-like stuff, and so, I wonder, should all these be auto closed and only have the "manual" popovers left open or should it be auto? But that's just my initial guesses after having peaked at it for 5 minutes.
@lukewarlow
Copy link
Member

PR to export the necessary definitions from HTML: whatwg/html#11140

theIDinside added a commit to theIDinside/fullscreen that referenced this issue Mar 17, 2025
@theIDinside
Fix whatwg#245 - Addressing new popover values
57139fb 
Since the popover attribute got a new value, it's
"find topmost popover ancestor" algorithm changed, and this PR
attempts to address this in the Fullscreen API specification.

After discussing with Luke Warlow we came to the conclusion
that this is probably the intended change for the Fullscreen API spec.
@theIDinside theIDinside mentioned this issue Mar 18, 2025
Open
5 tasks
@josepharhar
Copy link
Contributor

Thanks luke!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@josepharhar @theIDinside @lukewarlow