From 5dc870bfe1609a50702ef078a3da1b01620f7ef8 Mon Sep 17 00:00:00 2001 From: Elizabeth Mitchell Date: Tue, 2 Jan 2024 13:40:56 -0800 Subject: [PATCH] fix(chips): filter's `click.preventDefault()` not working when also updating `selected` Fixes #5199 This bug appeared when calling prevent default as well as changing the state of the chip in the same listener. Now calling preventDefault will always revert to the previous value. PiperOrigin-RevId: 595199149 --- chips/internal/filter-chip.ts | 8 +++++++- chips/internal/filter-chip_test.ts | 33 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/chips/internal/filter-chip.ts b/chips/internal/filter-chip.ts index 89127d4f8..c361b4373 100644 --- a/chips/internal/filter-chip.ts +++ b/chips/internal/filter-chip.ts @@ -108,11 +108,17 @@ export class FilterChip extends MultiActionChip { return; } + // Store prevValue to revert in case `chip.selected` is changed during an + // event listener. + const prevValue = this.selected; this.selected = !this.selected; const preventDefault = !redispatchEvent(this, event); if (preventDefault) { - this.selected = !this.selected; + // We should not do `this.selected = !this.selected`, since a client + // click listener could change its value. Instead, always revert to the + // original value. + this.selected = prevValue; return; } } diff --git a/chips/internal/filter-chip_test.ts b/chips/internal/filter-chip_test.ts index 266551598..ae894b1be 100644 --- a/chips/internal/filter-chip_test.ts +++ b/chips/internal/filter-chip_test.ts @@ -66,5 +66,38 @@ describe('Filter chip', () => { await harness.clickWithMouse(); expect(handler).toHaveBeenCalledTimes(0); }); + + it('always reverts value on preventDefault() even if selected is changed in listener', async () => { + const {chip, harness} = await setupTest(); + + chip.addEventListener( + 'click', + (event) => { + event.preventDefault(); + chip.selected = false; + }, + {once: true}, + ); + + await harness.clickWithMouse(); + expect(chip.selected) + .withContext('chip.selected reverts to false') + .toBeFalse(); + + chip.selected = true; + chip.addEventListener( + 'click', + (event) => { + event.preventDefault(); + chip.selected = false; + }, + {once: true}, + ); + + await harness.clickWithMouse(); + expect(chip.selected) + .withContext('chip.selected reverts to true') + .toBeTrue(); + }); }); });