Skip to content

Commit

Permalink
Don't use kHideImmediately for dialog and fullscreen
Browse files Browse the repository at this point in the history
When a modal dialog or fullscreen element is shown, leave time
for the hide animation to be shown for open Popovers, and fire
the "hide" event synchronously.

See resolution here:
openui/open-ui#578 (comment)

Bug: 1307772
Change-Id: I36d7c26a7ab4dc9f1808ca86441c95f363cf3a45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4032790
Commit-Queue: Mason Freed <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1078251}
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Dec 1, 2022
1 parent b763d6d commit 185461a
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,19 @@
popover.showPopover();
assert_true(isElementVisible(popover));
assert_equals(popover.getAnimations({subtree: true}).length,0);
let animation
popover.addEventListener('popoverhide', () => {
popover.animate([{opacity: 1},{opacity: 0}],1000000);
animation = popover.animate([{opacity: 1},{opacity: 0}],1000000);
});
assert_equals(popover.getAnimations({subtree: true}).length,0,'There should be no animations yet');
dialog.showModal(); // Force hide the popover
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');
assert_true(isElementVisible(popover),'And the animation should keep the popover visible');
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, even when the hide is due to dialog.showModal');

promise_test(async (t) => {
const {popover, descendent} = createPopover(t,'');
Expand Down
3 changes: 2 additions & 1 deletion html/semantics/popovers/popover-focus.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,9 @@
assert_equals(document.activeElement, expectedFocusedElement, `${testName} activated by popover.showPopover()`);
const dialog = document.body.appendChild(document.createElement('dialog'));
dialog.showModal();
assert_false(isElementVisible(popover), 'Opening a modal dialog should hide the popover immediately');
assert_false(popover.matches(':open'), 'Opening a modal dialog should hide the popover');
assert_not_equals(document.activeElement, priorFocus, 'prior element should *not* get focus when a modal dialog is shown');
await finishAnimationsAndVerifyHide(popover);
dialog.close();
dialog.remove();

Expand Down

0 comments on commit 185461a

Please sign in to comment.