Skip to content

Commit 588dae3

Browse files
authored
Fix sl-change event being emitted during initial render of radiogroup (#2537)
1 parent 0c4f19b commit 588dae3

File tree

3 files changed

+57
-38
lines changed

3 files changed

+57
-38
lines changed

.changeset/violet-rice-attend.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sl-design-system/radio-group': patch
3+
---
4+
5+
Fix `sl-change` event being emitted during initial render

packages/components/radio-group/src/radio-group.spec.ts

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,6 @@ import { RadioGroup } from './radio-group.js';
1010
describe('sl-radio-group', () => {
1111
let el: RadioGroup;
1212

13-
describe('empty', () => {
14-
beforeEach(async () => {
15-
el = await fixture(html`<sl-radio-group></sl-radio-group>`);
16-
});
17-
18-
it('should not break', () => {
19-
expect(el).shadowDom.to.equalSnapshot();
20-
});
21-
});
22-
2313
describe('defaults', () => {
2414
beforeEach(async () => {
2515
el = await fixture(html`
@@ -31,10 +21,6 @@ describe('sl-radio-group', () => {
3121
`);
3222
});
3323

34-
it('should render correctly', () => {
35-
expect(el).shadowDom.to.equalSnapshot();
36-
});
37-
3824
it('should have a role of radiogroup', () => {
3925
expect(el.internals.role).to.equal('radiogroup');
4026
});
@@ -402,6 +388,47 @@ describe('sl-radio-group', () => {
402388
});
403389
});
404390

391+
describe('sl-change event', () => {
392+
it('should not be emitted during initial render with a value', async () => {
393+
const onChange = spy();
394+
395+
document.body.addEventListener('sl-change', onChange);
396+
397+
el = await fixture(html`
398+
<sl-radio-group value="2">
399+
<sl-radio value="1">Option 1</sl-radio>
400+
<sl-radio value="2">Option 2</sl-radio>
401+
<sl-radio value="3">Option 3</sl-radio>
402+
</sl-radio-group>
403+
`);
404+
405+
expect(onChange).to.not.have.been.called;
406+
407+
document.body.removeEventListener('sl-change', onChange);
408+
});
409+
410+
it('should be emitted after initial render when a radio is checked', async () => {
411+
const onChange = spy();
412+
413+
document.body.addEventListener('sl-change', onChange);
414+
415+
el = await fixture(html`
416+
<sl-radio-group value="2">
417+
<sl-radio value="1">Option 1</sl-radio>
418+
<sl-radio value="2">Option 2</sl-radio>
419+
<sl-radio value="3">Option 3</sl-radio>
420+
</sl-radio-group>
421+
`);
422+
423+
el.querySelector<HTMLElement>('sl-radio[value="1"]')?.click();
424+
await new Promise(resolve => setTimeout(resolve));
425+
426+
expect(onChange).to.have.been.calledOnce;
427+
428+
document.body.removeEventListener('sl-change', onChange);
429+
});
430+
});
431+
405432
describe('form reset', () => {
406433
let form: HTMLFormElement;
407434

packages/components/radio-group/src/radio-group.ts

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,6 @@ export class RadioGroup<T = any> extends FormControlMixin(LitElement) {
5454
/** The initial state when the form was associated with the radio group. Used to reset the group. */
5555
#initialState?: T;
5656

57-
/**
58-
* Stores the initial value of the radio group when it is first rendered.
59-
* Used to determine if the value has changed since initialization and to manage event emission, avoiding unnecessary events on the first render.
60-
*/
61-
#initialValue?: T;
62-
63-
/**
64-
* Indicates whether the component is rendering for the first time.
65-
* Used to track the initial render and preventing unnecessary event emission.
66-
*/
67-
#isInitialRender = true;
68-
6957
/** When an option is checked, update the state. */
7058
#observer = new MutationObserver(mutations => {
7159
const { target } = mutations.find(m => m.attributeName === 'checked' && m.oldValue === null) || {};
@@ -189,11 +177,6 @@ export class RadioGroup<T = any> extends FormControlMixin(LitElement) {
189177
}
190178

191179
if (changes.has('value')) {
192-
if (this.#isInitialRender) {
193-
this.#initialValue = this.value;
194-
this.#isInitialRender = false;
195-
}
196-
197180
this.#observer.disconnect();
198181
this.radios?.forEach(radio => (radio.checked = radio.value === this.value));
199182
this.#observer.observe(this, OBSERVER_OPTIONS);
@@ -223,10 +206,12 @@ export class RadioGroup<T = any> extends FormControlMixin(LitElement) {
223206
this.updateState({ touched: true });
224207
}
225208

226-
#onSlotchange(): void {
209+
async #onSlotchange(): Promise<void> {
227210
this.#rovingTabindexController.clearElementCache();
228211

229-
this.radios?.forEach(radio => {
212+
this.#observer.disconnect();
213+
214+
for (const radio of this.radios ?? []) {
230215
radio.checked = radio.value === this.value;
231216

232217
if (typeof this.disabled === 'boolean') {
@@ -236,22 +221,24 @@ export class RadioGroup<T = any> extends FormControlMixin(LitElement) {
236221
if (this.size) {
237222
radio.size = this.size;
238223
}
239-
});
224+
225+
// Wait for the `<sl-radio>` to stabilize, otherwise we'll trigger the observer
226+
await radio.updateComplete;
227+
}
228+
229+
this.#observer.observe(this, OBSERVER_OPTIONS);
240230
}
241231

242232
#setSelectedOption(option?: Radio<T>, emitEvent = true): void {
243233
this.radios?.forEach(radio => (radio.checked = radio.value === option?.value));
244234
this.value = option?.value;
245235

246236
if (emitEvent) {
247-
if (!this.#initialValue) {
248-
this.changeEvent.emit(this.value);
249-
}
237+
this.changeEvent.emit(this.value);
250238
this.updateState({ dirty: true });
251239
}
252240

253241
this.#updateValueAndValidity();
254-
this.#initialValue = undefined;
255242
}
256243

257244
#updateValueAndValidity(): void {

0 commit comments

Comments
 (0)