Skip to content

Commit

Permalink
update option label on change; fixes #1971 (#2236)
Browse files Browse the repository at this point in the history
  • Loading branch information
claviska authored Oct 28, 2024
1 parent 2e73b3d commit aca0f3d
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 28 deletions.
1 change: 1 addition & 0 deletions docs/pages/resources/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti

- Added support for <kbd>Enter</kbd> to `<sl-split-panel>` to align with ARIA APG's [window splitter pattern](https://www.w3.org/WAI/ARIA/apg/patterns/windowsplitter/) [#2234]
- Fixed a bug in `<sl-carousel>` that caused the navigation icons to be reversed
- Fixed a bug in `<sl-select>` that prevented label changes in `<sl-option>` from updating the controller [#1971]

## 2.18.0

Expand Down
21 changes: 7 additions & 14 deletions src/components/option/option.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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() {
Expand Down
14 changes: 1 addition & 13 deletions src/components/option/option.test.ts
Original file line number Diff line number Diff line change
@@ -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('<sl-option>', () => {
Expand Down Expand Up @@ -32,17 +31,6 @@ describe('<sl-option>', () => {
expect(el.getAttribute('aria-disabled')).to.equal('true');
});

it('emits the slotchange event when the label changes', async () => {
const el = await fixture<SlOption>(html` <sl-option>Text</sl-option> `);
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<SlOption>(html` <sl-option>Text</sl-option> `);

Expand Down
3 changes: 2 additions & 1 deletion src/components/select/select.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down

0 comments on commit aca0f3d

Please sign in to comment.