From 9f5066f9a484a9eb42b9fc3f27233721ead6a250 Mon Sep 17 00:00:00 2001
From: Mason Freed <masonf@chromium.org>
Date: Mon, 28 Nov 2022 09:25:31 -0800
Subject: [PATCH] Combine the popoverhide/popovershow events into one toggle
 event

See [1] for context. This eliminates the "popoverhide" and
"popovershow" events, and re-uses the "toggle" event for both
of these transitions. The event in this case is a ToggleEvent,
which has a `state` that is either "opening" or "closing".

[1] https://github.com/openui/open-ui/issues/607#issuecomment-1319114654

Bug: 1307772
Change-Id: I404861a10579365b56e6e8eae7c29f88a5aac166
---
 ...over-animation-corner-cases.tentative.html |  22 ++--
 .../popover-attribute-basic.tentative.html    |  18 ++-
 .../popovers/popover-events.tentative.html    |  40 +++---
 .../popover-invoking-attribute.tentative.html |   8 +-
 ...ver-light-dismiss-on-scroll.tentative.html |   3 +-
 .../popover-light-dismiss.tentative.html      |  13 +-
 ...pover-toggle-opening-event.tentative.html} |   5 +-
 .../toggleevent-interface.tentative.html      | 121 ++++++++++++++++++
 8 files changed, 182 insertions(+), 48 deletions(-)
 rename html/semantics/popovers/{popover-show-event.tentative.html => popover-toggle-opening-event.tentative.html} (83%)
 create mode 100644 html/semantics/popovers/toggleevent-interface.tentative.html

diff --git a/html/semantics/popovers/popover-animation-corner-cases.tentative.html b/html/semantics/popovers/popover-animation-corner-cases.tentative.html
index a61917ca8b8f121..3f462187ef2acb1 100644
--- a/html/semantics/popovers/popover-animation-corner-cases.tentative.html
+++ b/html/semantics/popovers/popover-animation-corner-cases.tentative.html
@@ -100,7 +100,8 @@
   assert_true(isElementVisible(popover));
   assert_equals(popover.getAnimations({subtree: true}).length,0);
   let animation;
