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

Change popover invoking attributes #38701

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Feb 24, 2023

See [1] for discussion. This changes the Popover invoking attribute
behavior. Previously, three attributes were used to set the invoking
relationship and behavior:

  • popovertoggletarget
  • popovershowtarget
  • popoverhidetarget

Those each could point to an idref for a popover, and the behavior
would be controlled by which attribute was used. However, there was
a confusing situation in the case that multiple such attributes
were used on a single element, and even more so when they pointed
to different elements. While there was concrete logic for resolving
that situation, it was confusing at best.

The new logic is controlled by two attributes:

  • popovertarget
  • popovertargetaction

The element reference is dictated by popovertarget, via an idref
or through element reflection (via foo.popoverTargetElement = bar).
The behavior is dictated by the (string valued) popovertargetaction
attribute, which can be one of "toggle", "show", or "hide". If any
other value is used (including missing attribute or invalid value),
the behavior and IDL reflected value is "toggle".

[1] whatwg/html#8894 (comment)

Bug: 1307772
Change-Id: I2e530efe26032599f9376c8d5f4fe2a7f430a26b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4288730
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1109867}

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 discussion. This changes the Popover invoking attribute
behavior. Previously, three attributes were used to set the invoking
relationship and behavior:
 - popovertoggletarget
 - popovershowtarget
 - popoverhidetarget

Those each could point to an idref for a popover, and the behavior
would be controlled by which attribute was used. However, there was
a confusing situation in the case that multiple such attributes
were used on a single element, and even more so when they pointed
to different elements. While there was concrete logic for resolving
that situation, it was confusing at best.

The new logic is controlled by two attributes:
 - popovertarget
 - popovertargetaction

The element reference is dictated by `popovertarget`, via an idref
or through element reflection (via `foo.popoverTargetElement = bar`).
The behavior is dictated by the (string valued) popovertargetaction
attribute, which can be one of "toggle", "show", or "hide". If any
other value is used (including missing attribute or invalid value),
the behavior and IDL reflected value is "toggle".

[1] whatwg/html#8894 (comment)

Bug: 1307772
Change-Id: I2e530efe26032599f9376c8d5f4fe2a7f430a26b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4288730
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1109867}
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 588e828 into master Feb 24, 2023
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-4288730 branch February 24, 2023 23:24
annevk added a commit to whatwg/html that referenced this pull request Mar 1, 2023
Instead of popoverhidetarget, popovershowtarget, and popovertoggletarget, we will now have popovertarget and popovertargetaction. The former for targeting an element and the latter for determining what to do with the target.

Tests: web-platform-tests/wpt#38701.
annevk added a commit to whatwg/html that referenced this pull request Mar 1, 2023
Instead of popoverhidetarget, popovershowtarget, and popovertoggletarget, we will now have popovertarget and popovertargetaction. The former for targeting an element and the latter for determining what to do with the target.

Tests: web-platform-tests/wpt#38701.

Fixes #8894.
annevk added a commit to whatwg/html that referenced this pull request Mar 3, 2023
Instead of popoverhidetarget, popovershowtarget, and popovertoggletarget, we will now have popovertarget and popovertargetaction. The former for targeting an element and the latter for determining what to do with the target.

Tests: web-platform-tests/wpt#38701.

Fixes #8894.
annevk added a commit to whatwg/html that referenced this pull request Mar 7, 2023
Instead of popoverhidetarget, popovershowtarget, and popovertoggletarget, we will now have popovertarget and popovertargetaction. The former for targeting an element and the latter for determining what to do with the target.

Tests: web-platform-tests/wpt#38701.

Fixes #8894.
annevk added a commit to whatwg/html that referenced this pull request Mar 8, 2023
Instead of popoverhidetarget, popovershowtarget, and popovertoggletarget, we will now have popovertarget and popovertargetaction. The former for targeting an element and the latter for determining what to do with the target.

This also corrects popover validity checks in activation behavior and consistifies the popover attribute.

Tests: web-platform-tests/wpt#38701.

HTML-AAM: w3c/html-aam#446.

Fixes #8894, fixes #8983, and fixes #8979.
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.

4 participants