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

Add popover=hint #9778

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open

Add popover=hint #9778

wants to merge 36 commits into from

Conversation

josepharhar
Copy link
Contributor

@josepharhar josepharhar commented Sep 23, 2023

Fixes #9776

Explainer: https://open-ui.org/components/popover-hint.research.explainer/#popoverhint-behavior

(See WHATWG Working Mode: Changes for more details.)


/interactive-elements.html ( diff )
/popover.html ( diff )

@mfreed7 mfreed7 mentioned this pull request Sep 29, 2023
3 tasks
@nt1m
Copy link
Member

nt1m commented Oct 8, 2023

I would probably want https://github.com/keithamus/invoker-buttons-proposal to be worked on first before addressing this, as I think it's a building block for this and other declarative attributes.

@mfreed7
Copy link
Collaborator

mfreed7 commented Oct 30, 2023

I would probably want https://github.com/keithamus/invoker-buttons-proposal to be worked on first before addressing this, as I think it's a building block for this and other declarative attributes.

So I've significantly updated the popover=hint explainer, and there are a few sections detailing why I believe the functionality described in this PR is independent of the hover-triggering functionality described by the Invokers explainer (note that it was moved to the OpenUI repo). See this section:

Indeed both are important bits of functionality, but I'm hoping we can tackle them independently rather than needing to put up another huge spec PR. That didn't work very well for the original Popover spec PR, at least in my opinion.

@mfreed7 mfreed7 mentioned this pull request Oct 30, 2023
@josepharhar josepharhar added the topic: popover The popover attribute and friends label Dec 21, 2023
@josepharhar
Copy link
Contributor Author

Note to self: this will require additional changes in https://chromium-review.googlesource.com/c/chromium/src/+/5229300 if the corresponding PR to be opened gets merged

Copy link
Collaborator

@mfreed7 mfreed7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I reviewed mostly for the correctness of the algorithms, and things look pretty good. I made a few corrections, but other than those, LGTM.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
@mfreed7
Copy link
Collaborator

mfreed7 commented Mar 7, 2024

Gentle ping - any chance we could get an initial review of this PR? @annevk @domenic

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more editorial nits

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with final nit

source Show resolved Hide resolved
@mfreed7 mfreed7 added the agenda+ To be discussed at a triage meeting label Apr 25, 2024
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

Successfully merging this pull request may close these issues.

popover=hint
5 participants