Skip to content

Commit 2f69c8f

Browse files
mfreed7chromium-wpt-export-bot
authored andcommitted
Update popover post-toggle event naming and behavior
This CL updates the post-toggle event in the following ways: 1. Rename the 'aftertoggle' event to 'toggle'. 2. Rename PopoverToggleEvent to ToggleEvent. 3. Rename the currentState attribute to oldState. 4. Add event coalescing behavior. If two transitions occur before the first 'toggle' event has been fired, cancel the first event and queue a replacement that has oldState === newState. These changes were driven by the corresponding changes to the spec PR: whatwg/html#8717 Bug: 1307772 Change-Id: Iabc5a9093d7cef3bbd6e54e488d8e571c51ea568 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195120 Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1098728}
1 parent 859dd63 commit 2f69c8f

File tree

4 files changed

+186
-120
lines changed

4 files changed

+186
-120
lines changed

html/semantics/popovers/idlharness.tentative.html

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,12 @@
4141
'document.getElementById("b3")',
4242
],
4343
BeforeToggleEvent: [
44-
'new PopoverToggleEvent("beforetoggle")',
45-
'new PopoverToggleEvent("beforetoggle", {currentState: "open"})',
46-
'new PopoverToggleEvent("beforetoggle", {currentState: "open",newState: "open"})',
47-
'new PopoverToggleEvent("aftertoggle")',
48-
'new PopoverToggleEvent("aftertoggle", {currentState: "open"})',
49-
'new PopoverToggleEvent("aftertoggle", {currentState: "open",newState: "open"})',
44+
'new ToggleEvent("beforetoggle")',
45+
'new ToggleEvent("beforetoggle", {oldState: "open"})',
46+
'new ToggleEvent("beforetoggle", {oldState: "open",newState: "open"})',
47+
'new ToggleEvent("toggle")',
48+
'new ToggleEvent("toggle", {oldState: "open"})',
49+
'new ToggleEvent("toggle", {oldState: "open",newState: "open"})',
5050
],
5151
});
5252
}

html/semantics/popovers/popover-events.tentative.html

Lines changed: 93 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@
1010
<div popover>Popover</div>
1111

