From e3246afc19a62a2d4f26786e6eae72a601a2c032 Mon Sep 17 00:00:00 2001 From: Mason Freed Date: Thu, 12 Jan 2023 17:03:18 -0800 Subject: [PATCH] Add `aftertoggle` event for popover This CL adds an async `aftertoggle` event to both the show and hide popover transitions, and renames the event class from BeforeToggle to PopoverToggle, to be used by both events. This was resolved by OpenUI here: https://github.com/openui/open-ui/issues/342#issuecomment-1380882740 Bug: 1307772 Change-Id: I996be74b90f43eee4bf859cecfed051fa6f633d5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4160443 Auto-Submit: Mason Freed Commit-Queue: Joey Arhar Commit-Queue: Mason Freed Reviewed-by: Joey Arhar Cr-Commit-Position: refs/heads/main@{#1092158} --- .../popovers/idlharness.tentative.html | 9 +- .../popovers/popover-events.tentative.html | 69 +++++++++++--- .../toggleevent-interface.tentative.html | 92 +++++++++---------- interfaces/popover.tentative.idl | 6 +- 4 files changed, 109 insertions(+), 67 deletions(-) diff --git a/html/semantics/popovers/idlharness.tentative.html b/html/semantics/popovers/idlharness.tentative.html index 478dec84c8195f..d1a258bb4ad3ac 100644 --- a/html/semantics/popovers/idlharness.tentative.html +++ b/html/semantics/popovers/idlharness.tentative.html @@ -41,9 +41,12 @@ 'document.getElementById("b3")', ], BeforeToggleEvent: [ - 'new BeforeToggleEvent("beforetoggle")', - 'new BeforeToggleEvent("beforetoggle", {currentState: "open"})', - 'new BeforeToggleEvent("beforetoggle", {currentState: "open",newState: "open"})', + 'new PopoverToggleEvent("beforetoggle")', + 'new PopoverToggleEvent("beforetoggle", {currentState: "open"})', + 'new PopoverToggleEvent("beforetoggle", {currentState: "open",newState: "open"})', + 'new PopoverToggleEvent("aftertoggle")', + 'new PopoverToggleEvent("aftertoggle", {currentState: "open"})', + 'new PopoverToggleEvent("aftertoggle", {currentState: "open",newState: "open"})', ], }); } diff --git a/html/semantics/popovers/popover-events.tentative.html b/html/semantics/popovers/popover-events.tentative.html index c88dc21dfa23fb..b96a0f5a53f489 100644 --- a/html/semantics/popovers/popover-events.tentative.html +++ b/html/semantics/popovers/popover-events.tentative.html @@ -16,19 +16,40 @@ const popover = document.querySelector('[popover]'); assert_false(popover.matches(':open')); let showCount = 0; + let afterShowCount = 0; let hideCount = 0; + let afterHideCount = 0; function listener(e) { - if (e.newState === "open") { - assert_equals(e.currentState,"closed",'Popover toggleevent states should be "open" and "closed"') - assert_true(e.target.matches(':closed'),'The popover should be in the :closed state when the opening event fires.'); - assert_false(e.target.matches(':open'),'The popover should *not* be in the :open state when the opening event fires.'); - ++showCount; + if (e.type === "beforetoggle") { + if (e.newState === "open") { + assert_equals(e.currentState,"closed",'The "beforetoggle" event should be fired before the popover is open'); + assert_true(e.target.matches(':closed'),'The popover should be in the :closed state when the opening event fires.'); + assert_false(e.target.matches(':open'),'The popover should *not* be in the :open state when the opening event fires.'); + ++showCount; + } else { + assert_equals(e.newState,"closed",'Popover toggleevent states should be "open" and "closed"'); + assert_equals(e.currentState,"open",'The "beforetoggle" event should be fired before the popover is closed') + assert_true(e.target.matches(':open'),'The popover should be in the :open state when the hiding event fires.'); + assert_false(e.target.matches(':closed'),'The popover should *not* be in the :closed state when the hiding event fires.'); + ++hideCount; + } } else { - assert_equals(e.currentState,"open",'Popover toggleevent states should be "open" and "closed"') - assert_equals(e.newState,"closed",'Popover toggleevent states should be "open" and "closed"') - assert_true(e.target.matches(':open'),'The popover should be in the :open state when the hiding event fires.'); - assert_false(e.target.matches(':closed'),'The popover should *not* be in the :closed state when the hiding event fires.'); - ++hideCount; + assert_equals(e.type,"aftertoggle",'Popover events should be "beforetoggle" and "aftertoggle"') + if (e.newState === "open") { + assert_equals(e.currentState,"open",'Aftertoggle should be fired after the popover is open'); + if (document.body.contains(e.target)) { + assert_true(e.target.matches(':open'),'The popover should be in the :open state when the after opening event fires.'); + assert_false(e.target.matches(':closed'),'The popover should *not* be in the :closed state when the after opening event fires.'); + } + ++afterShowCount; + } else { + assert_equals(e.newState,"closed",'Popover toggleevent states should be "open" and "closed"'); + assert_equals(e.currentState,"closed",'Aftertoggle should be fired after the popover is closed'); + assert_true(e.target.matches(':closed'),'The popover should be in the :closed state when the after hiding event fires.'); + assert_false(e.target.matches(':open'),'The popover should *not* be in the :open state when the after hiding event fires.'); + ++afterHideCount; + } + e.preventDefault(); // "aftertoggle" should not be cancelable. } }; switch (method) { @@ -36,34 +57,52 @@ const controller = new AbortController(); const signal = controller.signal; t.add_cleanup(() => controller.abort()); - // The 'beforetoggle' event bubbles. + // These events bubble. document.addEventListener('beforetoggle', listener, {signal}); + document.addEventListener('aftertoggle', listener, {signal}); break; case "attribute": assert_false(popover.hasAttribute('onbeforetoggle')); t.add_cleanup(() => popover.removeAttribute('onbeforetoggle')); popover.onbeforetoggle = listener; + assert_false(popover.hasAttribute('onaftertoggle')); + t.add_cleanup(() => popover.removeAttribute('onaftertoggle')); + popover.onaftertoggle = listener; break; default: assert_unreached(); } assert_equals(0,showCount); assert_equals(0,hideCount); + assert_equals(0,afterShowCount); + assert_equals(0,afterHideCount); popover.showPopover(); assert_true(popover.matches(':open')); assert_equals(1,showCount); assert_equals(0,hideCount); + assert_equals(0,afterShowCount); + assert_equals(0,afterHideCount); await waitForRender(); + assert_equals(1,afterShowCount,'aftertoggle show is fired asynchronously'); + assert_equals(0,afterHideCount); assert_true(popover.matches(':open')); popover.hidePopover(); assert_false(popover.matches(':open')); assert_equals(1,showCount); assert_equals(1,hideCount); + assert_equals(1,afterShowCount); + assert_equals(0,afterHideCount); + await waitForRender(); + assert_equals(1,afterShowCount); + assert_equals(1,afterHideCount,'aftertoggle hide is fired asynchronously'); + // No additional events + await waitForRender(); await waitForRender(); - // No additional events after animation frame assert_false(popover.matches(':open')); assert_equals(1,showCount); assert_equals(1,hideCount); - }, `Beforetoggle event (${method}) get properly dispatched for popovers`); + assert_equals(1,afterShowCount); + assert_equals(1,afterHideCount); + }, `The "beforetoggle" event (${method}) get properly dispatched for popovers`); } promise_test(async t => { @@ -86,7 +125,7 @@ assert_true(popover.matches(':open')); popover.hidePopover(); assert_false(popover.matches(':open')); - }, 'Beforetoggle event is cancelable for the "opening" transition'); + }, 'The "beforetoggle" event is cancelable for the "opening" transition'); promise_test(async t => { const popover = document.querySelector('[popover]'); @@ -104,6 +143,6 @@ await waitForRender(); // Check for async events also await waitForRender(); // Check for async events also assert_false(popover.matches(':open')); - }, 'Beforetoggle event is not fired for element removal'); + }, 'The "beforetoggle" event is not fired for element removal'); }; diff --git a/html/semantics/popovers/toggleevent-interface.tentative.html b/html/semantics/popovers/toggleevent-interface.tentative.html index 8ee63c4071856c..4d437b0c0ad523 100644 --- a/html/semantics/popovers/toggleevent-interface.tentative.html +++ b/html/semantics/popovers/toggleevent-interface.tentative.html @@ -7,200 +7,200 @@