From 9b96debaf6d0e22b0c026fad7e4f16f0138b3a36 Mon Sep 17 00:00:00 2001 From: Joe Anderson Date: Sat, 23 Dec 2023 12:27:11 +0000 Subject: [PATCH 1/2] Add comments to tabbable code --- packages/tabbable/src/TabbableEffects.tsx | 32 +++++++++++++++++++++ packages/tabbable/src/findTabDestination.ts | 29 +++++++++++++++++-- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/packages/tabbable/src/TabbableEffects.tsx b/packages/tabbable/src/TabbableEffects.tsx index 67b3346e67..383d5a1e9d 100644 --- a/packages/tabbable/src/TabbableEffects.tsx +++ b/packages/tabbable/src/TabbableEffects.tsx @@ -29,6 +29,7 @@ export function TabbableEffects() { if (!editorDOMNode) return; const handler = (event: KeyboardEvent) => { + // Check if the keydown is a tab key that should be handled if ( event.key !== 'Tab' || event.defaultPrevented || @@ -37,11 +38,20 @@ export function TabbableEffects() { return; } + /** + * Get the list of additional tabbable entries specified in the plugin + * options + */ const insertedTabbableEntries = insertTabbableEntries?.( editor, event ) as TabbableEntry[]; + /** + * Global event listener only. Do not handle the tab event if the keydown + * was sent to an element other than the editor or one of the additional + * tabbable elements. + */ if ( globalEventListener && event.target && @@ -53,8 +63,13 @@ export function TabbableEffects() { return; } + // Get all tabbable DOM nodes in the editor const tabbableDOMNodes = tabbable(editorDOMNode) as HTMLElement[]; + /** + * Construct a tabbable entry for each tabbable Slate node, filtered by + * the `isTabbable` option (defaulting to only void nodes). + */ const defaultTabbableEntries = tabbableDOMNodes .map((domNode) => { const slateNode = toSlateNode(editor, domNode); @@ -69,17 +84,28 @@ export function TabbableEffects() { (entry) => entry && isTabbable?.(editor, entry) ) as TabbableEntry[]; + /** + * The list of all tabbable entries. Sorting by path ensures a consistent + * tab order. + */ const tabbableEntries = [ ...insertedTabbableEntries, ...defaultTabbableEntries, ].sort((a, b) => Path.compare(a.path, b.path)); + /** + * TODO: Refactor everything ABOVE this line into a util function and + * test separately + */ + + // Check if any tabbable entry is the active element const { activeElement } = document; const activeTabbableEntry = (activeElement && tabbableEntries.find((entry) => entry.domNode === activeElement)) ?? null; + // Find the next Slate node or DOM node to focus const tabDestination = findTabDestination(editor, { tabbableEntries, activeTabbableEntry, @@ -107,6 +133,12 @@ export function TabbableEffects() { return; } + /** + * There was no tab destination, so let the browser handle the tab event. + * We don't want the browser to focus anything that could have been + * focused by us, so we make make all tabbable DOM nodes in the editor + * unfocusable. This ensures that the focus exits the editor cleanly. + */ tabbableDOMNodes.forEach((domNode) => { const oldTabIndex = domNode.getAttribute('tabindex'); domNode.setAttribute('tabindex', '-1'); diff --git a/packages/tabbable/src/findTabDestination.ts b/packages/tabbable/src/findTabDestination.ts index 35d0b40bbb..bd8bf410c2 100644 --- a/packages/tabbable/src/findTabDestination.ts +++ b/packages/tabbable/src/findTabDestination.ts @@ -18,6 +18,7 @@ export const findTabDestination = ( editor: PlateEditor, { tabbableEntries, activeTabbableEntry, direction }: FindTabDestinationOptions ): TabDestination | null => { + // Case 1: A tabbable entry was active before tab was pressed if (activeTabbableEntry) { // Find the next tabbable entry after the active one const activeTabbableEntryIndex = @@ -26,7 +27,23 @@ export const findTabDestination = ( activeTabbableEntryIndex + (direction === 'forward' ? 1 : -1); const nextTabbableEntry = tabbableEntries[nextTabbableEntryIndex]; - // If the next tabbable entry is in the same void, focus it + /** + * If the next tabbable entry originated from the same path as the active + * tabbable entry, focus it. + * + * Examples of when this is true: + * - We're inside a void node and there is an additional tabbable inside + * the same void node. + * - We're inside a popover containing multiple tabbable elements all + * anchored to the same slate node, and there is an additional tabbable + * inside the same popover. + * + * Examples of when this is false: + * - We're inside a void node and the next tabbable is outside the void + * node. + * - We're in the last tabbable element of a popover. + * - There is no next tabbable element. + */ if ( nextTabbableEntry && Path.equals(activeTabbableEntry.path, nextTabbableEntry.path) @@ -37,7 +54,13 @@ export const findTabDestination = ( }; } - // Otherwise, focus the first path after the void + /** + * Otherwise, return the focus to the editor. If we're moving forward, + * focus the first point after the active tabbable's path. If we're moving + * backward, focus the point of the active tabbable's path. + * TODO: Let a tabable entry specify custom before and after points. + */ + if (direction === 'forward') { const pointAfter = getPointAfter(editor, activeTabbableEntry.path); if (!pointAfter) return null; @@ -53,6 +76,8 @@ export const findTabDestination = ( }; } + // Case 2: No tabbable entry was active before tab was pressed + const selectionPath = editor.selection?.anchor?.path || []; // Find the first tabbable entry after the selection From 93b092f72530ac30ab939b919db9861a7e2881f1 Mon Sep 17 00:00:00 2001 From: Joe Anderson Date: Sat, 23 Dec 2023 12:31:32 +0000 Subject: [PATCH 2/2] Fix typo --- packages/tabbable/src/findTabDestination.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tabbable/src/findTabDestination.ts b/packages/tabbable/src/findTabDestination.ts index bd8bf410c2..b60fc078a7 100644 --- a/packages/tabbable/src/findTabDestination.ts +++ b/packages/tabbable/src/findTabDestination.ts @@ -58,7 +58,7 @@ export const findTabDestination = ( * Otherwise, return the focus to the editor. If we're moving forward, * focus the first point after the active tabbable's path. If we're moving * backward, focus the point of the active tabbable's path. - * TODO: Let a tabable entry specify custom before and after points. + * TODO: Let a tabbable entry specify custom before and after points. */ if (direction === 'forward') {