-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Update popover=hint behavior to allow a stack of hints
The previous implementation only allowed one popover=hint to be open at a time. Per conversation at [1], developers feel that it should be possible to nest popover=hint popovers. This CL implements that capability. There are now two popover stacks in Document: PopoverAutoStack and PopoverHintStack. Since it is possible to nest hints within autos, the PopoverAutoStack can contain hints. However, there are a few constraints: - The PopoverHintStack only ever contains hints. - Once the PopoverAutoStack contains a hint, all subsequent popovers in the stack must also be hints. - A popover=hint can never be the ancestor of a popover=auto. The light dismiss behavior is roughly the same as before, with a slight tweak that simplifies behavior: closing anything in the PopoverAutoStack will always close everything in the PopoverHintStack. That was not the case before, but it was a bit of a weird corner case. Note that I found a crasher (happens in stable, with just auto popovers) that I added a test for here. The bug for that is crbug.com/1513282. I'll fix that in a followup. [1] whatwg/html#9776 (comment) Bug: 1416284,1513282 Change-Id: Ic064ecf1377bb8abfc812654c85016e6d1cbbdaf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5133909 Reviewed-by: Joey Arhar <[email protected]> Commit-Queue: Mason Freed <[email protected]> Auto-Submit: Mason Freed <[email protected]> Cr-Commit-Position: refs/heads/main@{#1246573}
- Loading branch information
1 parent
b22db43
commit 51c87dc
Showing
4 changed files
with
195 additions
and
119 deletions.
There are no files selected for viewing
107 changes: 107 additions & 0 deletions
107
html/semantics/popovers/popover-light-dismiss-hint.tentative.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| <!DOCTYPE html> | ||
| <meta charset="utf-8" /> | ||
| <title>Popover light dismiss behavior for hints</title> | ||
| <meta name="timeout" content="long"> | ||
| <link rel="author" href="mailto:[email protected]"> | ||
| <link rel=help href="https://open-ui.org/components/popover-hint.research.explainer"> | ||
| <script src="/resources/testharness.js"></script> | ||
| <script src="/resources/testharnessreport.js"></script> | ||
| <script src="/resources/testdriver.js"></script> | ||
| <script src="/resources/testdriver-actions.js"></script> | ||
| <script src="/resources/testdriver-vendor.js"></script> | ||
| <script src="resources/popover-utils.js"></script> | ||
|
|
||
| <div id=outside></div> | ||
| <div popover id=auto1>auto 1 | ||
| <div popover id=auto2>auto 2 | ||
| <div popover=hint id=innerhint1>inner hint 1 | ||
| <div popover=hint id=innerhint2>inner hint 2 | ||
| <div popover id=invalidauto1>Improperly nested auto 1</div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <div popover=hint id=hint1>hint 1 | ||
| <div popover=hint id=hint2>hint 2 | ||
| <div popover id=invalidauto2>Improperly nested auto 2</div> | ||
| </div> | ||
| </div> | ||
| <div popover=manual id=manual1>Manual</div> | ||
|
|
||
| <style> | ||
| [popover] {right:auto;bottom:auto;} | ||
| #auto1 {left:100px; top:100px;} | ||
| #auto2 {left:100px; top:200px;} | ||
| #innerhint1 {left:100px; top:300px;} | ||
| #innerhint2 {left:100px; top:400px;} | ||
| #invalidauto1 {left:100px; top:500px;} | ||
| #hint1 {left:200px; top:100px;} | ||
| #hint2 {left:200px; top:200px;} | ||
| #invalidauto1 {left:200px; top:400px;} | ||
| #manual1 {left:300px; top:100px;} | ||
| #outside {width:25px;height:25px} | ||
| </style> | ||
|
|
||
| <script> | ||
| const popovers = [ | ||
| document.querySelector('#auto1'), | ||
| document.querySelector('#auto2'), | ||
| document.querySelector('#innerhint1'), | ||
| document.querySelector('#innerhint2'), | ||
| document.querySelector('#hint1'), | ||
| document.querySelector('#hint2'), | ||
| document.querySelector('#manual1'), | ||
| ]; | ||
| function assertState(expectedState,description) { | ||
| description = description || 'Error'; | ||
| const n = popovers.length; | ||
| assert_equals(expectedState.length,n,'Invalid'); | ||
| for(let i=0;i<n;++i) { | ||
| assert_equals(popovers[i].matches(':popover-open'),expectedState[i],`${description}, index ${i} (${popovers[i].id})`); | ||
| } | ||
| } | ||
| function openall(t) { | ||
| // All popovers can be open at once, if shown in order: | ||
| popovers.forEach((p) => p.hidePopover()); | ||
| popovers.forEach((p) => p.showPopover()); | ||
| assertState(Array(popovers.length).fill(true),'All popovers should be able to be open at once'); | ||
| t.add_cleanup(() => popovers.forEach((p) => p.hidePopover())); | ||
| } | ||
| function nvals(n,val) { | ||
| return new Array(n).fill(val); | ||
| } | ||
| for(let i=0;i<(popovers.length-1);++i) { | ||
| promise_test(async (t) => { | ||
| openall(t); | ||
| await clickOn(popovers[i]); | ||
| let expectedState = [...nvals(i+1,true),...nvals(popovers.length-i-2,false),true]; | ||
| assertState(expectedState); | ||
| },`Mixed auto/hint light dismiss behavior, click on ${popovers[i].id}`); | ||
| } | ||
|
|
||
| promise_test(async (t) => { | ||
| openall(t); | ||
| await clickOn(outside); | ||
| assertState([false,false,false,false,false,false,true]); | ||
| },'Clicking outside closes all'); | ||
|
|
||
| promise_test(async (t) => { | ||
| openall(t); | ||
| invalidauto1.showPopover(); | ||
| assertState([true,true,false,false,false,false,true],'auto inside hint ignores the hints and gets nested within auto2'); | ||
| assert_true(invalidauto1.matches(':popover-open'),'the inner nested auto should be open'); | ||
| invalidauto1.hidePopover(); | ||
| assertState([true,true,false,false,false,false,true]); | ||
| assert_false(invalidauto1.matches(':popover-open')); | ||
| },'Auto cannot be nested inside hint (invalidauto1)'); | ||
|
|
||
| promise_test(async (t) => { | ||
| openall(t); | ||
| invalidauto2.showPopover(); | ||
| assertState([false,false,false,false,false,false,true],'auto inside hint works as an independent (non-nested) auto'); | ||
| assert_true(invalidauto2.matches(':popover-open'),'the inner nested auto should be open'); | ||
| invalidauto2.hidePopover(); | ||
| assertState([false,false,false,false,false,false,true]); | ||
| assert_false(invalidauto2.matches(':popover-open')); | ||
| },'Auto cannot be nested inside hint (invalidauto2)'); | ||
| </script> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| <!DOCTYPE html> | ||
| <meta charset="utf-8" /> | ||
| <link rel="author" href="mailto:[email protected]"> | ||
| <link rel=help href="https://html.spec.whatwg.org/multipage/popover.html#attr-popover"> | ||
|
|
||
| <div popover id=p1>Popover 1 | ||
| <div popover id=p2>Popover 2</div> | ||
| </div> | ||
| <script> | ||
| const p1 = document.querySelector('#p1'); | ||
| const p2 = document.querySelector('#p2'); | ||
| p1.addEventListener('beforetoggle',e => { | ||
| if (e.newState === "closed") { | ||
| p2.showPopover(); | ||
| } | ||
| }) | ||
| p1.showPopover(); | ||
| p1.hidePopover(); | ||
| // This test passes if it does not crash | ||
| </script> |
Oops, something went wrong.