Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not close parent popover when shadowed nested popover is opened #190

Merged
merged 1 commit into from
Mar 27, 2024
Merged

Do not close parent popover when shadowed nested popover is opened #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 44 additions & 12 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,16 @@
}
}
</style>
<script src="./dist/popover.js"></script>
<script type="module">
import { apply, isSupported } from './dist/popover-fn.js';

if (isSupported()) {
console.log('native `popover` support detected, no polyfill used');
} else {
apply();
console.log('polyfill applied');
}
</script>
</head>

<body>
Expand Down Expand Up @@ -59,22 +68,40 @@ <h1>Popover Attribute Polyfill</h1>
<div id="popover13" popover>
Popover 13 (should be red if browser has @layer support)
</div>
<div id="menu-with-shadowed-popover" popover="auto">
Menu with shadowed popover
<div id="menu-host"></div>
</div>
<dialog popover>I'm a dialog!</dialog>
<div id="host"></div>

<script type="module">
const host = document.getElementById('host');
const shadowRoot = host.attachShadow({ mode: 'open' });
shadowRoot.innerHTML = `<button popovertarget="shadowedPopover">
Click to toggle shadowed Popover (auto)
</button>
<button popovertarget="shadowedNestedPopover">
Click to toggle shadowed nested Popover (auto)
</button>
<div id="shadowedPopover" popover="auto">Shadowed Popover (auto)</div>
<div>
<div id="shadowedNestedPopover" popover="auto">Shadowed Nested Popover (auto)</div>
</div>`;
shadowRoot.innerHTML = `
<button popovertarget="shadowedPopover">
Click to toggle shadowed Popover (auto)
</button>
<button popovertarget="shadowedNestedPopover">
Click to toggle shadowed nested Popover (auto)
</button>
<div id="shadowedPopover" popover="auto">Shadowed Popover (auto)</div>
<div>
<div id="shadowedNestedPopover" popover="auto">Shadowed Nested Popover (auto)</div>
</div>
`;

const menuHost = document.getElementById('menu-host');
const menuShadowRoot = menuHost.attachShadow({ mode: 'open' });
menuShadowRoot.innerHTML = `
<button popovertarget="shadowedMenuInner">
Toggle inner shadowed popover
</button>
<div id="shadowedMenuInner" popover="auto" style="top: 100px">
Inner shadowed popover
</div>
`;

document.getElementById('crossTreeToggle').popoverTargetElement =
shadowRoot.getElementById('shadowedPopover');
</script>
Expand All @@ -99,12 +126,17 @@ <h1>Popover Attribute Polyfill</h1>
</button>
<button popovertarget="popover10">Click to toggle Popover 10</button>
<button popovertarget="notExist">Click to toggle nothing</button>
<button id="crossTreeToggle">Click to toggle shadowed Popover</button>
<button id="crossTreeToggle">
Click to toggle shadowed Popover (cross-tree)
</button>
<button popovertargetaction="show" popovertarget="popover11">Menu</button>
<button popovertarget="popover12">
<span id="shadowInInvoker"></span>
</button>
<button popovertarget="popover13">Click to toggle Popover 13</button>
<button popovertarget="menu-with-shadowed-popover">
Toggle menu with shadowed nested popover
</button>

<script type="module">
const shadowInInvoker = document.getElementById('shadowInInvoker');
Expand Down
6 changes: 3 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@
"format": "run-p format:*",
"lint:js": "run-s prettier:check eslint:check tsc",
"lint": "run-p lint:*",
"test:unit": "playwright test",
"test:e2e": "echo No e2e tests yet",
"test:e2e:ci": "echo No e2e tests yet",
"test:unit": "echo No unit tests yet",
"test:e2e": "playwright test",
"test:e2e:ci": "run-s test:e2e \"test:e2e -- --browser=firefox\"",
"test": "run-p test:unit test:e2e",
"test:ci": "run-p test:unit test:e2e:ci",
"prepare": "husky",
Expand Down
7 changes: 3 additions & 4 deletions src/popover-helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,14 @@ function nearestInclusiveTargetPopoverForInvoker(
function topMostPopoverAncestor(newPopover: HTMLElement): HTMLElement | null {
const popoverPositions = new Map();
let i = 0;
const document = newPopover.ownerDocument;
for (const popover of autoPopoverList.get(document) || []) {
for (const popover of autoPopoverList.get(newPopover.ownerDocument) || []) {
popoverPositions.set(popover, i);
i += 1;
}
popoverPositions.set(newPopover, i);
i += 1;
let topMostPopoverAncestor: HTMLElement | null = null;
function checkAncestor(candidate: HTMLElement | null) {
function checkAncestor(candidate: Node | null) {
const candidateAncestor = nearestInclusiveOpenPopover(candidate);
if (candidateAncestor === null) return null;
const candidatePosition = popoverPositions.get(candidateAncestor);
Expand All @@ -155,7 +154,7 @@ function topMostPopoverAncestor(newPopover: HTMLElement): HTMLElement | null {
topMostPopoverAncestor = candidateAncestor!;
}
}
checkAncestor(newPopover?.parentElement);
checkAncestor(newPopover.parentElement || getRootNode(newPopover));
return topMostPopoverAncestor;
}

Expand Down
21 changes: 21 additions & 0 deletions tests/triggers.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,27 @@ test('clicking button[popovertarget=shadowedNestedPopover] should hide open nest
await expect(popover).toBeHidden();
});

test('clicking button[popovertarget=shadowedMenuInner] should not hide open parent popover in a parent (shadow) tree scope', async ({
page,
}) => {
const outerPopover = (await page.locator('#menu-with-shadowed-popover')).nth(
0,
);
await expect(outerPopover).toBeHidden();
await expect(
await outerPopover.evaluate((node) => node.showPopover()),
).toBeUndefined();
await expect(outerPopover).toBeVisible();
const innerPopover = (await page.locator('#shadowedMenuInner')).nth(0);
await expect(innerPopover).toBeHidden();
await page.click('button[popovertarget=shadowedMenuInner]');
await expect(innerPopover).toBeVisible();
await expect(outerPopover).toBeVisible();
await page.click('button[popovertarget=menu-with-shadowed-popover]');
await expect(innerPopover).toBeHidden();
await expect(outerPopover).toBeHidden();
});

test("button 'popovertargetElement' property should return target element", async ({
page,
}) => {
Expand Down