1212
<script>
13+
function getPopoverAndSignal(t) {
14+
const popover = document.querySelector('[popover]');
15+
const controller = new AbortController();
16+
const signal = controller.signal;
17+
t.add_cleanup(() => controller.abort());
18+
return {popover, signal};
19+
}
1320
window.onload = () => {
1421
for(const method of ["listener","attribute"]) {
1522
promise_test(async t => {
@@ -22,52 +29,48 @@
2229
function listener(e) {
2330
if (e.type === "beforetoggle") {
2431
if (e.newState === "open") {
25-
assert_equals(e.currentState,"closed",'The "beforetoggle" event should be fired before the popover is open');
32+
++showCount;
33+
assert_equals(e.oldState,"closed",'The "beforetoggle" event should be fired before the popover is open');
2634
assert_true(e.target.matches(':closed'),'The popover should be in the :closed state when the opening event fires.');
2735
assert_false(e.target.matches(':open'),'The popover should *not* be in the :open state when the opening event fires.');
28-
++showCount;
2936
} else {
37+
++hideCount;
3038
assert_equals(e.newState,"closed",'Popover toggleevent states should be "open" and "closed"');
31-
assert_equals(e.currentState,"open",'The "beforetoggle" event should be fired before the popover is closed')
39+
assert_equals(e.oldState,"open",'The "beforetoggle" event should be fired before the popover is closed')
3240
assert_true(e.target.matches(':open'),'The popover should be in the :open state when the hiding event fires.');
3341
assert_false(e.target.matches(':closed'),'The popover should *not* be in the :closed state when the hiding event fires.');
34-
++hideCount;
3542
}
3643
} else {
37-
assert_equals(e.type,"aftertoggle",'Popover events should be "beforetoggle" and "aftertoggle"')
44+
assert_equals(e.type,"toggle",'Popover events should be "beforetoggle" and "toggle"')
3845
if (e.newState === "open") {
39-
assert_equals(e.currentState,"open",'Aftertoggle should be fired after the popover is open');
46+
++afterShowCount;
4047
if (document.body.contains(e.target)) {
4148
assert_true(e.target.matches(':open'),'The popover should be in the :open state when the after opening event fires.');
4249
assert_false(e.target.matches(':closed'),'The popover should *not* be in the :closed state when the after opening event fires.');
4350
}
44-
++afterShowCount;
4551
} else {
52+
++afterHideCount;
4653
assert_equals(e.newState,"closed",'Popover toggleevent states should be "open" and "closed"');
47-
assert_equals(e.currentState,"closed",'Aftertoggle should be fired after the popover is closed');
4854
assert_true(e.target.matches(':closed'),'The popover should be in the :closed state when the after hiding event fires.');
4955
assert_false(e.target.matches(':open'),'The popover should *not* be in the :open state when the after hiding event fires.');
50-
++afterHideCount;
5156
}
52-
e.preventDefault(); // "aftertoggle" should not be cancelable.
57+
e.preventDefault(); // "toggle" should not be cancelable.
5358
}
5459
};
5560
switch (method) {
5661
case "listener":
57-
const controller = new AbortController();
58-
const signal = controller.signal;
59-
t.add_cleanup(() => controller.abort());
62+
const {signal} = getPopoverAndSignal(t);
6063
// These events bubble.
6164
document.addEventListener('beforetoggle', listener, {signal});
62-
document.addEventListener('aftertoggle', listener, {signal});
65+
document.addEventListener('toggle', listener, {signal});
6366
break;
6467
case "attribute":
6568
assert_false(popover.hasAttribute('onbeforetoggle'));
6669
t.add_cleanup(() => popover.removeAttribute('onbeforetoggle'));
6770
popover.onbeforetoggle = listener;
68-
assert_false(popover.hasAttribute('onaftertoggle'));
69-
t.add_cleanup(() => popover.removeAttribute('onaftertoggle'));
70-
popover.onaftertoggle = listener;
71+
assert_false(popover.hasAttribute('ontoggle'));
72+
t.add_cleanup(() => popover.removeAttribute('ontoggle'));
73+
popover.ontoggle = listener;
7174
break;
7275
default: assert_unreached();
7376
}
@@ -82,7 +85,7 @@
8285
assert_equals(0,afterShowCount);
8386
assert_equals(0,afterHideCount);
8487
await waitForRender();
85-
assert_equals(1,afterShowCount,'aftertoggle show is fired asynchronously');
88+
assert_equals(1,afterShowCount,'toggle show is fired asynchronously');
8689
assert_equals(0,afterHideCount);
8790
assert_true(popover.matches(':open'));
8891
popover.hidePopover();
@@ -93,7 +96,7 @@
9396
assert_equals(0,afterHideCount);
9497
await waitForRender();
9598
assert_equals(1,afterShowCount);
96-
assert_equals(1,afterHideCount,'aftertoggle hide is fired asynchronously');
99+
assert_equals(1,afterHideCount,'toggle hide is fired asynchronously');
97100
// No additional events
98101
await waitForRender();
99102
await waitForRender();
@@ -106,10 +109,7 @@
106109
}
107110

108111
promise_test(async t => {
109-
const popover = document.querySelector('[popover]');
110-
const controller = new AbortController();
111-
const signal = controller.signal;
112-
t.add_cleanup(() => controller.abort());
112+
const {popover,signal} = getPopoverAndSignal(t);
113113
let cancel = true;
114114
popover.addEventListener('beforetoggle',(e) => {
115115
if (e.newState !== "open")
@@ -128,10 +128,7 @@
128128
}, 'The "beforetoggle" event is cancelable for the "opening" transition');
129129

130130
promise_test(async t => {
131-
const popover = document.querySelector('[popover]');
132-
const controller = new AbortController();
133-
const signal = controller.signal;
134-
t.add_cleanup(() => {controller.abort();});
131+
const {popover,signal} = getPopoverAndSignal(t);
135132
popover.addEventListener('beforetoggle',(e) => {
136133
assert_not_equals(e.newState,"closed",'The "beforetoggle" event was fired for the closing transition');
137134
}, {signal});
@@ -144,5 +141,74 @@
144141
await waitForRender(); // Check for async events also
145142
assert_false(popover.matches(':open'));
146143
}, 'The "beforetoggle" event is not fired for element removal');
144+
145+
promise_test(async t => {
146+
const {popover,signal} = getPopoverAndSignal(t);
147+
let events;
148+
function resetEvents() {
149+
events = {
150+
singleShow: false,
151+
singleHide: false,
152+
coalescedShow: false,
153+
coalescedHide: false,
154+
};
155+
}
156+
function setEvent(type) {
157+
assert_equals(events[type],false,'event repeated');
158+
events[type] = true;
159+
}
160+
function assertOnly(type,msg) {
161+
Object.keys(events).forEach(val => {
162+
assert_equals(events[val],val===type,`${msg} (${val})`);
163+
});
164+
}
165+
popover.addEventListener('toggle',(e) => {
166+
switch (e.newState) {
167+
case "open":
168+
switch (e.oldState) {
169+
case "open": setEvent('coalescedShow'); break;
170+
case "closed": setEvent('singleShow'); break;
171+
default: assert_unreached();
172+
}
173+
break;
174+
case "closed":
175+
switch (e.oldState) {
176+
case "closed": setEvent('coalescedHide'); break;
177+
case "open": setEvent('singleHide'); break;
178+
default: assert_unreached();
179+
}
180+
break;
181+
default: assert_unreached();
182+
}
183+
}, {signal});
184+
185+
resetEvents();
186+
assertOnly('none');
187+
assert_false(popover.matches(':open'));
188+
popover.showPopover();
189+
await waitForRender();
190+
assert_true(popover.matches(':open'));
191+
assertOnly('singleShow','Single event should have been fired, which is a "show"');
192+
193+
resetEvents();
194+
popover.hidePopover();
195+
popover.showPopover(); // Immediate re-show
196+
await waitForRender();
197+
assert_true(popover.matches(':open'));
198+
assertOnly('coalescedShow','Single coalesced event should have been fired, which is a "show"');
199+
200+
resetEvents();
201+
popover.hidePopover();
202+
await waitForRender();
203+
assertOnly('singleHide','Single event should have been fired, which is a "hide"');
204+
assert_false(popover.matches(':open'));
205+
206+
resetEvents();
207+
popover.showPopover();
208+
popover.hidePopover(); // Immediate re-hide
209+
await waitForRender();
210+
assertOnly('coalescedHide','Single coalesced event should have been fired, which is a "hide"');
211+
assert_false(popover.matches(':open'));
212+
}, 'The "toggle" event is coalesced');
147213
};
148214
</script>

0 commit comments

Comments
 (0)