fix: sd-combobox and sd-select (#1742)

.changeset/red-drinks-travel.md

@@ -0,0 +1,11 @@
+---
+'@solid-design-system/components': patch
+'@solid-design-system/docs': patch
+---
+
+Bugfixes and minor non-breaking changes to the sd-select and sd-combobox components
+
+- sd-combobox: emit events correctly
+- sd-combobox: set options' initial attributes
+- sd-select and sd-combobox: add max-options-tag-label attribute
+- sd-select: add --tag-max-width and ellipsis

packages/components/src/components/combobox/combobox.test.ts

@@ -1,3 +1,4 @@
+import '../../../dist/solid-components';
 import { aTimeout, expect, fixture, html, oneEvent, waitUntil } from '@open-wc/testing';
 import { clickOnElement } from '../../internal/test';
 import { sendKeys } from '@web/test-runner-commands';
@@ -7,7 +8,7 @@ import type SdCombobox from './combobox';
 import type SdOption from '../option/option';
 import type SdSelect from '../select/select';
 
-describe.skip('<sd-combobox>', () => {
+describe('<sd-combobox>', () => {
   describe('accessibility', () => {
     it('should pass accessibility tests when closed', async () => {
       const combobox = await fixture<SdCombobox>(html`
@@ -573,7 +574,7 @@ describe.skip('<sd-combobox>', () => {
 
     it('should remove tag and option when tag is focused and backspace is pressed', async () => {
       const el = await fixture<SdSelect>(html`
-        <sd-combobox value="option-1 option-2" multiple useTags>
+        <sd-combobox value="option-1 option-2" multiple>
           <sd-option value="option-1">Option 1</sd-option>
           <sd-option value="option-2">Option 2</sd-option>
           <sd-option value="option-3">Option 3</sd-option>
@@ -627,6 +628,33 @@ describe.skip('<sd-combobox>', () => {
 
       expect(el.value).to.deep.equal(['Option 2', 'Option 3']);
     });
+
+    it('should emit sd-change and sd-input when the value changes', async () => {
+      const el = await fixture<SdCombobox>(html`
+        <sd-combobox value="option-1" multiple>
+          <sd-option value="option-1">Option 1</sd-option>
+          <sd-option value="option-2">Option 2</sd-option>
+          <sd-option value="option-3">Option 3</sd-option>
+        </sd-combobox>
+      `);
+      const inputHandler = sinon.spy();
+      const changeHandler = sinon.spy();
+
+      el.addEventListener('sd-input', inputHandler);
+      el.addEventListener('sd-change', changeHandler);
+      await el.show();
+      await el.updateComplete;
+      const filteredListbox = el.shadowRoot!.querySelector('[part="filtered-listbox"]')!;
+      const secondOption = filteredListbox.querySelectorAll<SdOption>('sd-option')[1];
+      await clickOnElement(secondOption);
+      await el.updateComplete;
+      const thirdOption = filteredListbox.querySelectorAll<SdOption>('sd-option')[2];
+      await clickOnElement(thirdOption);
+      await el.updateComplete;
+
+      expect(inputHandler).to.have.been.calledTwice;
+      expect(changeHandler).to.have.been.calledTwice;
+    });
   });
 
   describe('when using constraint validation', () => {

packages/components/src/components/combobox/combobox.ts

@@ -156,6 +156,9 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
   /** Placeholder text to show as a hint when the combobox is empty. */
   @property() placeholder = this.localize.term('comboboxDefaultPlaceholder');
 
+  /** Label text shown on tag if max-options-visible is reached. */
+  @property({ attribute: 'max-options-tag-label' }) maxOptionsTagLabel = this.localize.term('tagsSelected');
+
   /** Disables the combobox control. */
   @property({ reflect: true, type: Boolean }) disabled = false;
 
@@ -284,7 +287,6 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
       this.selectedTextLabel = option?.getTextLabel() || '';
     }
     this.formControlController.updateValidity();
-    this.applySizeToOptions();
   }
 
   /** Gets the validity state object */
@@ -345,7 +347,7 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
             removable
             @keydown=${(event: KeyboardEvent) => this.handleTagMaxOptionsKeyDown(event)}
             @sd-remove=${(event: CustomEvent) => this.handleTagRemove(event)}
-            >${this.selectedOptions.length} ${this.localize.term('tagsSelected')}</sd-tag
+            >${this.selectedOptions.length} ${this.maxOptionsTagLabel}</sd-tag
           >
         `
       ];
@@ -455,16 +457,16 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
           this.setSelectedOptions(currentOption);
         }
 
-        // Set focus after updating so the value is announced by screen readers
-        this.updateComplete.then(() => this.displayInput.focus({ preventScroll: true }));
+        this.updateComplete.then(() => {
+          // Set focus after updating so the value is announced by screen readers
+          this.displayInput.focus({ preventScroll: true });
 
-        if (this.value !== oldValue) {
           // Emit after updating
-          this.updateComplete.then(() => {
+          if (this.value !== oldValue) {
             this.emit('sd-input');
             this.emit('sd-change');
-          });
-        }
+          }
+        });
       }
 
       this.displayInput.focus({ preventScroll: true });
@@ -513,6 +515,7 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
 
   private handleTagKeyDown(event: KeyboardEvent, option: SdOption) {
     if (event.key === 'Backspace' && this.multiple) {
+      event.preventDefault();
       event.stopPropagation();
       this.handleTagRemove(new CustomEvent('sd-remove'), option);
       this.updateComplete.then(() => this.displayInput.focus({ preventScroll: true }));
@@ -521,6 +524,7 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
 
   private handleTagMaxOptionsKeyDown(event: KeyboardEvent) {
     if (event.key === 'Backspace' && this.multiple) {
+      event.preventDefault();
       event.stopPropagation();
       this.handleTagRemove(new CustomEvent('sd-remove'), this.selectedOptions[this.selectedOptions.length - 1]);
       this.updateComplete.then(() => this.displayInput.focus({ preventScroll: true }));
@@ -627,16 +631,16 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
         this.setOrderedSelectedOptions(option);
         this.setSelectedOptions(option);
       }
-      // Set focus after updating so the value is announced by screen readers
-      this.updateComplete.then(() => this.displayInput.focus({ preventScroll: true }));
 
-      if (this.value !== oldValue) {
+      this.updateComplete.then(() => {
+        // Set focus after updating so the value is announced by screen readers
+        this.displayInput.focus({ preventScroll: true });
         // Emit after updating
-        this.updateComplete.then(() => {
+        if (this.value !== oldValue) {
           this.emit('sd-input');
           this.emit('sd-change');
-        });
-      }
+        }
+      });
       if (!this.multiple) {
         this.hide();
         this.displayInput.focus({ preventScroll: true });
@@ -838,6 +842,8 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
 
         clonedOption.current = clonedOption.value === this.lastOption?.value;
         clonedOption.selected = option.selected;
+        clonedOption.checkbox = option.checkbox;
+        clonedOption.size = option.size;
 
         // Check if the option has a sd-optgroup as parent
         const hasOptgroup = option.parentElement?.tagName.toLowerCase() === 'sd-optgroup';
@@ -1081,6 +1087,12 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
 
     const slottedOptions = this.getSlottedOptions();
     const slottedOptgroups = this.getSlottedOptGroups();
+    this.applySizeToOptions();
+
+    // initially set the displayLabel if the value was set via property
+    const selectedOption = this.findOptionByValue(slottedOptions, this.value);
+    this.selectedTextLabel = selectedOption?.getTextLabel() || '';
+
     slottedOptions.forEach((option, index) => {
       if (this.multiple) {
         option.checkbox = true;
@@ -1098,10 +1110,6 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
     } else {
       this.createComboboxOptionsFromQuery('');
     }
-
-    if (this.hasFocus && this.value.length > 0 && !this.open) {
-      await this.show();
-    }
   }
 
   render() {
@@ -1115,7 +1123,7 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
     const hasHelpTextSlot = this.hasSlotController.test('help-text');
     const hasLabel = this.label ? true : !!hasLabelSlot;
     const hasHelpText = this.helpText ? true : !!hasHelpTextSlot;
-    const hasClearIcon = this.clearable && !this.disabled && this.value.length > 0;
+    const hasClearIcon = this.clearable && !this.disabled;
 
     // Hierarchy of input states:
     const selectState = this.disabled
@@ -1283,7 +1291,11 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
                 ? html`
                     <button
                       part="clear-button"
-                      class=${cx('flex justify-center', iconMarginLeft)}
+                      class=${cx(
+                        'flex justify-center',
+                        iconMarginLeft,
+                        this.value.length > 0 ? 'visible' : 'invisible'
+                      )}
                       type="button"
                       aria-label=${this.localize.term('clearEntry')}
                       @mousedown=${this.preventLoosingFocus}
@@ -1404,11 +1416,8 @@ export default class SdCombobox extends SolidElement implements SolidFormControl
       }
 
       sd-tag::part(content) {
+        @apply overflow-hidden whitespace-nowrap inline-block text-ellipsis;
         max-width: var(--tag-max-width, 15ch);
-        overflow: hidden;
-        white-space: nowrap;
-        display: inline-block;
-        text-overflow: ellipsis;
       }
 
       sd-tag[size='lg']::part(base) {

packages/components/src/components/combobox/option-renderer.test.ts

@@ -1,4 +1,5 @@
-import { defaultOptionRenderer, highlightOptionRenderer } from './option-renderer.js';
+import '../../../dist/solid-components';
+import { defaultOptionRenderer, highlightOptionRenderer } from './option-renderer';
 import { expect, fixture, html } from '@open-wc/testing';
 import type SdOption from '../option/option.js';
 
@@ -28,8 +29,8 @@ describe('option-renderer', () => {
       expect(renderedOption.textContent).to.equal('Option 1');
     });
 
-    it.skip('should use <mark> element to highlight the query string', async () => {
-      const option = await fixture<SdOption>(html` <sd-option>Option 1</sd-option> `);
+    it('should use <mark> element to highlight the query string', async () => {
+      const option = await fixture<SdOption>(html`<sd-option>Option 1</sd-option>`);
 
       const query = 'pt';
       expect(option.childNodes[0].textContent).to.equal('Option 1');
@@ -43,7 +44,7 @@ describe('option-renderer', () => {
       expect(renderedOption.childNodes[2].textContent).to.equal('ion 1');
     });
 
-    it.skip('should use the correct capitalization of the option text content for the <mark> text', async () => {
+    it('should use the correct capitalization of the option text content for the <mark> text', async () => {
       const option = await fixture<SdOption>(html` <sd-option>Option 1</sd-option> `);
 
       const query = 'opt';

packages/components/src/components/optgroup/optgroup.test.ts

@@ -1,3 +1,5 @@
+import '../../../dist/solid-components';
+
 import { aTimeout, expect, fixture, html } from '@open-wc/testing';
 
 import type SdOptgroup from './optgroup';
@@ -23,7 +25,7 @@ describe('<sd-optgroup>', () => {
     await expect(el).to.be.accessible();
   });
 
-  it.skip('default properties', async () => {
+  it('default properties', async () => {
     const el = await fixture<SdOptgroup>(html`<sd-optgroup></sd-optgroup>`);
 
     expect(el.disabled).to.be.false;
@@ -44,7 +46,7 @@ describe('<sd-optgroup>', () => {
       expect(getDisabledOptions(el)).to.have.length(0);
     });
 
-    it.skip('changes all <sd-option /> tags disabled attributes to true when sd-optgroups disabled attribute is true', async () => {
+    it('changes all <sd-option /> tags disabled attributes to true when sd-optgroups disabled attribute is true', async () => {
       const el = await fixture<SdOptgroup>(html`
         <sd-optgroup disabled>
           <sd-option value="1">Value 1</sd-option>
@@ -58,7 +60,7 @@ describe('<sd-optgroup>', () => {
       expect(getDisabledOptions(el)).to.have.length(2);
     });
 
-    it.skip('should set its childrens sd-option disabled property according to its own when the slot content changes', async () => {
+    it('should set its childrens sd-option disabled property according to its own when the slot content changes', async () => {
       const el = await fixture<SdOptgroup>(html`
         <sd-optgroup>
           <sd-option value="1">Option 1</sd-option>

packages/components/src/components/select/select.test.ts

@@ -99,7 +99,7 @@ describe('<sd-select>', () => {
     expect(input.hasAttribute('aria-invalid')).to.be.true;
   });
 
-  it.skip('should remove tag and option when tag is focused and backspace is pressed', async () => {
+  it('should remove tag and option when tag is focused and backspace is pressed', async () => {
     const el = await fixture<SdSelect>(html`
       <sd-select value="option-1 option-2" multiple useTags>
         <sd-option value="option-1">Option 1</sd-option>

packages/components/src/components/select/select.ts

@@ -62,6 +62,8 @@ import type SdOption from '../option/option';
  * @csspart tag__removable-indicator - The tag's remove button.
  * @csspart clear-button - The clear button.
  * @csspart expand-icon - The container that wraps the expand icon.
+ *
+ * @cssproperty --tag-max-width - Set the maximum width of the tags and to show an ellipsis. Defaults to "15ch"
  */
 @customElement('sd-select')
 export default class SdSelect extends SolidElement implements SolidFormControl {
@@ -134,6 +136,9 @@ export default class SdSelect extends SolidElement implements SolidFormControl {
   /** Placeholder text to show as a hint when the select is empty. */
   @property() placeholder = this.localize.term('selectDefaultPlaceholder');
 
+  /** Label text shown on tag if max-options-visible is reached. */
+  @property({ attribute: 'max-options-tag-label' }) maxOptionsTagLabel = this.localize.term('tagsSelected');
+
   /** Disables the select control. */
   @property({ type: Boolean, reflect: true }) disabled = false;
 
@@ -412,6 +417,7 @@ export default class SdSelect extends SolidElement implements SolidFormControl {
 
   private handleTagKeyDown(event: KeyboardEvent, option: SdOption) {
     if (event.key === 'Backspace' && this.multiple) {
+      event.preventDefault();
       event.stopPropagation();
       const tagParent = (event.currentTarget as HTMLElement)?.parentElement;
       const previousTag = tagParent?.previousElementSibling?.querySelector('sd-tag');
@@ -432,6 +438,7 @@ export default class SdSelect extends SolidElement implements SolidFormControl {
 
   private handleTagMaxOptionsKeyDown(event: KeyboardEvent) {
     if (event.key === 'Backspace' && this.multiple) {
+      event.preventDefault();
       event.stopPropagation();
       this.handleTagRemove(new CustomEvent('sd-remove'), this.selectedOptions[this.selectedOptions.length - 1]);
       this.updateComplete.then(() => {
@@ -690,7 +697,7 @@ export default class SdSelect extends SolidElement implements SolidFormControl {
             removable
             @keydown=${(event: KeyboardEvent) => this.handleTagMaxOptionsKeyDown(event)}
             @sd-remove=${(event: CustomEvent) => this.handleTagRemove(event)}
-            >${this.selectedOptions.length} ${this.localize.term('tagsSelected')}</sd-tag
+            >${this.selectedOptions.length} ${this.maxOptionsTagLabel}</sd-tag
           >
         `
       ];
@@ -868,7 +875,7 @@ export default class SdSelect extends SolidElement implements SolidFormControl {
 
     const hasLabel = this.label ? true : !!slots['label'];
     const hasHelpText = this.helpText ? true : !!slots['helpText'];
-    const hasClearIcon = this.clearable && !this.disabled && this.value.length > 0;
+    const hasClearIcon = this.clearable && !this.disabled;
 
     // Hierarchy of input states:
     const selectState = this.disabled
@@ -1034,7 +1041,11 @@ export default class SdSelect extends SolidElement implements SolidFormControl {
                 ? html`
                     <button
                       part="clear-button"
-                      class=${cx('select__clear flex justify-center', iconMarginLeft)}
+                      class=${cx(
+                        'select__clear flex justify-center',
+                        iconMarginLeft,
+                        this.value.length > 0 ? 'visible' : 'invisible'
+                      )}
                       type="button"
                       aria-label=${this.localize.term('clearEntry')}
                       @mousedown=${this.handleClearMouseDown}
@@ -1148,6 +1159,11 @@ export default class SdSelect extends SolidElement implements SolidFormControl {
         @apply rounded-default px-1;
       }
 
+      sd-tag::part(content) {
+        @apply overflow-hidden whitespace-nowrap inline-block text-ellipsis;
+        max-width: var(--tag-max-width, 15ch);
+      }
+
       sd-tag[size='lg']::part(base) {
         @apply px-2;
       }

packages/docs/src/stories/components/optgroup.stories.ts

@@ -78,8 +78,7 @@ export const Disabled = {
   render: () => html`
     <div class="h-[260px] w-[400px]">
       <sd-combobox>
-        <sd-optgroup disabled>
-          <span slot="label">Section 1</span>
+        <sd-optgroup label="Section 1" disabled>
           <sd-option value="1">Option</sd-option>
           <sd-option value="2">Option</sd-option>
           <sd-option value="3">Option</sd-option>

packages/docs/src/stories/components/select.stories.ts

@@ -238,6 +238,7 @@ export const Clearable = {
 
 /**
  * Use the `multiple` attribute to allow multiple options to be selected.
+ * Use `--tag-max-width` to set the maximum width of the tags and to show an ellipsis, e.g. `<sd-select style="--tag-max-width: 40px">`. The default value is `15ch`
  */
 
 export const Multiple = {