-  popover.addEventListener('popoverhide', () => {
+  popover.addEventListener('toggle', (e) => {
+    if (e.state !== "closing") return;
     animation = popover.animate([{opacity: 1},{opacity: 0}],1000000);
   });
   assert_equals(popover.getAnimations({subtree: true}).length,0,'There should be no animations yet');
@@ -111,7 +112,7 @@
   animation.finish();
   await waitForRender();
   assert_false(isElementVisible(popover),'Once the animation ends, the popover is hidden');
-},'It should be possible to use the "popoverhide" event handler to animate the hide');
+},'It should be possible to use the "toggle" event handler to animate the hide');
 
 
 promise_test(async (t) => {
@@ -121,7 +122,8 @@
   popover.showPopover();
   assert_true(isElementVisible(popover));
   assert_equals(popover.getAnimations({subtree: true}).length,0);
-  popover.addEventListener('popoverhide', () => {
+  popover.addEventListener('toggle', (e) => {
+    if (e.state !== "closing") return;
     popover.animate([{opacity: 1},{opacity: 0}],1000000);
   });
   assert_equals(popover.getAnimations({subtree: true}).length,0,'There should be no animations yet');
@@ -129,19 +131,17 @@
   await waitForRender();
   assert_equals(popover.getAnimations({subtree: true}).length,1,'the hide animation should now be running');
   assert_false(isElementVisible(popover),'But the animation should *not* keep the popover visible in this case');
-},'It should *not* be possible to use the "popoverhide" event handler to animate the hide, if the hide is due to dialog.showModal');
+},'It should *not* be possible to use the "toggle" event handler to animate the hide, if the hide is due to dialog.showModal');
 
 promise_test(async (t) => {
   const {popover, descendent} = createPopover(t,'');
   popover.showPopover();
   assert_true(isElementVisible(popover));
-  popover.addEventListener('popoverhide', (e) => {
-    e.preventDefault();
-  });
+  popover.addEventListener('toggle', (e) => e.preventDefault());
   popover.hidePopover();
   await waitForRender();
   assert_false(isElementVisible(popover),'Even if hide event is cancelled, the popover still closes');
-},'hide event cannot be cancelled');
+},'toggle event cannot be cancelled');
 
 promise_test(async (t) => {
   const {popover, descendent} = createPopover(t,'animation');
@@ -169,7 +169,8 @@
   popover.showPopover();
   assert_true(isElementVisible(popover));
   assert_equals(popover.getAnimations({subtree: true}).length,0);
-  popover.addEventListener('popoverhide', () => {
+  popover.addEventListener('toggle', (e) => {
+    if (e.state !== "closing") return;
     popover.animate([{opacity: 1},{opacity: 0}],1000000);
   });
   assert_equals(popover.getAnimations({subtree: true}).length,0,'There should be no animations yet');
@@ -193,7 +194,8 @@
 promise_test(async (t) => {
   const {popover, descendent} = createPopover(t,'');
   popover.showPopover();
-  popover.addEventListener('popoverhide', () => {
+  popover.addEventListener('toggle', (e) => {
+    if (e.state !== "closing") return;
     popover.animate([{opacity: 1},{opacity: 0}],1000000);
   });
   popover.hidePopover();
diff --git a/html/semantics/popovers/popover-attribute-basic.tentative.html b/html/semantics/popovers/popover-attribute-basic.tentative.html
index 23aaf75d71d28ee..9422f682abc48b8 100644
--- a/html/semantics/popovers/popover-attribute-basic.tentative.html
+++ b/html/semantics/popovers/popover-attribute-basic.tentative.html
@@ -275,7 +275,8 @@
     other_popover.showPopover();
     const popover = createPopover(t);
     popover.setAttribute('popover','auto');
-    other_popover.addEventListener('popoverhide',() => {
+    other_popover.addEventListener('toggle', (e) => {
+      if (e.state !== "closing") return;
       popover.setAttribute('popover','manual');
     },{once: true});
     assert_true(other_popover.matches(':open'));
@@ -283,7 +284,7 @@
     popover.showPopover();
     assert_false(other_popover.matches(':open'),'unrelated popover is hidden');
     assert_false(popover.matches(':open'),'popover is not shown if its type changed during show');
-  },`Changing the popover type in a "popoverhide" event handler should not cause problems (during showPopover())`);
+  },`Changing the popover type in a "toggle" event handler should not cause problems (during showPopover())`);
 
   test((t) => {
     const popover = createPopover(t);
@@ -294,11 +295,13 @@
     popover.showPopover();
     other_popover.showPopover();
     let nested_popover_hidden=false;
-    other_popover.addEventListener('popoverhide',() => {
+    other_popover.addEventListener('toggle', (e) => {
+      if (e.state !== "closing") return;
       nested_popover_hidden = true;
       popover.setAttribute('popover','manual');
     },{once: true});
-    popover.addEventListener('popoverhide',() => {
+    popover.addEventListener('toggle', (e) => {
+      if (e.state !== "closing") return;
       assert_true(nested_popover_hidden,'The nested popover should be hidden first');
     },{once: true});
     assert_true(popover.matches(':open'));
@@ -307,7 +310,7 @@
     assert_false(other_popover.matches(':open'),'unrelated popover is hidden');
     assert_false(popover.matches(':open'),'popover is still hidden if its type changed during hide event');
     assert_throws_dom("InvalidStateError",() => other_popover.hidePopover(),'Nested popover should already be hidden');
-  },`Changing the popover type in a "popoverhide" event handler should not cause problems (during hidePopover())`);
+  },`Changing the popover type in a "toggle" event handler should not cause problems (during hidePopover())`);
 
   function interpretedType(typeString,method) {
     if (validTypes.includes(typeString))
@@ -344,7 +347,8 @@
             popover.showPopover();
             assert_true(popover.matches(':open'));
             let gotEvent = false;
-            popover.addEventListener('popoverhide',() => {
+            popover.addEventListener('toggle', (e) => {
+              if (e.state !== "closing") return;
               gotEvent = true;
               setPopoverValue(popover,inEventType,method);
             },{once:true});
@@ -380,7 +384,7 @@
                 }
               }
             }
-          },`Changing a popover from ${type} to ${newType} (via ${method}), and then ${inEventType} during 'popoverhide' works`);
+          },`Changing a popover from ${type} to ${newType} (via ${method}), and then ${inEventType} during 'toggle' works`);
         });
       });
     });
