Dropdown improvements

Author: clintcs

.changeset/chilly-singers-end.md

@@ -0,0 +1,23 @@
+---
+'@crowdstrike/glide-core': patch
+---
+
+### Dropdown
+
+- The tag overflow button is now updated when a selected Dropdown Option is enabled or disabled.
+- Dropdown now reverts its `value` and Dropdown Options revert back to their initial `selected` state when `reset()` is called on the form.
+- Selected Dropdown Options whose `value` is an empty string now appear as selected.
+- Only the last selected option is presented to screenreaders as selected when single-select and multiple options are initially selected.
+- Tags now appear in the order that Dropdown Options are selected by the user.
+- Removing a tag via Enter no longer submits the form.
+- Disabled Dropdown Options are now enabled when selected programmatically.
+
+- When a Dropdown Option is disabled programmatically and more than one selected Dropdown Option has the same `value`, Dropdown now only removes the value corresponding to the disabled Dropdown Option from `value`.
+
+- When `value` is set programmatically, only the first Dropdown Option with a matching value is now selected instead of every Dropdown Option with a matching value.
+
+- Dropdown's input field is now populated with the `label` of the last selected Dropdown Option when filterable and `multiple` is set to `false` programmatically.
+
+### Tab Group
+
+- The trailing overflow button is no longer incorrectly enabled in a rare case.

.changeset/young-plums-brush.md

@@ -0,0 +1,5 @@
+---
+'@crowdstrike/glide-core': minor
+---
+
+- Dropdown Option's `selected` attribute is no longer reflected to match native and to resolve a bug when `reset()` is called on its form. This may be a breaking change for you if you have a mutation observer or CSS selector that targets `selected`.

custom-elements.json

@@ -2221,8 +2221,7 @@
                 "text": "boolean"
               },
               "default": "false",
-              "attribute": "selected",
-              "reflects": true
+              "attribute": "selected"
             },
             {
               "kind": "field",

src/checkbox-group.stories.ts

@@ -212,7 +212,7 @@ const meta: Meta = {
     ) {
       checkboxGroup.reportValidity();
 
-      // `reportValidity` scrolls the element into view, which means the "autodocs"
+      // `reportValidity()` scrolls the element into view, which means the "autodocs"
       // story upon load will be scrolled to the first error story. No good.
       document.documentElement.scrollTop = 0;
 

src/checkbox.stories.ts

@@ -206,7 +206,7 @@ const meta: Meta = {
     ) {
       checkbox.reportValidity();
 
-      // `reportValidity` scrolls the element into view, which means the "autodocs"
+      // `reportValidity()` scrolls the element into view, which means the "autodocs"
       // story upon load will be scrolled to the first error story. No good.
       document.documentElement.scrollTop = 0;
 

src/dropdown.option.ts

@@ -158,7 +158,7 @@ export default class DropdownOption extends LitElement {
   /**
    * @default false
    */
-  @property({ reflect: true, type: Boolean })
+  @property({ type: Boolean })
   get selected(): boolean {
     return this.#selected;
   }
@@ -170,9 +170,6 @@ export default class DropdownOption extends LitElement {
       this.#checkboxElementRef.value.checked = isSelected;
     }
 
-    // Prefixed with "private" because Dropdown uses it internally, to track the set of
-    // selected options in the case of multiselect. Dropdown itself then dispatches in
-    // response to this event a "change" event after updating its `this.value`.
     this.dispatchEvent(new Event('private-selected-change', { bubbles: true }));
   }
 

src/dropdown.stories.ts

@@ -472,7 +472,7 @@ class Component extends LitElement {
     ) {
       dropdown.reportValidity();
 
-      // `reportValidity` scrolls the element into view, which means the "autodocs"
+      // `reportValidity()` scrolls the element into view, which means the "autodocs"
       // story upon load will be scrolled to the first error story. No good.
       document.documentElement.scrollTop = 0;
 

src/dropdown.test.basics.multiple.ts

@@ -35,12 +35,13 @@ it('is accessible', async () => {
   await expect(host).to.be.accessible();
 });
 
-it('sets `value` to that of its selected options', async () => {
+it('can have selected options', async () => {
   const host = await fixture<Dropdown>(
-    html`<glide-core-dropdown label="Label" open multiple>
+    html`<glide-core-dropdown label="Label" multiple>
       <glide-core-dropdown-option
         label="One"
         value="one"
+        selected
       ></glide-core-dropdown-option>
 
       <glide-core-dropdown-option
@@ -52,54 +53,28 @@ it('sets `value` to that of its selected options', async () => {
       <glide-core-dropdown-option
         label="Three"
         value="three"
-        selected
       ></glide-core-dropdown-option>
     </glide-core-dropdown>`,
   );
 
-  expect(host.value).to.deep.equal(['two', 'three']);
-});
-
-it('has selected option labels when options are selected', async () => {
-  const host = await fixture<Dropdown>(
-    html`<glide-core-dropdown label="Label" multiple>
-      <glide-core-dropdown-option
-        label="One"
-        selected
-      ></glide-core-dropdown-option>
-
-      <glide-core-dropdown-option
-        label="Two"
-        selected
-      ></glide-core-dropdown-option>
-    </glide-core-dropdown>`,
-  );
+  const options = host.querySelectorAll('glide-core-dropdown-option');
 
   const labels = host.shadowRoot?.querySelectorAll(
     '[data-test="selected-option-label"]',
   );
 
+  const tags = host.shadowRoot?.querySelectorAll<Tag>('[data-test="tag"]');
+
+  expect(options[0]?.selected).to.be.true;
+  expect(options[1]?.selected).to.be.true;
+  expect(options[2]?.selected).to.be.false;
   expect(labels?.length).to.equal(2);
   expect(labels?.[0]?.textContent?.trim()).to.equal('One,');
   expect(labels?.[1]?.textContent?.trim()).to.equal('Two,');
-});
-
-it('has a tag when an option is selected', async () => {
-  const host = await fixture<Dropdown>(
-    html`<glide-core-dropdown label="Label" multiple>
-      <glide-core-dropdown-option
-        label="One"
-        selected
-      ></glide-core-dropdown-option>
-
-      <glide-core-dropdown-option label="Two"></glide-core-dropdown-option>
-    </glide-core-dropdown>`,
-  );
-
-  const tag = host.shadowRoot?.querySelector<Tag>('[data-test="tag"]');
-
-  expect(tag?.checkVisibility()).to.be.true;
-  expect(tag?.label).to.equal('One');
+  expect(host.value).to.deep.equal(['one', 'two']);
+  expect(tags?.length).to.equal(2);
+  expect(tags?.[0]?.label).to.equal('One');
+  expect(tags?.[1]?.label).to.equal('Two');
 });
 
 it('shows Select All', async () => {
@@ -367,3 +342,56 @@ it('does not include in its `value` disabled options that are selected', async (
 
   expect(host.value).to.deep.equal([]);
 });
+
+it('only selects the first option with a matching value when multiple options have the same value and `value` is set', async () => {
+  const host = await fixture<Dropdown>(
+    html`<glide-core-dropdown label="Label" multiple .value=${['one']}>
+      <glide-core-dropdown-option
+        label="One"
+        value="one"
+      ></glide-core-dropdown-option>
+
+      <glide-core-dropdown-option
+        label="One"
+        value="one"
+      ></glide-core-dropdown-option>
+    </glide-core-dropdown>`,
+  );
+
+  const options = host.querySelectorAll('glide-core-dropdown-option');
+
+  const labels = host.shadowRoot?.querySelectorAll(
+    '[data-test="selected-option-label"]',
+  );
+
+  const tags = host.shadowRoot?.querySelectorAll<Tag>('[data-test="tag"]');
+
+  expect(options[0]?.selected).to.be.true;
+  expect(options[1]?.selected).to.be.false;
+  expect(labels?.length).to.equal(1);
+  expect(labels?.[0]?.textContent?.trim()).to.equal('One,');
+  expect(host.value).to.deep.equal(['one']);
+  expect(tags?.length).to.equal(1);
+  expect(tags?.[0]?.label).to.equal('One');
+});
+
+it('selects multiple options with the same value when `value` is set programmatically', async () => {
+  const host = await fixture<Dropdown>(
+    html`<glide-core-dropdown label="Label" multiple .value=${['one', 'one']}>
+      <glide-core-dropdown-option
+        label="One"
+        value="one"
+      ></glide-core-dropdown-option>
+
+      <glide-core-dropdown-option
+        label="One"
+        value="one"
+      ></glide-core-dropdown-option>
+    </glide-core-dropdown>`,
+  );
+
+  const options = host.querySelectorAll('glide-core-dropdown-option');
+
+  expect(options[0]?.selected).to.be.true;
+  expect(options[1]?.selected).to.be.true;
+});

src/dropdown.test.basics.single.ts

@@ -12,22 +12,40 @@ it('is accessible ', async () => {
   await expect(host).to.be.accessible();
 });
 
-it('has a selected option label when an option is initially selected', async () => {
+it('can have a selected option', async () => {
   const host = await fixture<Dropdown>(
     html`<glide-core-dropdown label="Label">
       <glide-core-dropdown-option
         label="One"
+        value="one"
+        selected
+      ></glide-core-dropdown-option>
+
+      <glide-core-dropdown-option
+        label="Two"
+        value="two"
         selected
       ></glide-core-dropdown-option>
+
+      <glide-core-dropdown-option
+        label="Three"
+        value="three"
+      ></glide-core-dropdown-option>
     </glide-core-dropdown>`,
   );
 
+  const options = host.querySelectorAll('glide-core-dropdown-option');
+
   const labels = host.shadowRoot?.querySelectorAll(
     '[data-test="selected-option-label"]',
   );
 
+  expect(options[0]?.selected).to.be.true;
+  expect(options[1]?.selected).to.be.true;
+  expect(options[2]?.selected).to.be.false;
   expect(labels?.length).to.equal(1);
-  expect(labels?.[0]?.textContent?.trim()).to.equal('One,');
+  expect(labels?.[0]?.textContent?.trim()).to.equal('Two,');
+  expect(host.value).to.deep.equal(['two']);
 });
 
 it('has an Edit button when its last selected option is editable', async () => {

src/dropdown.test.focus.multiple.ts

@@ -105,8 +105,7 @@ it('focuses the second tag when the first one is removed', async () => {
   tags?.[0]?.click();
   await host.updateComplete;
 
-  // Wait for the timeout in `#onTagRemove`.
-  await aTimeout(0);
+  await aTimeout(0); // Wait for the timeout in `#onTagRemove()`
 
   expect(host.shadowRoot?.activeElement).to.equal(tags?.[1]);
 });
@@ -136,8 +135,7 @@ it('focuses the third tag when the second one is removed', async () => {
   tags?.[1]?.click();
   await host.updateComplete;
 
-  // Wait for the timeout in `#onTagRemove`.
-  await aTimeout(0);
+  await aTimeout(0); // Wait for the timeout in `#onTagRemove()`
 
   expect(host.shadowRoot?.activeElement).to.equal(tags?.[2]);
 });
@@ -167,8 +165,7 @@ it('focuses the second tag when the third tag removed', async () => {
   tags?.[2]?.click();
   await host.updateComplete;
 
-  // Wait for the timeout in `#onTagRemove`.
-  await aTimeout(0);
+  await aTimeout(0); // Wait for the timeout in `#onTagRemove()`
 
   expect(host.shadowRoot?.activeElement).to.equal(tags?.[1]);
 });
@@ -186,9 +183,7 @@ it('focuses itself when the last tag is removed', async () => {
   host.shadowRoot?.querySelector<Tag>('[data-test="tag"]')?.click();
 
   await host.updateComplete;
-
-  // Wait for the timeout in `#onTagRemove`.
-  await aTimeout(0);
+  await aTimeout(0); // Wait for the timeout in `#onTagRemove()`
 
   expect(document.activeElement).to.equal(host);
 });