From aca0f3d64a084d40be3eb00fbb9f2d3fbf2b9e6f Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Mon, 28 Oct 2024 13:02:57 -0400 Subject: [PATCH] update option label on change; fixes #1971 (#2236) --- docs/pages/resources/changelog.md | 1 + src/components/option/option.component.ts | 21 +++++++-------------- src/components/option/option.test.ts | 14 +------------- src/components/select/select.component.ts | 3 ++- 4 files changed, 11 insertions(+), 28 deletions(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index 59251fa62e..487f79b7e7 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -16,6 +16,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti - Added support for Enter to `` to align with ARIA APG's [window splitter pattern](https://www.w3.org/WAI/ARIA/apg/patterns/windowsplitter/) [#2234] - Fixed a bug in `` that caused the navigation icons to be reversed +- Fixed a bug in `` that prevented label changes in `` from updating the controller [#1971] ## 2.18.0 diff --git a/src/components/option/option.component.ts b/src/components/option/option.component.ts index a9d35b50d6..27e1f09648 100644 --- a/src/components/option/option.component.ts +++ b/src/components/option/option.component.ts @@ -31,7 +31,6 @@ export default class SlOption extends ShoelaceElement { static styles: CSSResultGroup = [componentStyles, styles]; static dependencies = { 'sl-icon': SlIcon }; - private cachedTextLabel: string; // @ts-expect-error - Controller is currently unused private readonly localize = new LocalizeController(this); @@ -58,19 +57,13 @@ export default class SlOption extends ShoelaceElement { } private handleDefaultSlotChange() { - const textLabel = this.getTextLabel(); - - // Ignore the first time the label is set - if (typeof this.cachedTextLabel === 'undefined') { - this.cachedTextLabel = textLabel; - return; - } - - // When the label changes, emit a slotchange event so parent controls see it - if (textLabel !== this.cachedTextLabel) { - this.cachedTextLabel = textLabel; - this.emit('slotchange', { bubbles: true, composed: false, cancelable: false }); - } + // When the label changes, tell the controller to update + customElements.whenDefined('sl-select').then(() => { + const controller = this.closest('sl-select'); + if (controller) { + controller.handleDefaultSlotChange(); + } + }); } private handleMouseEnter() { diff --git a/src/components/option/option.test.ts b/src/components/option/option.test.ts index 2d5320a7ca..ac7f243a8a 100644 --- a/src/components/option/option.test.ts +++ b/src/components/option/option.test.ts @@ -1,6 +1,5 @@ import '../../../dist/shoelace.js'; -import { aTimeout, expect, fixture, html, waitUntil } from '@open-wc/testing'; -import sinon from 'sinon'; +import { aTimeout, expect, fixture, html } from '@open-wc/testing'; import type SlOption from './option.js'; describe('', () => { @@ -32,17 +31,6 @@ describe('', () => { expect(el.getAttribute('aria-disabled')).to.equal('true'); }); - it('emits the slotchange event when the label changes', async () => { - const el = await fixture(html` Text `); - const slotChangeHandler = sinon.spy(); - - el.addEventListener('slotchange', slotChangeHandler); - el.textContent = 'New Text'; - await waitUntil(() => slotChangeHandler.calledOnce); - - expect(slotChangeHandler).to.have.been.calledOnce; - }); - it('should convert non-string values to string', async () => { const el = await fixture(html` Text `); diff --git a/src/components/select/select.component.ts b/src/components/select/select.component.ts index 8698c7615e..d91adfb2a8 100644 --- a/src/components/select/select.component.ts +++ b/src/components/select/select.component.ts @@ -501,7 +501,8 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon } } - private handleDefaultSlotChange() { + /* @internal - used by options to update labels */ + public handleDefaultSlotChange() { if (!customElements.get('wa-option')) { customElements.whenDefined('wa-option').then(() => this.handleDefaultSlotChange()); }