diff --git a/html/semantics/popovers/popover-events.tentative.html b/html/semantics/popovers/popover-events.tentative.html
index 78d4a22c78e7be4..a9562432607ce4e 100644
--- a/html/semantics/popovers/popover-events.tentative.html
+++ b/html/semantics/popovers/popover-events.tentative.html
@@ -17,31 +17,28 @@
       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;
+      function listener(e) {
+        if (e.state === "opening") {
+          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_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;
+        }
       };
       switch (method) {
         case "listener":
           const controller = new AbortController();
           const signal = controller.signal;
           t.add_cleanup(() => controller.abort());
-          document.addEventListener('popovershow',showListener, {signal});
-          document.addEventListener('popoverhide',hideListener, {signal});
+          document.addEventListener('toggle', listener, {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 = showListener;
-          popover.onpopoverhide = hideListener;
+          assert_false(popover.hasAttribute('ontoggle'));
+          t.add_cleanup(() => popover.removeAttribute('ontoggle'));
+          popover.ontoggle = listener;
           break;
         default: assert_unreached();
       }
@@ -62,7 +59,7 @@
       assert_false(popover.matches(':open'));
       assert_equals(1,showCount);
       assert_equals(1,hideCount);
-    }, `Popovershow and popoverhide events (${method}) get properly dispatched for popovers`);
+    }, `Toggle event (${method}) get properly dispatched for popovers`);
   }
 
   promise_test(async t => {
@@ -71,18 +68,19 @@
     const signal = controller.signal;
     t.add_cleanup(() => controller.abort());
     let cancel = true;
-    popover.addEventListener('popovershow',(e) => {
+    popover.addEventListener('toggle',(e) => {
+      if (e.state !== "opening") return;
       if (cancel)
         e.preventDefault();
     }, {signal});
     assert_false(popover.matches(':open'));
     popover.showPopover();
-    assert_false(popover.matches(':open'),'The "popovershow" event should be cancelable');
+    assert_false(popover.matches(':open'),'The "toggle" event should be cancelable for the "opening" transition');
     cancel = false;
     popover.showPopover();
     assert_true(popover.matches(':open'));
     popover.hidePopover();
     assert_false(popover.matches(':open'));
-  }, 'Popovershow event is cancelable');
+  }, 'Toggle event is cancelable for the "opening" transition');
 };
 </script>
diff --git a/html/semantics/popovers/popover-invoking-attribute.tentative.html b/html/semantics/popovers/popover-invoking-attribute.tentative.html
index affe201b69a4a21..6ddf9459e8543b9 100644
--- a/html/semantics/popovers/popover-invoking-attribute.tentative.html
+++ b/html/semantics/popovers/popover-invoking-attribute.tentative.html
@@ -175,8 +175,12 @@
 const button = document.querySelector('button');
 let showCount = 0;
 let hideCount = 0;
