fix dropdown keyboard controls; closes #1177

docs/resources/changelog.md

@@ -15,6 +15,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 - Fixed a bug in the template for `<sl-select>` that caused the `form-control-help-text` part to not be in the same location as other form controls [#1178](https://github.com/shoelace-style/shoelace/issues/1178)
 - Fixed a bug in `<sl-checkbox>` and `<sl-switch>` that caused the browser to scroll incorrectly when focusing on a control in a container with overflow [#1169](https://github.com/shoelace-style/shoelace/issues/1169)
 - Fixed a bug in `<sl-menu-item>` that caused the `click` event to be emitted when the item was disabled [#1113](https://github.com/shoelace-style/shoelace/issues/1113)
+- Improved the behavior of `<sl-dropdown>` in Safari so keyboard interaction works the same as in other browsers [#1177](https://github.com/shoelace-style/shoelace/issues/1177)
 
 ## 2.0.0
 

src/components/dropdown/dropdown.test.ts

@@ -1,4 +1,5 @@
-import { aTimeout, expect, fixture, html, waitUntil } from '@open-wc/testing';
+import { clickOnElement } from '../../internal/test';
+import { expect, fixture, html, waitUntil } from '@open-wc/testing';
 import { sendKeys, sendMouse } from '@web/test-runner-commands';
 import sinon from 'sinon';
 import type SlDropdown from './dropdown';
@@ -192,14 +193,12 @@ describe('<sl-dropdown>', () => {
     const trigger = el.querySelector('sl-button')!;
     const item = el.querySelector('sl-menu-item')!;
 
-    trigger.focus();
+    await clickOnElement(trigger);
     await trigger.updateComplete;
     await sendKeys({ press: 'ArrowDown' });
     await el.updateComplete;
-    await aTimeout(500); // sigh, Safari
-    const itemFocused = document.activeElement === item;
 
-    expect(itemFocused).to.be.true;
+    expect(document.activeElement).to.equal(item);
   });
 
   it('should close on escape key', async () => {
@@ -256,6 +255,30 @@ describe('<sl-dropdown>', () => {
     expect(el.open).to.be.true;
   });
 
+  it('should focus on menu items when clicking the trigger and arrowing through options', async () => {
+    const el = await fixture<SlDropdown>(html`
+      <sl-dropdown>
+        <sl-button slot="trigger" caret>Toggle</sl-button>
+        <sl-menu>
+          <sl-menu-item>Item 1</sl-menu-item>
+          <sl-menu-item>Item 2</sl-menu-item>
+          <sl-menu-item>Item 3</sl-menu-item>
+        </sl-menu>
+      </sl-dropdown>
+    `);
+    const trigger = el.querySelector('sl-button')!;
+    const secondMenuItem = el.querySelectorAll('sl-menu-item')[1];
+
+    await clickOnElement(trigger);
+    await trigger.updateComplete;
+    await sendKeys({ press: 'ArrowDown' });
+    await el.updateComplete;
+    await sendKeys({ press: 'ArrowDown' });
+    await el.updateComplete;
+
+    expect(document.activeElement).to.equal(secondMenuItem);
+  });
+
   it('should open on enter key when no menu exists', async () => {
     const el = await fixture<SlDropdown>(html`
       <sl-dropdown>

src/components/dropdown/dropdown.ts

@@ -253,7 +253,7 @@ export default class SlDropdown extends ShoelaceElement {
 
         if (menuItems.length > 0) {
           // Focus on the first/last menu item after showing
-          requestAnimationFrame(() => {
+          this.updateComplete.then(() => {
             if (event.key === 'ArrowDown' || event.key === 'Home') {
               menu.setCurrentItem(firstMenuItem);
               firstMenuItem.focus();
@@ -319,6 +319,7 @@ export default class SlDropdown extends ShoelaceElement {
     }
 
     this.open = true;
+    this.focusOnTrigger();
     return waitForEvent(this, 'sl-after-show');
   }
 

src/components/menu-item/menu-item.styles.ts

@@ -69,7 +69,7 @@ export default css`
     color: var(--sl-color-neutral-1000);
   }
 
-  :host(:focus-visible) .menu-item {
+  :host(:focus) .menu-item {
     outline: none;
     background-color: var(--sl-color-primary-600);
     color: var(--sl-color-neutral-0);
@@ -93,7 +93,7 @@ export default css`
 
   @media (forced-colors: active) {
     :host(:hover:not([aria-disabled='true'])) .menu-item,
-    :host(:focus-visible) .menu-item {
+    :host(:focus) .menu-item {
       outline: dashed 1px SelectedItem;
       outline-offset: -1px;
     }