Improve keyboard logic

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Fixed a bug where swapping an animated element wouldn't restart the animation in `sl-animation`
 - Fixed a bug where the cursor was incorrect when `sl-select` was disabled
 - Fixed a bug where clicking on `sl-menu` wouldn't focus it
+- Improved keyboard logic in `sl-dropdown`, `sl-menu`, and `sl-select`
 - Updated `sl-animation` to stable
 - Updated to Stencil 2.0 (you may need to purge `node_modules` and run `npm install` after pulling)
 - Updated entry points in `package.json` to reflect new filenames generated by Stencil 2

src/components/dropdown/dropdown.tsx

@@ -109,8 +109,8 @@ export class Dropdown {
     this.handleDocumentMouseDown = this.handleDocumentMouseDown.bind(this);
     this.handleMenuItemActivate = this.handleMenuItemActivate.bind(this);
     this.handlePanelSelect = this.handlePanelSelect.bind(this);
+    this.handleTriggerClick = this.handleTriggerClick.bind(this);
     this.handleTriggerKeyDown = this.handleTriggerKeyDown.bind(this);
-    this.togglePanel = this.togglePanel.bind(this);
   }
 
   componentDidLoad() {
@@ -156,8 +156,8 @@ export class Dropdown {
 
     this.panel.addEventListener('slActivate', this.handleMenuItemActivate);
     this.panel.addEventListener('slSelect', this.handlePanelSelect);
-    document.addEventListener('mousedown', this.handleDocumentMouseDown);
     document.addEventListener('keydown', this.handleDocumentKeyDown);
+    document.addEventListener('mousedown', this.handleDocumentMouseDown);
 
     this.isShowing = true;
     this.open = true;
@@ -180,8 +180,8 @@ export class Dropdown {
 
     this.panel.removeEventListener('slActivate', this.handleMenuItemActivate);
     this.panel.removeEventListener('slSelect', this.handlePanelSelect);
+    document.addEventListener('keydown', this.handleDocumentKeyDown);
     document.removeEventListener('mousedown', this.handleDocumentMouseDown);
-    document.removeEventListener('keydown', this.handleDocumentKeyDown);
 
     this.isShowing = false;
     this.open = false;
@@ -208,8 +208,6 @@ export class Dropdown {
   }
 
   handleDocumentKeyDown(event: KeyboardEvent) {
-    const menu = this.getMenu();
-
     // Close when escape is pressed
     if (event.key === 'Escape') {
       this.hide();
@@ -217,9 +215,10 @@ export class Dropdown {
       return;
     }
 
-    // Close when tabbing results in the focus leaving the containing element
+    // Handle tabbing
     if (event.key === 'Tab') {
       setTimeout(() => {
+        // Tabbing outside of the containing element closes the panel
         if (
           document.activeElement &&
           document.activeElement.closest(this.containingElement.tagName.toLowerCase()) !== this.containingElement
@@ -229,17 +228,6 @@ export class Dropdown {
         }
       });
     }
-
-    // Prevent the page from scrolling when certain keys are pressed
-    if (['ArrowDown', 'ArrowUp', 'Home', 'End'].includes(event.key)) {
-      event.preventDefault();
-    }
-
-    // If a menu is present, focus on it when certain keys are pressed
-    if (menu && ['ArrowDown', 'ArrowUp'].includes(event.key)) {
-      event.preventDefault();
-      menu.setFocus();
-    }
   }
 
   handleDocumentMouseDown(event: MouseEvent) {
@@ -266,27 +254,55 @@ export class Dropdown {
     }
   }
 
+  handleTriggerClick() {
+    this.open ? this.hide() : this.show();
+  }
+
   handleTriggerKeyDown(event: KeyboardEvent) {
-    // Open the panel when pressing down or up while focused on the trigger
-    if (!this.open && ['ArrowDown', 'ArrowUp'].includes(event.key)) {
-      this.show();
-      event.preventDefault();
-      event.stopPropagation();
+    const menu = this.getMenu();
+
+    // Close when escape or tab is pressed
+    if (event.key === 'Escape') {
+      this.hide();
+      this.focusOnTrigger();
+      return;
     }
 
-    // All other keys focus the menu and initiate type-to-select
-    const menu = this.getMenu();
-    if (menu && event.target !== menu) {
+    // When spacebar/enter is pressed, show the panel but don't focus on the menu. This let's the user press the same
+    // key again to hide the menu in case they don't want to make a selection.
+    if ([' ', 'Enter'].includes(event.key)) {
+      event.preventDefault();
+      this.open ? this.hide() : this.show();
+      return;
+    }
+
+    // When up/down is pressed, we make the assumption that the user is familiar with the menu and plans to make a
+    // selection. Rather than toggle the panel, we focus on the menu (if one exists) and activate the first item for
+    // faster navigation.
+    if (['ArrowDown', 'ArrowUp'].includes(event.key)) {
+      event.preventDefault();
+
+      // Show the menu if it's not already open
+      if (!this.open) {
+        this.show();
+      }
+
+      // Focus on the menu, if one exists
+      if (menu) {
+        menu.setFocus();
+        return;
+      }
+    }
+
+    // Other keys bring focus to the menu and initiate type-to-select behavior
+    const ignoredKeys = ['Tab', 'Shift', 'Meta', 'Ctrl', 'Alt'];
+    if (this.open && menu && !ignoredKeys.includes(event.key)) {
       menu.setFocus();
       menu.typeToSelect(event.key);
       return;
     }
   }
 
-  togglePanel() {
-    this.open ? this.hide() : this.show();
-  }
-
   render() {
     return (
       <div
@@ -303,8 +319,8 @@ export class Dropdown {
           part="trigger"
           class="dropdown__trigger"
           ref={el => (this.trigger = el)}
+          onClick={this.handleTriggerClick}
           onKeyDown={this.handleTriggerKeyDown}
-          onClick={this.togglePanel}
         >
           <slot name="trigger" />
         </span>

src/components/menu/menu.tsx

@@ -93,11 +93,14 @@ export class Menu {
   }
 
   handleFocus() {
-    const item = this.getActiveItem();
-    if (!item) {
-      this.setActiveItem(this.getItems()[0]);
-    }
     this.slFocus.emit();
+
+    // Activate the first item if no other item is active
+    const activeItem = this.getActiveItem();
+    if (!activeItem) {
+      const items = this.getItems();
+      this.setActiveItem(items[0]);
+    }
   }
 
   handleBlur() {

src/components/select/select.tsx

@@ -122,7 +122,7 @@ export class Select {
     this.handleKeyUp = this.handleKeyUp.bind(this);
     this.handleLabelClick = this.handleLabelClick.bind(this);
     this.handleTagClick = this.handleTagClick.bind(this);
-    this.handleMenuKeyDown = this.handleMenuKeyDown.bind(this);
+    this.handleTagKeyDown = this.handleTagKeyDown.bind(this);
     this.handleMenuHide = this.handleMenuHide.bind(this);
     this.handleMenuShow = this.handleMenuShow.bind(this);
     this.handleMenuSelect = this.handleMenuSelect.bind(this);
@@ -179,16 +179,7 @@ export class Select {
     this.dropdown.hide();
   }
 
-  handleKeyDown(event: KeyboardEvent) {
-    const target = event.target as HTMLElement;
-
-    // Open the dropdown when enter is pressed while the input is focused
-    if (!this.isOpen && event.key === 'Enter' && target === this.input) {
-      this.dropdown.show();
-      event.preventDefault();
-      return;
-    }
-
+  handleKeyDown() {
     // We can't make the <sl-input> readonly since that will block the browser's validation messages, so this prevents
     // key presses from modifying the input's value by briefly making it readonly. We don't use `preventDefault()` since
     // that would block tabbing, shortcuts, etc.
@@ -213,15 +204,6 @@ export class Select {
     this.input.setFocus();
   }
 
-  handleMenuKeyDown(event: KeyboardEvent) {
-    // Close when escape or tab pressed
-    if (event.key === 'Escape' || event.key === 'Tab') {
-      this.dropdown.hide();
-      event.preventDefault();
-      return;
-    }
-  }
-
   handleMenuSelect(event: CustomEvent) {
     const item = event.detail.item;
 
@@ -273,6 +255,12 @@ export class Select {
     }
   }
 
+  handleTagKeyDown(event: KeyboardEvent) {
+    if (event.key === 'Enter') {
+      event.stopPropagation();
+    }
+  }
+
   reportDuplicateItemValues() {
     const items = this.getItems();
 
@@ -308,6 +296,7 @@ export class Select {
             pill={this.pill}
             clearable
             onClick={this.handleTagClick}
+            onKeyDown={this.handleTagKeyDown}
             onSlClear={event => {
               event.stopPropagation();
               item.checked = false;
@@ -436,13 +425,7 @@ export class Select {
             </span>
           </sl-input>
 
-          <sl-menu
-            ref={el => (this.menu = el)}
-            part="menu"
-            class="select__menu"
-            onSlSelect={this.handleMenuSelect}
-            onKeyDown={this.handleMenuKeyDown}
-          >
+          <sl-menu ref={el => (this.menu = el)} part="menu" class="select__menu" onSlSelect={this.handleMenuSelect}>
             <slot onSlotchange={this.handleSlotChange} />
           </sl-menu>
         </sl-dropdown>