-popover.addEventListener('popovershow',() => ++showCount);
-popover.addEventListener('popoverhide',() => ++hideCount);
+popover.addEventListener('toggle',(e) => {
+  if (e.state === "opening")
+      ++showCount;
+    else
+      ++hideCount;
+  });
 
 async function assertState(expectOpen,expectShow,expectHide) {
   assert_equals(popover.matches(':open'),expectOpen,'Popover open state is incorrect');
diff --git a/html/semantics/popovers/popover-light-dismiss-on-scroll.tentative.html b/html/semantics/popovers/popover-light-dismiss-on-scroll.tentative.html
index 7a09712d8149a6f..54edce452a12a9d 100644
--- a/html/semantics/popovers/popover-light-dismiss-on-scroll.tentative.html
+++ b/html/semantics/popovers/popover-light-dismiss-on-scroll.tentative.html
@@ -44,7 +44,8 @@
   }
   async_test(t => {
     for(let popover of popovers) {
-      popover.addEventListener('popoverhide',e => {
+      popover.addEventListener('toggle',e => {
+        if (e.state !== "closing") return;
         assert_unreached('Scrolling should not light-dismiss a popover');
       });
     }
diff --git a/html/semantics/popovers/popover-light-dismiss.tentative.html b/html/semantics/popovers/popover-light-dismiss.tentative.html
index 2a101483cb8c8e1..ddc10971e7f37ba 100644
--- a/html/semantics/popovers/popover-light-dismiss.tentative.html
+++ b/html/semantics/popovers/popover-light-dismiss.tentative.html
@@ -47,14 +47,16 @@
   const afterp1 = document.querySelector('#after_p1');
 
   let popover1HideCount = 0;
-  popover1.addEventListener('popoverhide',(e) => {
+  popover1.addEventListener('toggle',(e) => {
+    if (e.state !== "closing") return;
     ++popover1HideCount;
-    e.preventDefault(); // 'popoverhide' should not be cancellable.
+    e.preventDefault(); // 'toggle' should not be cancellable.
   });
   let popover2HideCount = 0;
-  popover2.addEventListener('popoverhide',(e) => {
+  popover2.addEventListener('toggle',(e) => {
+    if (e.state !== "closing") return;
     ++popover2HideCount;
-    e.preventDefault(); // 'popoverhide' should not be cancellable.
+    e.preventDefault(); // 'toggle' should not be cancellable.
   });
   promise_test(async () => {
     assert_false(popover1.matches(':open'));
@@ -482,7 +484,8 @@
   p13.showPopover();
   p14.showPopover();
   p15.showPopover();
-  p15.addEventListener('popoverhide',() => {
+  p15.addEventListener('toggle', (e) => {
+    if (e.state !== "closing") return;
     p14.hidePopover();
   },{once:true});
   assert_true(p13.matches(':open') && p14.matches(':open') && p15.matches(':open'),'all three should be open');
diff --git a/html/semantics/popovers/popover-show-event.tentative.html b/html/semantics/popovers/popover-toggle-opening-event.tentative.html
similarity index 83%
rename from html/semantics/popovers/popover-show-event.tentative.html
rename to html/semantics/popovers/popover-toggle-opening-event.tentative.html
index 69996bd65daba30..8d2728139e12e72 100644
--- a/html/semantics/popovers/popover-show-event.tentative.html
+++ b/html/semantics/popovers/popover-toggle-opening-event.tentative.html
@@ -14,7 +14,8 @@
   requestAnimationFrame(() => {++frameCount;});
   const popover = document.querySelector('[popover]');
   const testText = 'Show Event Occurred';
-  popover.addEventListener('popovershow',() => {
+  popover.addEventListener('toggle',(e) => {
+    if (e.state !== "opening") return;
     popover.textContent = testText;
   })
   popover.offsetHeight;
@@ -25,5 +26,5 @@
   assert_equals(popover.textContent,testText);
   assert_equals(frameCount,0,'nothing should be rendered before the popover is updated');
   popover.hidePopover(); // Cleanup
-},'Ensure the `show` event can be used to populate content before the popover renders');
+},'Ensure the `toggle` event can be used to populate content before the popover renders');
 </script>
diff --git a/html/semantics/popovers/toggleevent-interface.tentative.html b/html/semantics/popovers/toggleevent-interface.tentative.html
new file mode 100644
index 000000000000000..8e0d019c60d03db
--- /dev/null
+++ b/html/semantics/popovers/toggleevent-interface.tentative.html
@@ -0,0 +1,121 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="author" href="mailto:masonf@chromium.org">
+<link rel=help href="https://open-ui.org/components/popup.research.explainer">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+test(function() {
+  var event = new ToggleEvent("");
+  assert_true(event instanceof window.ToggleEvent);
+}, "the event is an instance of ToggleEvent");
+
+test(function() {
+  var event = new ToggleEvent("");
+  assert_true(event instanceof window.Event);
+}, "the event inherts from Event");
+
+test(function() {
+  assert_throws_js(TypeError, function() {
+    new ToggleEvent();
+  }, 'First argument is required, so was expecting a TypeError.');
+}, 'Missing type argument');
+
+test(function() {
+  var event = new ToggleEvent("test");
+  assert_equals(event.type, "test");
+}, "type argument is string");
+
+test(function() {
+  var event = new ToggleEvent(null);
+  assert_equals(event.type, "null");
+}, "type argument is null");
+
+test(function() {
+  var event = new ToggleEvent(undefined);
+  assert_equals(event.type, "undefined");
+}, "event type set to undefined");
+
+test(function() {
+  var event = new ToggleEvent("test");
+  assert_equals(event.state, "");
+}, "state has default value of empty string");
+
+test(function() {
+  var event = new ToggleEvent("test");
+  assert_readonly(event, "state", "readonly attribute value");
+}, "state is readonly");
+
+test(function() {
+  var event = new ToggleEvent("test", null);
+  assert_equals(event.state, "");
+}, "toggleEventInit argument is null");
+
+test(function() {
+  var event = new ToggleEvent("test", undefined);
+  assert_equals(event.state, "");
+}, "toggleEventInit argument is undefined");
+
+test(function() {
+  var event = new ToggleEvent("test", {});
+  assert_equals(event.state, "");
+}, "toggleEventInit argument is empty dictionary");
+
+test(function() {
+  var event = new ToggleEvent("test", {state: "sample"});
+  assert_equals(event.state, "sample");
+}, "state set to 'sample'");
+
+test(function() {
+  var event = new ToggleEvent("test", {state: undefined});
+  assert_equals(event.state, "");
+}, "state set to undefined");
+
+test(function() {
+  var event = new ToggleEvent("test", {state: null});
+  assert_equals(event.state, "null");
+}, "state set to null");
+
+test(function() {
+  var event = new ToggleEvent("test", {state: false});
+  assert_equals(event.state, "false");
+}, "state set to false");
+
+test(function() {
+  var event = new ToggleEvent("test", {state: true});
+  assert_equals(event.state, "true");
+}, "state set to true");
+
+test(function() {
+  var event = new ToggleEvent("test", {state: 0.5});
+  assert_equals(event.state, "0.5");
+}, "state set to a number");
+
+test(function() {
+  var event = new ToggleEvent("test", {state: []});
+  assert_equals(event.state, "");
+}, "state set to []");
+
+test(function() {
+  var event = new ToggleEvent("test", {state: [1, 2, 3]});
+  assert_equals(event.state, "1,2,3");
+}, "state set to [1, 2, 3]");
+
+test(function() {
+  var event = new ToggleEvent("test", {state: {sample: 0.5}});
+  assert_equals(event.state, "[object Object]");
+}, "state set to an object");
+
+test(function() {
+  var event = new ToggleEvent("test",
+      {state: {valueOf: function () { return 'sample'; }}});
+  assert_equals(event.state, "[object Object]");
+}, "state set to an object with a valueOf function");
+
+test(function() {
+  var eventInit = {state: "sample"};
+  var event = new ToggleEvent("test", eventInit);
+  assert_equals(event.state, "sample");
+}, "ToggleEventInit properties set value");
+</script>