Skip to content

Commit

Permalink
Allow top layer elements to be nested within popovers
Browse files Browse the repository at this point in the history
See the issue raised at

  whatwg/html#9998

which was discussed at

  whatwg/html#9993

This CL makes the following changes:
 1. Change `FindTopmostPopoverAncestor()` so that the provided element
    does not have to be a popover. The logic does not materially
    change - all of the same mechanisms can be used to connect a
    non-popover top layer element (dialog or fullscreen) to an ancestor
    popover.
 2. Add a utility `TopLayerElementPopoverAncestor()` which finds the
    popover ancestor for a provided top layer element by calling
    `FindTopmostPopoverAncestor()` with the proper arguments.
 3. In dialog and fullscreen code, where it previously called
    `HideAllPopoversUntil(nullptr,...)` to hide **all** open popovers,
    it now (with flag enabled) hides only up to the nearest popover
    ancestor.

Tests are added, which are marked `.tentative`.

Bug: 1520938
Change-Id: I8d2c4000ed3959ac4e8bf521e22e9dfd532c62d5
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Jan 30, 2024
1 parent 49a46a3 commit dfc6263
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 0 deletions.
1 change: 1 addition & 0 deletions html/semantics/popovers/popover-attribute-basic.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<!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">
<link rel=help href="https://open-ui.org/components/popover.research.explainer">
<meta name="timeout" content="long">
<script src="/resources/testharness.js"></script>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<!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">
<link rel=help href="https://open-ui.org/components/popover.research.explainer">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/common/top-layer.js"></script>
<script src="resources/popover-utils.js"></script>
<script src="resources/popover-top-layer-nesting.js"></script>

<div id=tests>
<div> Single popover=auto ancestor
<div popover=auto class=target data-stay-open=true></div>
</div>

<div> Single popover=manual ancestor
<div popover=auto class=target data-stay-open=true></div>
</div>

<div> Nested popover=auto ancestors
<div popover=auto data-stay-open=true>
<div popover=auto class=target data-stay-open=true></div>
</div>
</div>

<div> Nested popover=auto ancestors, target is outer
<div popover=auto class=target data-stay-open=true>
<div popover=auto data-stay-open=false></div>
</div>
</div>

<div> Top layer inside of nested element
<div popover=auto data-stay-open=true>
<button class=target></button>
</div>
</div>
</div>

<script>
const tests = Array.from(document.querySelectorAll('#tests>div'));
runTopLayerTests(tests,/*testAnchorAttribute*/true);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<!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">
<link rel=help href="https://open-ui.org/components/popover.research.explainer">
<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-vendor.js"></script>
<script src="/common/top-layer.js"></script>
<script src="resources/popover-utils.js"></script>
<script src="resources/popover-top-layer-nesting.js"></script>

<div id=tests>
<div> Single popover=hint ancestor
<div popover=hint class=target data-stay-open=true></div>
</div>

<div> Nested auto/hint ancestors
<div popover=auto data-stay-open=true>
<div popover=hint class=target data-stay-open=true></div>
</div>
</div>

<div> Nested auto/hint ancestors, target is auto
<div popover=auto class=target data-stay-open=true>
<div popover=hint data-stay-open=false></div>
</div>
</div>

<div> Unrelated hint, target=hint
<div popover=auto data-stay-open=true></div>
<div popover=hint class=target data-stay-open=true></div>
</div>

<div> Unrelated hint, target=auto
<div popover=auto class=target data-stay-open=true></div>
<div popover=hint data-stay-open=false></div>
</div>
</div>

<script>
const tests = Array.from(document.querySelectorAll('#tests>div'));
runTopLayerTests(tests);
</script>
45 changes: 45 additions & 0 deletions html/semantics/popovers/popover-top-layer-nesting.tentative.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<!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">
<link rel=help href="https://open-ui.org/components/popover.research.explainer">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/common/top-layer.js"></script>
<script src="resources/popover-utils.js"></script>
<script src="resources/popover-top-layer-nesting.js"></script>

<div id=tests>
<div> Single popover=auto ancestor
<div popover=auto class=target data-stay-open=true></div>
</div>

<div> Single popover=manual ancestor
<div popover=auto class=target data-stay-open=true></div>
</div>

<div> Nested popover=auto ancestors
<div popover=auto data-stay-open=true>
<div popover=auto class=target data-stay-open=true></div>
</div>
</div>

<div> Nested popover=auto ancestors, target is outer
<div popover=auto class=target data-stay-open=true>
<div popover=auto data-stay-open=false></div>
</div>
</div>

<div> Top layer inside of nested element
<div popover=auto data-stay-open=true>
<button class=target></button>
</div>
</div>
</div>

