Skip to content

Commit

Permalink
Bug 1935232 [wpt PR 49528] - Update dialog closedby behavior to only …
Browse files Browse the repository at this point in the history
…close one at a time, a=testonly

Automatic update from web-platform-tests
Update dialog closedby behavior to only close one at a time

Per the discussions at:
- openui/open-ui#1128
- whatwg/html#9373 (comment)

both developer and editor feedback is that the prior behavior was
a bit odd. In the case where there are "nested" dialogs, the desired
behavior is for light dismiss clicks to just pop the topmost dialog
off the stack, rather than trying to close all non-clicked dialogs.

This CL implements that behavior and updates the test. After
implementing this behavior and playing with it a bit, I agree
with the feedback - this feels more like what users would expect.

Once this lands and I get a bit of feedback, I'll incorporate this
into the spec PR.

Bug: 376516550
Change-Id: I4b5cfb7dd4a27fc304e52759d882b47394e55524
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6068807
Commit-Queue: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: David Baron <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1391794}

--

wpt-commits: e96c4c3d5c6510202598f558ea520b80f9507b31
wpt-pr: 49528
  • Loading branch information
mfreed7 authored and moz-wptsync-bot committed Dec 10, 2024
1 parent 6a4b83e commit 8be1339
Showing 1 changed file with 19 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#popoverA { top: 150px; bottom: auto; padding:0; }
#dialogB { top: 200px; bottom: auto; padding:0; }
#popoverB { top: 250px; bottom: auto; padding:0; }
dialog { position: fixed; }
</style>

<script>
Expand Down Expand Up @@ -71,38 +72,43 @@
await clickOn(unrelated);
// Clicking outside all is actually a click on a dialog backdrop.
// If dialogB is modal, it'll be dialogB, which is nested inside popoverA.
assertStates(false,modalB,false,false);
// Either way, both popoverB and dialogB should close.
assertStates(true,modalB,false,false);
await clickOn(unrelated);
// Clicking outside again should close the remaining two.
assertStates(false,false,false,false);
},`clicking outside all with ${modalAString} and ${modalBString}`);

promise_test(async (t) => {
openDialogPopoverStack(t,modalA,modalB);
await clickOn(popoverB);
// Clicking popoverB will keep both popovers plus the intervening dialogB
// open, because they're a stack.
assertStates(false,true,true,true);
// Clicking popoverB should keep everything open.
assertStates(true,true,true,true);
},`clicking popoverB with ${modalAString} and ${modalBString}`);

promise_test(async (t) => {
openDialogPopoverStack(t,modalA,modalB);
await clickOn(dialogB);
// dialogB is nested inside popoverA.
assertStates(false,true,true,false);
// Only popoverB should be light dismissed.
assertStates(true,true,true,false);
},`clicking dialogB with ${modalAString} and ${modalBString}`);

promise_test(async (t) => {
openDialogPopoverStack(t,modalA,modalB);
await clickOn(popoverA);
// If dialogB is modal, then clicking popoverA is actually a backdrop
// click on dialogB, which will close it. PopoverA stays open because
// dialogB is nested inside popoverA.
assertStates(false,true,!modalB,false);
// Both dialogB and popoverB should be light dismissed.
assertStates(true,true,false,false);
},`clicking popoverA with ${modalAString} and ${modalBString}`);

promise_test(async (t) => {
openDialogPopoverStack(t,modalA,modalB);
await clickOn(dialogB);
// Again, this is a backdrop click on dialogB.
assertStates(false,true,true,false);
await clickOn(dialogA);
// If dialogB is modal, clicking on dialogA is actually clicking on dialogB,
// which means popoverB will stay open.
assertStates(true,modalB,false,false);
await clickOn(dialogA);
// The next click on dialogA should light dismiss popoverA.
assertStates(true,false,false,false);
},`clicking dialogA with ${modalAString} and ${modalBString}`);
});
});
Expand Down

0 comments on commit 8be1339

Please sign in to comment.