diff --git a/packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.test.js b/packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.test.js index 70a69bd1be..21403ff456 100644 --- a/packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.test.js +++ b/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: '
' }, + }, + }, + }); + + 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(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.vue b/packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.vue index 82fb197bb8..7f0e5c5c33 100644 --- a/packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.vue +++ b/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(() => { diff --git a/packages/super-editor/src/editors/v1/components/toolbar/ToolbarButton.vue b/packages/super-editor/src/editors/v1/components/toolbar/ToolbarButton.vue index 8894a6b5e7..d1e9e7338f 100644 --- a/packages/super-editor/src/editors/v1/components/toolbar/ToolbarButton.vue +++ b/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" >
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: ` + + + + `, + }); + + 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); + }); +}); diff --git a/packages/super-editor/src/editors/v1/components/toolbar/ToolbarDropdown.vue b/packages/super-editor/src/editors/v1/components/toolbar/ToolbarDropdown.vue index 65cb02b409..5502a7d199 100644 --- a/packages/super-editor/src/editors/v1/components/toolbar/ToolbarDropdown.vue +++ b/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 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();