Skip to content

Commit b5ddb36

Browse files
asynclizcopybara-github
authored andcommitted
fix(iconbutton): use event dispatch hooks for toggle clicks
PiperOrigin-RevId: 876324676
1 parent 25ff820 commit b5ddb36

3 files changed

Lines changed: 51 additions & 43 deletions

File tree

iconbutton/internal/icon-button.ts

Lines changed: 35 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ import {
2020
type FormSubmitterType,
2121
} from '../../internal/controller/form-submitter.js';
2222
import {isRtl} from '../../internal/controller/is-rtl.js';
23+
import {
24+
afterDispatch,
25+
setupDispatchHooks,
26+
} from '../../internal/events/dispatch-hooks.js';
2327
import {
2428
internals,
2529
mixinElementInternals,
@@ -146,9 +150,36 @@ export class IconButton extends iconButtonBaseClass implements FormSubmitter {
146150

147151
constructor() {
148152
super();
149-
if (!isServer) {
150-
this.addEventListener('click', this.handleClick.bind(this));
151-
}
153+
if (isServer) return;
154+
setupDispatchHooks(this, 'click');
155+
this.addEventListener('click', (event) => {
156+
// If the button is soft-disabled or a disabled link, we need to
157+
// explicitly prevent the click from propagating to other event listeners
158+
// as well as prevent the default action. This is because the underlying
159+
// `<button>` or `<a>` element is not actually `:disabled`.
160+
if (this.softDisabled || (this.disabled && this.href)) {
161+
event.stopImmediatePropagation();
162+
event.preventDefault();
163+
return;
164+
}
165+
166+
// Save current selected state to toggle, since an external event listener
167+
// may also change the selected state on click.
168+
const wasSelected = this.selected;
169+
afterDispatch(event, () => {
170+
if (!this.toggle || this.disabled || event.defaultPrevented) {
171+
return;
172+
}
173+
174+
this.selected = !wasSelected;
175+
this.dispatchEvent(
176+
new InputEvent('input', {bubbles: true, composed: true}),
177+
);
178+
// Bubbles but does not compose to mimic native browser <input> & <select>
179+
// Additionally, native change event is not an InputEvent.
180+
this.dispatchEvent(new Event('change', {bubbles: true}));
181+
});
182+
});
152183
}
153184

154185
protected override willUpdate() {
@@ -180,8 +211,7 @@ export class IconButton extends iconButtonBaseClass implements FormSubmitter {
180211
aria-expanded="${(!this.href && ariaExpanded) || nothing}"
181212
aria-pressed="${ariaPressedValue}"
182213
aria-disabled=${(!this.href && this.softDisabled) || nothing}
183-
?disabled="${!this.href && this.disabled}"
184-
@click="${this.handleClickOnChild}">
214+
?disabled="${!this.href && this.disabled}">
185215
${this.renderFocusRing()}
186216
${this.renderRipple()}
187217
${!this.selected ? this.renderIcon() : nothing}
@@ -247,41 +277,4 @@ export class IconButton extends iconButtonBaseClass implements FormSubmitter {
247277
this.flipIcon = isRtl(this, this.flipIconInRtl);
248278
super.connectedCallback();
249279
}
250-
251-
/** Handles a click on this element. */
252-
private handleClick(event: MouseEvent) {
253-
// If the icon button is soft-disabled, we need to explicitly prevent the
254-
// click from propagating to other event listeners as well as prevent the
255-
// default action.
256-
if (!this.href && this.softDisabled) {
257-
event.stopImmediatePropagation();
258-
event.preventDefault();
259-
return;
260-
}
261-
}
262-
263-
/**
264-
* Handles a click on the child <div> or <button> element within this
265-
* element's shadow DOM.
266-
*/
267-
private async handleClickOnChild(event: Event) {
268-
// Allow the event to propagate
269-
await 0;
270-
if (
271-
!this.toggle ||
272-
this.disabled ||
273-
this.softDisabled ||
274-
event.defaultPrevented
275-
) {
276-
return;
277-
}
278-
279-
this.selected = !this.selected;
280-
this.dispatchEvent(
281-
new InputEvent('input', {bubbles: true, composed: true}),
282-
);
283-
// Bubbles but does not compose to mimic native browser <input> & <select>
284-
// Additionally, native change event is not an InputEvent.
285-
this.dispatchEvent(new Event('change', {bubbles: true}));
286-
}
287280
}

internal/events/dispatch-hooks.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ export function setupDispatchHooks(
154154
// Re-dispatch the event. We can't reuse `redispatchEvent()` since we
155155
// need to add the hooks to the copy before it's dispatched.
156156
isRedispatching = true;
157-
const dispatched = element.dispatchEvent(eventCopy);
157+
const dispatched = event.composedPath()[0].dispatchEvent(eventCopy);
158158
isRedispatching = false;
159159
if (!dispatched) {
160160
event.preventDefault();

internal/events/dispatch-hooks_test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,21 @@ describe('dispatch hooks', () => {
3939
.withContext('element.addEventListener')
4040
.toHaveBeenCalledTimes(3);
4141
});
42+
43+
it('triggers internal event listeners when a composed element is the source of the event', () => {
44+
const shadowRoot = element.attachShadow({mode: 'open'});
45+
const composedElement = document.createElement('button');
46+
shadowRoot.appendChild(composedElement);
47+
const innerClickListener = jasmine.createSpy('innerClickListener');
48+
composedElement.addEventListener('click', innerClickListener);
49+
50+
setupDispatchHooks(element, 'click');
51+
composedElement.click();
52+
53+
expect(innerClickListener)
54+
.withContext('innerClickListener')
55+
.toHaveBeenCalledTimes(1);
56+
});
4257
});
4358

4459
describe('afterDispatch()', () => {

0 commit comments

Comments
 (0)