fix(super-editor): restore toolbar dropdown keyboard focus

packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.test.js

@@ -1,8 +1,10 @@
-import { describe, it, expect } from 'vitest';
-import { shallowMount } from '@vue/test-utils';
-import { h, ref } from 'vue';
+import { afterEach, describe, it, expect } from 'vitest';
+import { mount, shallowMount } from '@vue/test-utils';
+import { h, nextTick, ref } from 'vue';
 import ButtonGroup from './ButtonGroup.vue';
 
+const waitForAnimationFrame = () => new Promise((resolve) => requestAnimationFrame(resolve));
+
 const createDropdownItem = (selectedKey) => ({
   type: 'dropdown',
   id: ref('btn-test'),
@@ -154,3 +156,162 @@ describe('ButtonGroup button argument forwarding', () => {
     expect(wrapper.emitted('command')).toBeUndefined();
   });
 });
+
+describe('ButtonGroup dropdown keyboard activation', () => {
+  it.each(['Enter', ' ', 'Spacebar'])('opens a dropdown item with %s', async (key) => {
+    const item = createDropdownItem('plain-match');
+    const wrapper = mountWithItem(item);
+
+    await wrapper.find('.toolbar-item-ctn').trigger('keydown', { key });
+
+    expect(item.expand.value).toBe(true);
+  });
+});
+
+// Regression for the codex P2 finding on PR #3304: after Escape closes the
+// dropdown, ToolbarDropdown.rememberTriggerFocusTarget restores focus to the
+// inner `.toolbar-item` (ToolbarButton root, role="button", tabindex="0"),
+// not to `.toolbar-item-ctn`. ToolbarButton used to handle Enter with
+// `@keydown.enter.stop`, which silently swallowed the event before
+// ButtonGroup's roving-tabindex handler could see it. Pressing Enter on the
+// restored focus would emit `buttonClick` (no listener on the dropdown
+// branch) and do nothing, so the dropdown could never be reopened by
+// keyboard until focus moved elsewhere.
+//
+// Fix is the `allowEnterPropagation` prop on ToolbarButton: when true the
+// keydown handler does NOT stopPropagation, so Enter bubbles to
+// `.toolbar-item-ctn` and ButtonGroup.activateToolbarItem runs.
+// Note: this only applies to non-split dropdown items. Split buttons
+// (bullet list / numbered list main button) call handleSplitMainClick on
+// Enter which itself stops propagation and runs the main command instead.
+describe('ButtonGroup dropdown trigger keyboard activation (codex P2 regression)', () => {
+  let wrapper;
+
+  afterEach(() => {
+    wrapper?.unmount();
+    wrapper = null;
+    document.body.innerHTML = '';
+  });
+
+  // `mount()` renders the real ToolbarButton, which destructures more
+  // refs off the toolbar item than `createDropdownItem` above provides.
+  // Build a fuller item with every ref ToolbarButton expects.
+  const createFullDropdownItem = (selectedKey = 'plain-match') => ({
+    ...createDropdownItem(selectedKey),
+    active: ref(false),
+    icon: ref(null),
+    label: ref('Test'),
+    hideLabel: ref(false),
+    iconColor: ref(null),
+    hasCaret: ref(true),
+    splitButton: ref(false),
+    inlineTextInputVisible: ref(false),
+    hasInlineTextInput: ref(false),
+    minWidth: ref(null),
+    style: ref(null),
+  });
+
+  // Real ToolbarButton (no stub) inside the dropdown branch so we exercise
+  // the actual @keydown.enter handler + allowEnterPropagation plumbing.
+  const mountWithDropdownItem = (item) =>
+    mount(ButtonGroup, {
+      props: { toolbarItems: [item], overflowItems: [] },
+      attachTo: document.body,
+      global: {
+        stubs: {
+          // Render SdTooltip's trigger slot so the real ToolbarButton mounts.
+          SdTooltip: { name: 'SdTooltip', template: '<div><slot name="trigger" /></div>' },
+        },
+      },
+    });
+
+  it('Enter on the inner .toolbar-item bubbles up and opens the dropdown', async () => {
+    const item = createFullDropdownItem('plain-match');
+    wrapper = mountWithDropdownItem(item);
+
+    const innerItem = wrapper.find('.toolbar-dropdown-trigger .toolbar-item').element;
+    expect(innerItem.getAttribute('tabindex')).toBe('0');
+    expect(innerItem.getAttribute('role')).toBe('button');
+
+    innerItem.focus();
+    expect(document.activeElement).toBe(innerItem);
+
+    innerItem.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }));
+    await nextTick();
+
+    expect(item.expand.value).toBe(true);
+  });
+
+  it('after Escape closes the dropdown, a second Enter on the restored focus reopens it', async () => {
+    const item = createFullDropdownItem('plain-match');
+    wrapper = mountWithDropdownItem(item);
+
+    const ctn = wrapper.find('.toolbar-item-ctn').element;
+    const innerItem = wrapper.find('.toolbar-dropdown-trigger .toolbar-item').element;
+
+    // Open the dropdown the way Tab + Enter does (focus on ctn).
+    ctn.focus();
+    ctn.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }));
+    await nextTick();
+    await nextTick();
+    expect(item.expand.value).toBe(true);
+
+    // Escape closes and ToolbarDropdown restores focus to the inner item.
+    document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
+    await nextTick();
+    await waitForAnimationFrame();
+    expect(item.expand.value).toBe(false);
+    expect(document.activeElement).toBe(innerItem);
+
+    // Enter on the inner item must reopen the dropdown (the bug previously
+    // left it closed because ToolbarButton's @keydown.enter.stop swallowed
+    // the event before ButtonGroup could handle it).
+    innerItem.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }));
+    await nextTick();
+    expect(item.expand.value).toBe(true);
+  });
+
+  it('Space on the inner .toolbar-item also opens the dropdown (control)', async () => {
+    const item = createFullDropdownItem('plain-match');
+    wrapper = mountWithDropdownItem(item);
+
+    const innerItem = wrapper.find('.toolbar-dropdown-trigger .toolbar-item').element;
+    innerItem.focus();
+    innerItem.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', bubbles: true }));
+    await nextTick();
+
+    expect(item.expand.value).toBe(true);
+  });
+
+  // Pin existing split-button behavior so it does not silently change.
+  // Bullet/Numbered list main buttons are split: Enter runs the main
+  // command (mainClick) and does NOT toggle the dropdown. The
+  // `allowEnterPropagation` flag has no effect because handleSplitMainClick
+  // stops propagation internally.
+  it('split button: Enter on inner runs the main command and does NOT open the dropdown', async () => {
+    const item = {
+      ...createFullDropdownItem('plain-match'),
+      splitButton: ref(true),
+      hasCaret: ref(true),
+      // ButtonGroup.handleSplitButtonMainClick uses these as plain
+      // properties (not refs) to choose which command to emit.
+      splitButtonCommand: 'toggleBulletList',
+    };
+    wrapper = mountWithDropdownItem(item);
+
+    const innerItem = wrapper.find('.toolbar-dropdown-trigger .toolbar-item').element;
+    innerItem.focus();
+    innerItem.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }));
+    await nextTick();
+
+    // Dropdown must stay closed.
+    expect(item.expand.value).toBe(false);
+    // ButtonGroup emits 'command' with the split-main command, not the
+    // dropdown's selected option - so the rest of the editor runs the
+    // main action (e.g. toggleBulletList) on this keystroke.
+    const events = wrapper.emitted('command');
+    expect(events).toHaveLength(1);
+    expect(events[0][0].item.command).toBe('toggleBulletList');
+    expect(events[0][0].argument).toBeNull();
+  });
+});

packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.vue

@@ -227,6 +227,17 @@ const moveToPreviousButtonGroup = (e) => {
   }
 };
 
+const activateToolbarItem = (item) => {
+  if (item.disabled.value) return;
+
+  if (isDropdown(item)) {
+    handleDropdownUpdateShowForItem(!getExpanded(item), item);
+    return;
+  }
+
+  handleToolbarButtonClick(item, null, false);
+};
+
 // Implement keyboard navigation using Roving Tabindex
 // https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#kbd_roving_tabindex
 // Set tabindex to 0 for the current focused button
@@ -239,11 +250,16 @@ const handleKeyDown = (e, item) => {
   if (isTypingField && isTypingToolbarItem) {
     return;
   }
+
+  const handledKeys = ['Enter', ' ', 'Spacebar', 'Escape', 'ArrowRight', 'ArrowLeft', 'Tab'];
+  if (!handledKeys.includes(e.key)) return;
   e.preventDefault();
 
   switch (e.key) {
     case 'Enter':
-      handleToolbarButtonClick(item, null, false);
+    case ' ':
+    case 'Spacebar':
+      activateToolbarItem(item);
       break;
     case 'Escape':
       closeDropdowns();
@@ -377,6 +393,7 @@ onBeforeUnmount(() => {
               <ToolbarButton
                 :toolbar-item="item"
                 :disabled="item.disabled.value"
+                :allow-enter-propagation="true"
                 @textSubmit="handleToolbarButtonTextSubmit(item, $event)"
                 @mainClick="handleSplitButtonMainClick(item)"
               />

packages/super-editor/src/editors/v1/components/toolbar/ToolbarButton.vue

@@ -34,6 +34,18 @@ const props = defineProps({
     type: Boolean,
     default: false,
   },
+  // When true, Enter does not stopPropagation at this button - the event
+  // bubbles up to whatever parent listens for keyboard activation (e.g.
+  // ButtonGroup's roving-tabindex handler when this button is the visual
+  // trigger inside a ToolbarDropdown). Plain-button uses keep the default
+  // (false) so the parent does not double-fire the command emission.
+  // Note: split buttons stop propagation internally inside
+  // handleSplitMainClick, so this prop has no effect for them - Enter still
+  // runs the main command on a split button regardless.
+  allowEnterPropagation: {
+    type: Boolean,
+    default: false,
+  },
 });
 
 const {
@@ -89,6 +101,11 @@ const handleOuterEnter = (event) => {
   handleClick();
 };
 
+const onEnterKeydown = (event) => {
+  if (!props.allowEnterPropagation) event.stopPropagation();
+  handleOuterEnter(event);
+};
+
 const handleInputSubmit = () => {
   const value = inlineTextInput.value;
   const cleanValue = value.match(/^\d+(\.5)?$/) ? value : Math.floor(parseFloat(value)).toString();
@@ -120,7 +137,7 @@ const caretIcon = computed(() => {
     :role="isOverflowItem ? 'menuitem' : 'button'"
     :aria-label="attributes.ariaLabel"
     @click="handleOuterClick"
-    @keydown.enter.stop="handleOuterEnter($event)"
+    @keydown.enter="onEnterKeydown($event)"
     tabindex="0"
   >
     <div

packages/super-editor/src/editors/v1/components/toolbar/ToolbarDropdown.test.js

@@ -0,0 +1,59 @@
+import { afterEach, describe, expect, it } from 'vitest';
+import { mount } from '@vue/test-utils';
+import { defineComponent, nextTick, ref } from 'vue';
+import ToolbarDropdown from './ToolbarDropdown.vue';
+
+const waitForAnimationFrame = () => new Promise((resolve) => requestAnimationFrame(resolve));
+let wrapper;
+
+afterEach(() => {
+  wrapper?.unmount();
+  wrapper = null;
+  document.body.innerHTML = '';
+});
+
+describe('ToolbarDropdown keyboard focus', () => {
+  it('returns focus to the trigger when Escape closes after option navigation', async () => {
+    const Harness = defineComponent({
+      components: { ToolbarDropdown },
+      setup() {
+        const show = ref(false);
+        const options = [
+          { key: 'georgia', label: 'Georgia', props: { class: 'selected' } },
+          { key: 'arial', label: 'Arial', props: {} },
+          { key: 'courier', label: 'Courier New', props: {} },
+        ];
+        return { options, show };
+      },
+      template: `
+        <ToolbarDropdown v-model:show="show" :options="options">
+          <template #trigger>
+            <button data-test="trigger" type="button">Font family</button>
+          </template>
+        </ToolbarDropdown>
+      `,
+    });
+
+    wrapper = mount(Harness, { attachTo: document.body });
+    const trigger = wrapper.get('[data-test="trigger"]');
+    trigger.element.focus();
+    expect(document.activeElement).toBe(trigger.element);
+
+    wrapper.vm.show = true;
+    await nextTick();
+    await nextTick();
+
+    const options = document.body.querySelectorAll('.toolbar-dropdown-option');
+    expect(options).toHaveLength(3);
+    expect(document.activeElement).toBe(options[0]);
+
+    document.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true }));
+    expect(document.activeElement).toBe(options[1]);
+
+    document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }));
+    await nextTick();
+    await waitForAnimationFrame();
+
+    expect(document.activeElement).toBe(trigger.element);
+  });
+});

packages/super-editor/src/editors/v1/components/toolbar/ToolbarDropdown.vue

@@ -249,6 +249,46 @@ const handlePointerDown = (event) => {
   close();
 };
 
+const TRIGGER_FOCUS_SELECTOR =
+  'button, [href], input, select, textarea, [role="button"], [tabindex]:not([tabindex="-1"])';
+const triggerFocusTargetRef = ref(null);
+const rememberTriggerFocusTarget = () => {
+  const trigger = triggerRef.value;
+  const active = document.activeElement;
+
+  // Dropdown options are teleported to <body> and receive focus while open.
+  // Remember the opener before focus moves so Escape can restore it later.
+  if (trigger && active instanceof HTMLElement && trigger.contains(active)) {
+    triggerFocusTargetRef.value = active;
+    return;
+  }
+
+  triggerFocusTargetRef.value = trigger?.matches?.(TRIGGER_FOCUS_SELECTOR)
+    ? trigger
+    : trigger?.querySelector(TRIGGER_FOCUS_SELECTOR);
+};
+
+const focusTrigger = () => {
+  const remembered = triggerFocusTargetRef.value;
+  if (remembered instanceof HTMLElement && document.contains(remembered)) {
+    remembered.focus();
+    return;
+  }
+
+  const trigger = triggerRef.value;
+  if (!trigger) return;
+
+  const fallback = trigger.matches?.(TRIGGER_FOCUS_SELECTOR) ? trigger : trigger.querySelector(TRIGGER_FOCUS_SELECTOR);
+
+  if (fallback instanceof HTMLElement) {
+    fallback.focus();
+    return;
+  }
+
+  trigger.setAttribute('tabindex', '-1');
+  trigger.focus();
+};
+
 const handleKeyDown = (event) => {
   if (!isOpen.value) return;
 
@@ -257,7 +297,12 @@ const handleKeyDown = (event) => {
   if (!supportedKeys.includes(key)) return;
 
   if (key === 'Escape') {
+    event.preventDefault();
     close();
+    nextTick(() => {
+      // Wait one frame so the focused teleported option is gone before restoring focus.
+      requestAnimationFrame(focusTrigger);
+    });
     return;
   }
 
@@ -291,6 +336,7 @@ watch(
       return;
     }
 
+    rememberTriggerFocusTarget();
     await nextTick();
     updateMenuPosition();