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

Only use the used invoker to establish popover hierarchy #39560

Merged
merged 1 commit into from Apr 18, 2023

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Apr 14, 2023

See [1] for more context, but the idea is that instead of using just
the DOM structure to establish the popover hierarchy, the user's
behavior should matter. For example, if one popover contains a popover
invoker pointing to another popover, it should matter whether that
invoker is actually used to open the second popover.

An example:

  • Component 1 is a third party widget, which uses popover
  • Component 2 is another third party widget, also using popover
  • A page wants to use both components separately, from separate
    invoking buttons.
  • Component 1 also wants to be able to use Component 2, via a button
    within Component 1.

In this example, the page should be able to still independently use
these components. So a user clicking the page's button for Component 2
is expected to close Component 1 if it's open, because that's a direct
invocation of Component 2. However, if the user clicks the button
within Component 1 to get Component 2, it is natural to leave Component
1 open because this is a nested call.

Important note: this often happens to be the behavior even before this
CL, since the user clicking on the page-level Component 2 invoking
button represents a light dismiss signal for Component 1, so it closes
either way. But this CL simplifies the implementation considerably,
removing the need to track all invokers on the page, and also removing
the need to continuously check whether invoker relationships have
changed.

Spec PR:
whatwg/html#9171

[1] whatwg/html#9160

Bug: 1307772
Change-Id: I60ccb133a79084db8c251218fdbd10684fea947b
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4429412
Commit-Queue: Mason Freed <masonf@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1131606}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

See [1] for more context, but the idea is that instead of using just
the DOM structure to establish the popover hierarchy, the user's
behavior should matter. For example, if one popover contains a popover
invoker pointing to another popover, it should matter whether that
invoker is *actually used* to open the second popover.

An example:
 - Component 1 is a third party widget, which uses popover
 - Component 2 is another third party widget, also using popover
 - A page wants to use both components separately, from separate
   invoking buttons.
 - Component 1 also wants to be able to use Component 2, via a button
   within Component 1.

In this example, the page should be able to still independently use
these components. So a user clicking the page's button for Component 2
is expected to close Component 1 if it's open, because that's a direct
invocation of Component 2. However, if the user clicks the button
within Component 1 to get Component 2, it is natural to leave Component
1 open because this is a nested call.

Important note: this often happens to be the behavior even before this
CL, since the user clicking on the page-level Component 2 invoking
button represents a light dismiss signal for Component 1, so it closes
either way. But this CL simplifies the implementation considerably,
removing the need to track all invokers on the page, and also removing
the need to continuously check whether invoker relationships have
changed.

Spec PR:
whatwg/html#9171

[1] whatwg/html#9160

Bug: 1307772
Change-Id: I60ccb133a79084db8c251218fdbd10684fea947b
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4429412
Commit-Queue: Mason Freed <masonf@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1131606}
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 5acfa51 into master Apr 18, 2023
18 checks passed
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-4429412 branch April 18, 2023 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants