fix in screen readers

2022-12-19 17:46:31 -05:00 · 2022-12-19 17:46:31 -05:00 · f28a0ec743
commit f28a0ec743
--- a/docs/components/select.md
+++ b/docs/components/select.md
@ -3,7 +3,7 @@
 [component-header:sl-select]

 ```html preview
-<sl-select>
+<sl-select value="option-1">
  <sl-option value="option-1">Option 1</sl-option>
  <sl-option value="option-2">Option 2</sl-option>
  <sl-option value="option-3">Option 3</sl-option>
--- a/src/components/option/option.styles.ts
+++ b/src/components/option/option.styles.ts
@ -9,6 +9,10 @@ export default css`
    user-select: none;
  }

+  :host(:focus) {
+    outline: none;
+  }
+
  .option {
    position: relative;
    display: flex;
@ -22,21 +26,20 @@ export default css`
    padding: var(--sl-spacing-2x-small) var(--sl-spacing-2x-small);
    transition: var(--sl-transition-fast) fill;
    user-select: none;
-    white-space: nowrap;
    cursor: pointer;
  }

-  :host(:hover) .option {
+  :host(:hover) .option:not(.option--current) {
    background-color: var(--sl-color-neutral-100);
    color: var(--sl-color-neutral-1000);
  }

-  :host([aria-selected='true']) .option {
+  .option--current {
    background-color: var(--sl-color-primary-600);
    color: var(--sl-color-neutral-0);
  }

-  .option.option--disabled {
+  .option--disabled {
    outline: none;
    opacity: 0.5;
    cursor: not-allowed;
@ -45,7 +48,20 @@ export default css`
  .option__label {
    flex: 1 1 auto;
    display: inline-block;
-    padding: 0 var(--sl-spacing-large);
+    padding: 0 var(--sl-spacing-2x-small);
+  }
+
+  .option .option__check {
+    flex: 0 0 auto;
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    width: 1.5em;
+    visibility: hidden;
+  }
+
+  .option--selected .option__check {
+    visibility: visible;
  }

  .option__prefix {
--- a/src/components/option/option.ts
+++ b/src/components/option/option.ts
@ -1,7 +1,10 @@
 import { html } from 'lit';
 import { customElement, property } from 'lit/decorators.js';
+import { classMap } from 'lit/directives/class-map.js';
 import ShoelaceElement from '../../internal/shoelace-element';
+import { watch } from '../../internal/watch';
 import { LocalizeController } from '../../utilities/localize';
+import '../icon/icon';
 import styles from './option.styles';
 import type { CSSResultGroup } from 'lit';

@ -31,6 +34,12 @@ export default class SlOption extends ShoelaceElement {
  /** The option's value. When selected, the containing form control will receive this value. */
  @property() value = '';

+  /** Draws the option in a current state, meaning the user has keyed into it but hasn't selected it yet. */
+  @property({ type: Boolean, reflect: true }) current = false;
+
+  /** Draws the option in a selected state. */
+  @property({ type: Boolean, reflect: true }) selected = false;
+
  /** Draws the option in a disabled state, preventing selection. */
  @property({ type: Boolean, reflect: true }) disabled = false;

@ -40,9 +49,28 @@ export default class SlOption extends ShoelaceElement {
    this.setAttribute('aria-selected', 'false');
  }

+  @watch('disabled')
+  handleDisabledChange() {
+    this.setAttribute('aria-disabled', this.disabled ? 'true' : 'false');
+  }
+
+  @watch('selected')
+  handleSelectedChange() {
+    this.setAttribute('aria-selected', this.selected ? 'true' : 'false');
+  }
+
  render() {
    return html`
-      <div class="option">
+      <div
+        class=${classMap({
+          option: true,
+          'option--current': this.current,
+          'option--selected': this.selected
+        })}
+      >
+        <span part="checked-icon" class="option__check">
+          <sl-icon name="check" library="system" aria-hidden="true"></sl-icon>
+        </span>
        <slot name="prefix" class="option__prefix"></slot>
        <slot class="option__label"></slot>
        <slot name="suffix" class="option__suffix"></slot>
--- a/src/components/select/select.styles.ts
+++ b/src/components/select/select.styles.ts
@ -34,6 +34,7 @@ export default css`
    flex: 1 0 auto;
    display: flex;
    width: 100%;
+    min-width: 0;
    font-family: var(--sl-input-font-family);
    font-weight: var(--sl-input-font-weight);
    letter-spacing: var(--sl-input-letter-spacing);
@ -45,10 +46,11 @@ export default css`
  }

  .select__combobox {
-    flex: 1 0 auto;
+    flex: 1;
    display: flex;
    align-items: stretch;
    justify-content: start;
+    overflow: hidden;
  }

  .select__combobox:focus {
@ -159,12 +161,18 @@ export default css`
    border-radius: var(--sl-input-height-large);
  }

-  /* Display label */
-  .select__display-label {
-    flex: 1 1 auto;
+  /* Display label (uses a wrapper to allow vertical centering with flex + text truncation on the label) */
+  .select__display-label-wrapper {
+    flex: 1 0 auto;
    display: flex;
    align-items: center;
-    user-select: none;
+    width: 100%;
+  }
+
+  .select__display-label {
+    white-space: nowrap;
+    overflow: hidden;
+    text-overflow: ellipsis;
  }

  .select--placeholder-visible .select__display-label {
--- a/src/components/select/select.ts
+++ b/src/components/select/select.ts
@ -174,28 +174,7 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
      event.stopPropagation();
      this.hide();
    }
-  }

-  handleDocumentMouseDown(event: MouseEvent) {
-    // Close when clicking outside of the select
-    const path = event.composedPath();
-    if (this && !path.includes(this)) {
-      this.hide();
-    }
-  }
-
-  handleLabelClick() {
-    this.combobox.focus();
-  }
-
-  // We use mousedown/mouseup instead of click to allow macOS-style menu behavior
-  handleComboboxMouseDown(event: MouseEvent) {
-    event.preventDefault();
-    this.combobox.focus();
-    this.open = !this.open;
-  }
-
-  handleComboboxKeyDown(event: KeyboardEvent) {
    // Handle enter and space. When pressing space, we allow for type to select behaviors so if there's anything in the
    // buffer we _don't_ close it.
    if (event.key === 'Enter' || (event.key === ' ' && this.typeToSelectString === '')) {
@ -208,10 +187,11 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
      }

      // If it is open, update the value based on the current selection and close it
-      const selectedOption = this.getSelectedOption();
-      if (selectedOption) {
-        this.value = selectedOption.value;
-        this.displayLabel = selectedOption.textContent ?? '';
+      const currentOption = this.getCurrentOption();
+      if (currentOption) {
+        this.setSelectedOption(currentOption);
+        this.value = currentOption.value;
+        this.displayLabel = currentOption.textContent ?? '';
      }

      this.hide();
@ -222,9 +202,9 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
    // Navigate options
    if (['ArrowUp', 'ArrowDown', 'Home', 'End'].includes(event.key)) {
      const allOptions = this.getAllOptions();
-      const selectedOption = this.getSelectedOption();
-      const selectedIndex = allOptions.indexOf(selectedOption);
-      let newIndex = Math.max(0, selectedIndex);
+      const currentOption = this.getCurrentOption();
+      const currentIndex = allOptions.indexOf(currentOption);
+      let newIndex = Math.max(0, currentIndex);

      // Prevent scrolling
      event.preventDefault();
@ -235,16 +215,16 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon

        // If an option is already selected, stop here because we want that one to remain highlighted when the listbox
        // opens for the first time
-        if (selectedOption) {
+        if (currentOption) {
          return;
        }
      }

      if (event.key === 'ArrowDown') {
-        newIndex = selectedIndex + 1;
+        newIndex = currentIndex + 1;
        if (newIndex > allOptions.length - 1) newIndex = 0;
      } else if (event.key === 'ArrowUp') {
-        newIndex = selectedIndex - 1;
+        newIndex = currentIndex - 1;
        if (newIndex < 0) newIndex = allOptions.length - 1;
      } else if (event.key === 'Home') {
        newIndex = 0;
@ -252,7 +232,7 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
        newIndex = allOptions.length - 1;
      }

-      this.setSelectedOption(allOptions[newIndex]);
+      this.setCurrentOption(allOptions[newIndex]);
    }

    // All other "printable" keys trigger type to select
@ -291,13 +271,37 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
        const label = (option.textContent ?? '').toLowerCase();

        if (label.startsWith(this.typeToSelectString)) {
-          this.setSelectedOption(option);
+          this.setCurrentOption(option);
          break;
        }
      }
    }
  }

+  handleDocumentMouseDown(event: MouseEvent) {
+    // Close when clicking outside of the select
+    const path = event.composedPath();
+    if (this && !path.includes(this)) {
+      this.hide();
+    }
+  }
+
+  handleLabelClick() {
+    this.combobox.focus();
+  }
+
+  // We use mousedown/mouseup instead of click to allow macOS-style menu behavior
+  handleComboboxMouseDown(event: MouseEvent) {
+    event.preventDefault();
+    this.combobox.focus();
+    this.open = !this.open;
+  }
+
+  handleComboboxKeyDown(event: KeyboardEvent) {
+    event.stopPropagation();
+    this.handleDocumentKeyDown(event);
+  }
+
  handleClearClick(event: MouseEvent) {
    event.stopPropagation();

@ -359,21 +363,45 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
    return this.getAllOptions().filter((el: SlOption) => el.value === value)[0];
  }

-  // Gets the option that currently has aria-selected="true"
-  getSelectedOption() {
-    return this.getAllOptions().filter(el => el.getAttribute('aria-selected') === 'true')[0];
+  // Gets the current option
+  getCurrentOption() {
+    return this.getAllOptions().filter(el => el.current)[0];
  }

-  // Adds aria-selected to the target option and removes it from all others
+  // Sets the current option
+  setCurrentOption(option: SlOption | null) {
+    const allOptions = this.getAllOptions();
+
+    // Clear selection
+    allOptions.forEach(el => {
+      el.current = false;
+      el.tabIndex = -1;
+    });
+
+    // Select the target option
+    if (option) {
+      option.current = true;
+      option.tabIndex = 0;
+      option.focus();
+      scrollIntoView(option, this.listbox);
+    }
+  }
+
+  // Gets the selected option
+  getSelectedOption() {
+    return this.getAllOptions().filter(el => el.selected)[0];
+  }
+
+  // Sets the selected option
  setSelectedOption(option: SlOption | null) {
    const allOptions = this.getAllOptions();

    // Clear selection
-    allOptions.forEach(el => el.setAttribute('aria-selected', 'false'));
+    allOptions.forEach(el => (el.selected = false));

    // Select the target option
    if (option) {
-      option.setAttribute('aria-selected', 'true');
+      option.selected = true;
      scrollIntoView(option, this.listbox);
    }
  }
@ -445,12 +473,15 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
      this.listbox.hidden = false;
      this.popup.active = true;

-      // Make sure the current option is selected
-      this.setSelectedOption(this.getOptionByValue(this.value));
+      // Select the correct option
+      const currentOption = this.getOptionByValue(this.value);
+      this.setCurrentOption(currentOption);
+      this.setSelectedOption(currentOption);

      // Scroll the selected option into view
      requestAnimationFrame(() => {
        const selectedOption = this.getSelectedOption();
+
        if (selectedOption) {
          //
          // TODO - improve this logic so the selected option is centered in the listbox instead of at the top
@ -555,9 +586,11 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon
                @blur=${this.handleBlur}
              >
                <slot name="prefix" class="select__prefix"></slot>
-                <span class="select__display-label"
-                  >${isPlaceholderVisible ? this.placeholder : this.displayLabel}</span
-                >
+                <span class="select__display-label-wrapper">
+                  <span class="select__display-label">
+                    ${isPlaceholderVisible ? this.placeholder : this.displayLabel}
+                  </span>
+                </span>
              </div>

              ${hasClearIcon
@ -587,7 +620,9 @@ export default class SlSelect extends ShoelaceElement implements ShoelaceFormCon

            <slot
              id="listbox"
+              role="listbox"
              aria-expanded=${this.open ? 'true' : 'false'}
+              aria-multiselectable="false"
              aria-labelledby="label"
              part="panel"
              class="select__listbox"