From 3dac015f3cd2d0c3deb2eb72849cca9dd3ba559c Mon Sep 17 00:00:00 2001 From: Mason Freed Date: Wed, 6 Jul 2022 09:22:01 -0700 Subject: [PATCH] Update the pop-up implementation to match hint/auto resolutions See the set of behaviors described here: https://github.com/openui/open-ui/issues/525#issuecomment-1119093412 which were resolved here: https://github.com/openui/open-ui/issues/525#issuecomment-1125293226 This CL implements those changes in behavior, which mostly deal with how popup=auto and popup=hint interact, and some small changes to how `defaultopen` works. Bug: 1307772 Change-Id: I4d280b60e7c341b4d0f97fe82e60134ff4a6e1fa --- .../popup-attribute-basic.tentative.html | 37 ++- .../popup-defaultopen-hints.tentative.html | 20 ++ .../popups/popup-defaultopen.tentative.html | 2 - .../popups/popup-focus.tentative.html | 25 +- .../popups/popup-light-dismiss.tentative.html | 28 +- .../popups/popup-types.tentative.html | 248 ++++++++++++------ .../semantics/popups/resources/popup-utils.js | 4 + 7 files changed, 256 insertions(+), 108 deletions(-) create mode 100644 html/semantics/popups/popup-defaultopen-hints.tentative.html diff --git a/html/semantics/popups/popup-attribute-basic.tentative.html b/html/semantics/popups/popup-attribute-basic.tentative.html index 145651173cf597d..d6090486aa27653 100644 --- a/html/semantics/popups/popup-attribute-basic.tentative.html +++ b/html/semantics/popups/popup-attribute-basic.tentative.html @@ -20,6 +20,17 @@
Not a pop-up
+
Animated pop-up
+ + diff --git a/html/semantics/popups/popup-defaultopen-hints.tentative.html b/html/semantics/popups/popup-defaultopen-hints.tentative.html new file mode 100644 index 000000000000000..9bc841eb772610a --- /dev/null +++ b/html/semantics/popups/popup-defaultopen-hints.tentative.html @@ -0,0 +1,20 @@ + + + + + + + + +
This is a popup=hint with defaultopen, which should NOT be open upon load
+ + diff --git a/html/semantics/popups/popup-defaultopen.tentative.html b/html/semantics/popups/popup-defaultopen.tentative.html index 7a82214623461ce..f673d1d832081b9 100644 --- a/html/semantics/popups/popup-defaultopen.tentative.html +++ b/html/semantics/popups/popup-defaultopen.tentative.html @@ -8,7 +8,6 @@
This is a popup, which should be open upon load
This is a second popup with defaultopen, which should NOT be open upon load
-
This is a hint popup with defaultopen, which should NOT be open upon load
Also not visible
This is a manual popup with defaultopen, which should be open upon load
@@ -25,7 +24,6 @@ assert_true(p2.hasAttribute('defaultopen'),'defaultopen should be present/true, even if not opened'); assert_true(p2.defaultOpen,'defaultopen should be present/true, even if not opened'); - assert_false(p2b.matches(':top-layer'),'Only the first popup/hint with defaultopen should be open on load'); assert_true(p4.matches(':top-layer'),'defaultopen should open all manual popups'); assert_true(p5.matches(':top-layer'),'defaultopen should open all manual popups'); diff --git a/html/semantics/popups/popup-focus.tentative.html b/html/semantics/popups/popup-focus.tentative.html index f4fea330b65aa14..0ab90bbf55457a9 100644 --- a/html/semantics/popups/popup-focus.tentative.html +++ b/html/semantics/popups/popup-focus.tentative.html @@ -132,10 +132,9 @@ t.add_cleanup(() => priorFocus.remove()); return priorFocus; } - async function finishAnimations(popUp) { - popUp.getAnimations({subtree: true}).forEach(animation => animation.finish()); - await waitForRender(); - assert_false(isElementVisible(popUp)); + async function finishAnimationsAndVerifyHide(popUp) { + await finishAnimations(popUp); + assert_false(isElementVisible(popUp),'After animations are finished, the pop-up should be hidden'); } function activateAndVerify(popUp) { const testName = popUp.getAttribute('data-test'); @@ -155,7 +154,7 @@ assert_equals(document.activeElement, expectedFocusedElement, `${testName} activated by popUp.showPopUp()`); popUp.hidePopUp(); assert_equals(document.activeElement, priorFocus, 'prior element should get focus on hide'); - await finishAnimations(popUp); + await finishAnimationsAndVerifyHide(popUp); // Hit Escape: priorFocus.focus(); @@ -164,7 +163,7 @@ assert_equals(document.activeElement, expectedFocusedElement, `${testName} activated by popUp.showPopUp()`); await sendEscape(); assert_equals(document.activeElement, priorFocus, 'prior element should get focus after Escape'); - await finishAnimations(popUp); + await finishAnimationsAndVerifyHide(popUp); // Change the pop-up type: priorFocus.focus(); @@ -174,7 +173,7 @@ popUp.popUp = 'hint'; assert_false(popUp.matches(':top-layer'), 'Changing the pop-up type should hide the pop-up'); assert_equals(document.activeElement, priorFocus, 'prior element should get focus when the type is changed'); - await finishAnimations(popUp); + await finishAnimationsAndVerifyHide(popUp); popUp.popUp = 'auto'; // Remove from the document: @@ -212,7 +211,7 @@ button.click(); // Button is set to toggle the pop-up assert_false(popUp.matches(':top-layer')); assert_equals(document.activeElement, priorFocus, 'prior element should get focus on button-toggled hide'); - await finishAnimations(popUp); + await finishAnimationsAndVerifyHide(popUp); }, "Popup focus test: " + testName); promise_test(async t => { @@ -229,7 +228,7 @@ await clickOn(button); // This will not light dismiss, but will "toggle" the popUp. assert_false(popUp.matches(':top-layer')); assert_equals(document.activeElement, priorFocus, 'Focus should return to prior focus'); - await finishAnimations(popUp); + await finishAnimationsAndVerifyHide(popUp); // Same thing, but the button is contained within the pop-up button.removeAttribute('togglepopup'); @@ -244,7 +243,7 @@ await clickOn(button); assert_false(popUp.matches(':top-layer'), 'clicking button should hide the pop-up'); assert_equals(document.activeElement, priorFocus, 'Contained button should return focus to the previously focused element'); - await finishAnimations(popUp); + await finishAnimationsAndVerifyHide(popUp); // Same thing, but the button is unrelated (no togglepopup) button = document.createElement('button'); @@ -255,7 +254,7 @@ await clickOn(button); // This will light dismiss the pop-up, focus the prior focus, then focus this button. assert_false(popUp.matches(':top-layer'), 'clicking button should hide the pop-up (via light dismiss)'); assert_equals(document.activeElement, button, 'Focus should go to unrelated button on light dismiss'); - await finishAnimations(popUp); + await finishAnimationsAndVerifyHide(popUp); }, "Popup button click focus test: " + testName); promise_test(async t => { @@ -276,7 +275,7 @@ assert_equals(document.activeElement, newFocus, 'focus should not change when prior focus is removed'); popUp.hidePopUp(); assert_not_equals(document.activeElement, priorFocus, 'focused element has been removed'); - await finishAnimations(popUp); + await finishAnimationsAndVerifyHide(popUp); document.body.appendChild(priorFocus); // Put it back // Move the prior focus inside the (already open) pop-up @@ -292,7 +291,7 @@ assert_true(isElementVisible(popUp),'Animations should keep the pop-up visible'); assert_not_equals(getComputedStyle(popUp).display,'none','Animations should keep the pop-up visible'); assert_equals(document.activeElement, priorFocus, 'focused element gets focused'); - await finishAnimations(popUp); + await finishAnimationsAndVerifyHide(popUp); assert_equals(getComputedStyle(popUp).display,'none','Animations have ended, pop-up should be hidden'); assert_not_equals(document.activeElement, priorFocus, 'focused element is display:none inside the pop-up'); document.body.appendChild(priorFocus); // Put it back diff --git a/html/semantics/popups/popup-light-dismiss.tentative.html b/html/semantics/popups/popup-light-dismiss.tentative.html index fc900439f8319f0..99a48d7230e33a2 100644 --- a/html/semantics/popups/popup-light-dismiss.tentative.html +++ b/html/semantics/popups/popup-light-dismiss.tentative.html @@ -425,37 +425,37 @@ diff --git a/html/semantics/popups/popup-types.tentative.html b/html/semantics/popups/popup-types.tentative.html index 2b82fcd7542d573..b34df5f79605660 100644 --- a/html/semantics/popups/popup-types.tentative.html +++ b/html/semantics/popups/popup-types.tentative.html @@ -5,84 +5,180 @@ - -
Hint
-
Async
-
Async
+
+
Popup
+
Hint
+
Async
+
Async
+ +
+ +
+
Popup 1 +
Popup 2 +

Anchor

+
Popup 3
+
+
+
Hint anchored to pop-up
+ +
-test(() => { - assert_state_1(false,false,false,false); - hint.showPopUp(); - manual.showPopUp(); - manual2.showPopUp(); - assert_state_1(false,true,true,true); - popup.showPopUp(); - assert_state_1(true,false,true,true); - popup.hidePopUp(); - assert_state_1(false,false,true,true); - manual.hidePopUp(); - manual2.hidePopUp(); - assert_state_1(false,false,false,false); -},'popups close hints but not manuals'); - +
+
Hint +
Nested hint
+
+ +
-
Popup 1 -
Popup 2 -

Anchor

-
Popup 3
+
+
Hint +
Nested auto (note - never visible, since inside display:none subtree)
+ +
+ +
+
Auto +
Nested Auto
+
Nested hint
+
+ +
+ +
+
Auto
+
Non-Nested hint
+
-
Hint anchored to popup
- diff --git a/html/semantics/popups/resources/popup-utils.js b/html/semantics/popups/resources/popup-utils.js index bbb628b40d26501..886a2302cad0ecc 100644 --- a/html/semantics/popups/resources/popup-utils.js +++ b/html/semantics/popups/resources/popup-utils.js @@ -23,3 +23,7 @@ async function sendEscape() { function isElementVisible(el) { return !!(el.offsetWidth || el.offsetHeight || el.getClientRects().length); } +async function finishAnimations(popUp) { + popUp.getAnimations({subtree: true}).forEach(animation => animation.finish()); + await waitForRender(); +}