Skip to content

Commit

Permalink
Move hide event before state changes on :open
Browse files Browse the repository at this point in the history
Move the timing of the popover `popoverhide` event a bit earlier
in the process, just *before* the state changes to "transitioning".
This has the effect of making the state clean (either :open or
:closed) for both `popoverhide` and `popovershow`.

See the discussion here:
openui/open-ui#607

Bug: 1307772
Change-Id: I5aaabfd48204e54831ce435c71a54e692e139caf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4032919
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1074159}
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Nov 21, 2022
1 parent c6addf5 commit 948353d
Showing 1 changed file with 14 additions and 4 deletions.
18 changes: 14 additions & 4 deletions html/semantics/popovers/popover-events.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,31 @@
assert_false(popover.matches(':open'));
let showCount = 0;
let hideCount = 0;
function showListener(e) {
assert_true(e.target.matches(':closed'),'The popover should be in the :closed state when the popovershow event fires.');
assert_false(e.target.matches(':open'),'The popover should *not* be in the :open state when the popovershow event fires.');
++showCount;
};
function hideListener(e) {
assert_true(e.target.matches(':open'),'The popover should be in the :open state when the popoverhide event fires.');
assert_false(e.target.matches(':closed'),'The popover should *not* be in the :closed state when the popoverhide event fires.');
++hideCount;
};
switch (method) {
case "listener":
const controller = new AbortController();
const signal = controller.signal;
t.add_cleanup(() => controller.abort());
document.addEventListener('popovershow',() => ++showCount, {signal});
document.addEventListener('popoverhide',() => ++hideCount, {signal});
document.addEventListener('popovershow',showListener, {signal});
document.addEventListener('popoverhide',hideListener, {signal});
break;
case "attribute":
assert_false(popover.hasAttribute('onpopovershow'));
assert_false(popover.hasAttribute('onpopoverhide'));
t.add_cleanup(() => popover.removeAttribute('onpopovershow'));
t.add_cleanup(() => popover.removeAttribute('onpopoverhide'));
popover.onpopovershow = () => ++showCount;
popover.onpopoverhide = () => ++hideCount;
popover.onpopovershow = showListener;
popover.onpopoverhide = hideListener;
break;
default: assert_unreached();
}
Expand Down

0 comments on commit 948353d

Please sign in to comment.