<script>
const tests = Array.from(document.querySelectorAll('#tests>div'));
runTopLayerTests(tests);
</script>
108 changes: 108 additions & 0 deletions html/semantics/popovers/resources/popover-top-layer-nesting.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
function createTopLayerElement(t,topLayerType) {
let element, show, showing;
switch (topLayerType) {
case 'dialog':
element = document.createElement('dialog');
show = () => element.showModal();
showing = () => element.matches(':modal');
break;
case 'fullscreen':
element = document.createElement('div');
show = async (topmostElement) => {
// Be sure to add user activation to the topmost visible target:
await blessTopLayer(topmostElement);
await element.requestFullscreen();
};
showing = () => document.fullscreenElement === element;
break;
default:
assert_unreached('Invalid top layer type');
}
t.add_cleanup(() => element.remove());
return {element,show,showing};
}
function runTopLayerTests(testCases, testAnchorAttribute) {
testAnchorAttribute = testAnchorAttribute || false;
testCases.forEach(test => {
const description = test.firstChild.data.trim();
assert_equals(test.querySelectorAll('.target').length,1,'There should be exactly one target');
const target = test.querySelector('.target');
assert_true(!!target,'Invalid test case');
const popovers = Array.from(test.querySelectorAll('[popover]'));
assert_true(popovers.length > 0,'No popovers found');
['dialog','fullscreen'].forEach(topLayerType => {
promise_test(async t => {
const {element,show,showing} = createTopLayerElement(t,topLayerType);
target.appendChild(element);

// Show the popovers.
t.add_cleanup(() => popovers.forEach(popover => popover.hidePopover()));
popovers.forEach(popover => popover.showPopover());
popovers.forEach(popover => assert_true(popover.matches(':popover-open'),'All popovers should be open'));

// Activate the top layer element.
await show(popovers[popovers.length-1]);
assert_true(showing());
popovers.forEach(popover => assert_equals(popover.matches(':popover-open'),popover.dataset.stayOpen==='true','Incorrect behavior'));

// Add another popover within the top layer element and make sure entire stack stays open.
const newPopover = document.createElement('div');
t.add_cleanup(() => newPopover.remove());
newPopover.popover = popoverHintSupported() ? 'hint' : 'auto';
element.appendChild(newPopover);
popovers.forEach(popover => assert_equals(popover.matches(':popover-open'),popover.dataset.stayOpen==='true','Adding another popover shouldn\'t change anything'));
assert_true(showing(),'top layer element should still be top layer');
newPopover.showPopover();
assert_true(newPopover.matches(':popover-open'));
popovers.forEach(popover => assert_equals(popover.matches(':popover-open'),popover.dataset.stayOpen==='true','Showing the popover shouldn\'t change anything'));
assert_true(showing(),'top layer element should still be top layer');
},`${description} with ${topLayerType}`);

promise_test(async t => {
const {element,show,showing} = createTopLayerElement(t,topLayerType);
element.popover = popoverHintSupported() ? 'hint' : 'auto';
target.appendChild(element);

// Show the popovers.
t.add_cleanup(() => popovers.forEach(popover => popover.hidePopover()));
popovers.forEach(popover => popover.showPopover());
popovers.forEach(popover => assert_true(popover.matches(':popover-open'),'All popovers should be open'));
const targetWasOpenPopover = target.matches(':popover-open');

// Show the top layer element as a popover first.
element.showPopover();
assert_true(element.matches(':popover-open'),'element should be open as a popover');
assert_equals(target.matches(':popover-open'),targetWasOpenPopover,'target shouldn\'t change popover state');

try {
await show(element);
assert_unreached('It is an error to activate a top layer element that is already a showing popover');
} catch (e) {
// We expect an InvalidStateError for dialogs, and a TypeError for fullscreens.
// Anything else should fall through to the test harness.
if (e.name !== 'InvalidStateError' && e.name !== 'TypeError') {
throw e;
}
}
},`${description} with ${topLayerType}, top layer element *is* a popover`);

if (testAnchorAttribute) {
promise_test(async t => {
const {element,show,showing} = createTopLayerElement(t,topLayerType);
element.anchorElement = target;
document.body.appendChild(element);

// Show the popovers.
t.add_cleanup(() => popovers.forEach(popover => popover.hidePopover()));
popovers.forEach(popover => popover.showPopover());
popovers.forEach(popover => assert_true(popover.matches(':popover-open'),'All popovers should be open'));

// Activate the top layer element.
await show(popovers[popovers.length-1]);
assert_true(showing());
popovers.forEach(popover => assert_equals(popover.matches(':popover-open'),popover.dataset.stayOpen==='true','Incorrect behavior'));
},`${description} with ${topLayerType}, anchor attribute`);
}
});
});
}

0 comments on commit dfc6263

Please sign in to comment.