From 84d4421575c530f9145bea88b77cf06591bcf698 Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Thu, 3 Sep 2020 17:44:52 -0400 Subject: [PATCH] Improve keyboard logic --- CHANGELOG.md | 1 + src/components/dropdown/dropdown.tsx | 76 +++++++++++++++++----------- src/components/menu/menu.tsx | 11 ++-- src/components/select/select.tsx | 37 ++++---------- 4 files changed, 64 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33d835c4..a9c4c3f9 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/components/dropdown/dropdown.tsx b/src/components/dropdown/dropdown.tsx index ae88e5a9..13032795 100644 --- a/src/components/dropdown/dropdown.tsx +++ b/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 (
(this.trigger = el)} + onClick={this.handleTriggerClick} onKeyDown={this.handleTriggerKeyDown} - onClick={this.togglePanel} > diff --git a/src/components/menu/menu.tsx b/src/components/menu/menu.tsx index 4bbbc6a9..701c2b6b 100644 --- a/src/components/menu/menu.tsx +++ b/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() { diff --git a/src/components/select/select.tsx b/src/components/select/select.tsx index 1735515b..61343ef0 100644 --- a/src/components/select/select.tsx +++ b/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 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 { - (this.menu = el)} - part="menu" - class="select__menu" - onSlSelect={this.handleMenuSelect} - onKeyDown={this.handleMenuKeyDown} - > + (this.menu = el)} part="menu" class="select__menu" onSlSelect={this.handleMenuSelect}>