diff --git a/.changeset/editing-store-dead-state-cleanup.md b/.changeset/editing-store-dead-state-cleanup.md new file mode 100644 index 00000000..c4ad36a5 --- /dev/null +++ b/.changeset/editing-store-dead-state-cleanup.md @@ -0,0 +1,5 @@ +--- +'json-edit-react': patch +--- + +Internal cleanup: remove the now-unused `previouslyEditedElement` / `recordPreviousEdit` and `tabDirection` / `setTabDirection` plumbing from the editing store. These were only ever consumed by the redirect `useLayoutEffect` retired in the previous Tab-viability work; with the redirect gone, the state was write-only and the actions had no readers. `EditingStore` shrinks to `{ open, cancel, submit, areChildrenBeingEdited }` (plus the subscribe/getSnapshot pair). No user-visible change. diff --git a/.changeset/tab-viability-skip-non-viable-targets.md b/.changeset/tab-viability-skip-non-viable-targets.md new file mode 100644 index 00000000..8bb79ce4 --- /dev/null +++ b/.changeset/tab-viability-skip-non-viable-targets.md @@ -0,0 +1,11 @@ +--- +'json-edit-react': patch +--- + +Tab navigation now skips filtered-out and non-editable nodes up front instead of opening them and bouncing reactively. The redirect `useLayoutEffect` in `ValueNodeWrapper` that previously fired transient `startEdit` / `cancelEdit` observer events on dead-end nodes is gone — `onEditEvent` consumers no longer see those spurious pairs. + +The change is in `getNextOrPrevious`, which gains an optional 5th `isViable?: (nodeData) => boolean` predicate. `useCommon` composes the predicate from the precomputed `filterState.visiblePaths` Set and the existing `allowEditFilter`, and threads it through. Tab navigation from both value edits and key edits (via `KeyDisplay`) now benefits. + +Behaviour change worth noting: when no viable Tab target exists, the editor cancels cleanly. The previous "fall back to `previouslyEditedElement`" hop is gone. + +When live search hides the actively-edited node, the input now unmounts cleanly and the editing record sits inactive in the store; clearing the search later resumes the edit. No off-screen commit footgun because there's no input element to commit through. diff --git a/src/JsonEditor.tsx b/src/JsonEditor.tsx index 4d371170..03e60fde 100644 --- a/src/JsonEditor.tsx +++ b/src/JsonEditor.tsx @@ -7,6 +7,7 @@ import React, { useState, } from 'react' import { assign, type AssignOptions, type AssignInput } from './utils/assign' +import { buildNodeData } from './utils/buildNodeData' import { extract } from './utils/extract' import { CollectionNode } from './CollectionNode' import { NativeSelect } from './NativeSelect' @@ -825,57 +826,6 @@ const updateDataObject = ( } } -// Canonical `NodeData` builder: construct the data for any node from the live -// tree given its path. Used for the root node and by the `editorRef` handle to -// run a node's `allowEdit` filter at call time (the filter takes the full -// NodeData, not just a path). `index`/`size` mirror the child construction in -// CollectionNode so a filter keying off them sees the same input the rendered -// node would; `sort` (the `sortKeys` comparator) is only needed to resolve the -// `index` of an object child, so it's optional. -const buildNodeData = ( - fullData: JsonData, - path: CollectionKey[], - rootName: string, - sort?: (arr: T[], nodeMap: (input: T) => [string | number, unknown]) => void -): NodeData => { - if (path.length === 0) { - return { - key: rootName, - path: [], - level: 0, - index: 0, - value: fullData, - size: isCollection(fullData) ? Object.keys(fullData).length : 1, - parentData: null, - fullData, - } - } - - const key = path[path.length - 1] - const value = extract(fullData, path) as JsonData - const parentData = (extract(fullData, path.slice(0, -1)) ?? null) as object | null - - let index = 0 - if (Array.isArray(parentData)) { - index = typeof key === 'number' ? key : Number(key) - } else if (parentData && typeof parentData === 'object') { - const entries = Object.entries(parentData) as Array<[string | number, unknown]> - sort?.(entries, (entry) => entry) - index = entries.findIndex(([k]) => k === key) - } - - return { - key, - path, - level: path.length, - index, - value, - size: isCollection(value) ? Object.keys(value as object).length : null, - parentData, - fullData, - } -} - const getFilterFunction = ( propValue: boolean | number | FilterFunction ): FilterFunction => { diff --git a/src/ValueNodeWrapper.tsx b/src/ValueNodeWrapper.tsx index af3a57ed..16a7f1c6 100644 --- a/src/ValueNodeWrapper.tsx +++ b/src/ValueNodeWrapper.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useLayoutEffect, useState, useMemo, useCallback, useRef } from 'react' +import React, { useEffect, useState, useMemo, useCallback, useRef } from 'react' import { StringValue, NumberValue, @@ -60,14 +60,11 @@ const ValueNodeWrapperBase: React.FC = (props) => { getLatestData, } = props const { getStyles } = useTheme() - // Actions + a getSnapshot for imperative reads. The editing *state* this node - // needs (active, tabDirection, previouslyEditedElement) is only read - // inside event handlers / the Tab-redirect effect — never during render — so - // it's read from the snapshot at use-time rather than subscribed to. The only - // editing state that drives this node's render (`isEditing`) comes from - // `useCommon`'s per-node selector. - const { open, cancel, submit, recordPreviousEdit, setTabDirection, getSnapshot } = - useEditingStore() + // Actions + a getSnapshot for imperative reads. The editing `active` state + // this node reads (via `getSnapshot`) is only consulted inside event + // handlers, never during render. The only editing state that drives this + // node's render (`isEditing`) comes from `useCommon`'s per-node selector. + const { open, cancel, submit, getSnapshot } = useEditingStore() const { setCollapseState } = useCollapse() const [value, setValue] = useState( // Bad things happen when you put a function into useState @@ -184,39 +181,6 @@ const ValueNodeWrapperBase: React.FC = (props) => { // Early return if this node is filtered out const isVisible = useNodeVisible(path) - // Skip hidden or uneditable nodes that Tab navigation has landed on by - // advancing the editing target to the next viable node. - // - // `useLayoutEffect` (not `useEffect`) is load-bearing: it fires synchronously - // before the browser paints, so the redirect state update batches into the - // same paint as the commit that flagged this node as `isEditing`. With plain - // `useEffect` the user would see a flicker — the current node briefly - // closes its editor (commit 1 with the filtered-out target "editing"), - // then reopens (commit 2 after the redirect). See V2-roadmap §16 for the - // followup: hoisting filter-awareness into `getNextOrPrevious` so the Tab - // handler can pick a viable target up front, eliminating the redirect - // entirely and dropping the setState-after-render pattern. - useLayoutEffect(() => { - if (!isEditing) return - if (isVisible && canEdit) return - // A forced (imperative `editorRef.startEdit`) edit overrides `allowEdit`, - // so don't bounce off this node just because it's normally uneditable. A - // search-filtered-out node (`!isVisible`) still redirects — it can't - // render. - if (isVisible && getSnapshot().active?.force) return - const { tabDirection, previouslyEditedElement } = getSnapshot() - const next = getNextOrPreviousAtPath(tabDirection) - if (next) open(next) - else if (previouslyEditedElement) open(previouslyEditedElement) - else cancel() - // The three booleans gate the redirect; `open`/`cancel` are included for - // hygiene (store-stable, so they almost never flip). The remaining reads - // (`tabDirection`, `previouslyEditedElement`, `path`, `sort`) are - // intentionally excluded — they change every render / edit transition and - // would cause spurious re-fires when no redirect is needed. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isEditing, isVisible, canEdit, open, cancel]) - if (!isVisible) return null // A local (deferred) type-switch is in progress when the editing `dataType` @@ -436,14 +400,13 @@ const ValueNodeWrapperBase: React.FC = (props) => { const setIsEditing = canEdit ? startEdit : NOOP // Commit this field's edit, then open the next/previous node in the given Tab - // direction. Not pure — it orchestrates store actions (`setTabDirection`, - // `recordPreviousEdit`, `open`) with this node's `handleEdit`/`path`, so it - // stays local rather than moving to keyboard utils (cf. the pure - // `getNextOrPrevious`). `handleEdit`'s `onCommit` defers `open` to the commit - // moment, so Tab advances only once this field's edit has landed. + // direction. Not pure — it orchestrates store actions (`open`) with this + // node's `handleEdit`/`path`, so it stays local rather than moving to + // keyboard utils (cf. the pure `getNextOrPrevious`). `handleEdit`'s + // `onCommit` defers `open` to the commit moment, so Tab advances only once + // this field's edit has landed. `getNextOrPreviousAtPath` already skips + // non-viable targets (the viability predicate lives in `useCommon`). const tabTo = (dir: TabDirection) => () => { - setTabDirection(dir) - recordPreviousEdit(path) const target = getNextOrPreviousAtPath(dir) if (target) handleEdit(undefined, () => open(target)) } diff --git a/src/contexts/EditingProvider.tsx b/src/contexts/EditingProvider.tsx index d17a74be..fec8a844 100644 --- a/src/contexts/EditingProvider.tsx +++ b/src/contexts/EditingProvider.tsx @@ -34,7 +34,6 @@ import React, { createContext, useContext, useMemo, useRef, useSyncExternalStore } from 'react' import { - type TabDirection, type CollectionKey, type OnEditEventFunction, type EditEvent, @@ -65,8 +64,6 @@ export interface EditingStateBundle { active: EditingState | null /** In-flight optimistic commits: path-string → the commit's token. */ settling: Record - previouslyEditedElement: CollectionKey[] | null - tabDirection: TabDirection } /** A commit to perform, discriminated by `op`. `path` is the target/source. */ @@ -187,8 +184,6 @@ export interface EditingStore { * `onUpdate`) so the calling node can report errors via its own `onError`. */ submit: (args: SubmitArgs) => Promise - setTabDirection: (dir: TabDirection) => void - recordPreviousEdit: (path: CollectionKey[]) => void /** Imperative read for event handlers — does not subscribe. */ areChildrenBeingEdited: (path: CollectionKey[]) => boolean } @@ -196,8 +191,6 @@ export interface EditingStore { const initialState: EditingStateBundle = { active: null, settling: {}, - previouslyEditedElement: null, - tabDirection: 'next', } const createEditingStore = ( @@ -485,17 +478,6 @@ const createEditingStore = ( return outcome } - const setTabDirection = (dir: TabDirection) => { - if (state.tabDirection !== dir) commit({ ...state, tabDirection: dir }) - } - - const recordPreviousEdit = (path: CollectionKey[]) => { - const prev = state.previouslyEditedElement - if (prev === null || !pathsEqual(prev, path)) { - commit({ ...state, previouslyEditedElement: path }) - } - } - return { subscribe: (onChange) => { listeners.add(onChange) @@ -506,8 +488,6 @@ const createEditingStore = ( open, cancel, submit, - setTabDirection, - recordPreviousEdit, areChildrenBeingEdited: (path) => state.active !== null && isDescendantOf(state.active.path, path), } @@ -591,8 +571,6 @@ export const useEditing = () => { open: store.open, cancel: store.cancel, submit: store.submit, - setTabDirection: store.setTabDirection, - recordPreviousEdit: store.recordPreviousEdit, areChildrenBeingEdited: store.areChildrenBeingEdited, }), [bundle, store] diff --git a/src/contexts/FilterStateProvider.tsx b/src/contexts/FilterStateProvider.tsx index 9de6668d..bb98eb5b 100644 --- a/src/contexts/FilterStateProvider.tsx +++ b/src/contexts/FilterStateProvider.tsx @@ -4,7 +4,7 @@ * visibility/count is an O(1) Set/Map lookup instead of the per-node * subtree walk the old `filterNode` did. * - * Three consumer hooks: + * Four consumer hooks: * * useFilterActive() — boolean: "is search active right now?" * Drives the n-of-m branch (no point @@ -20,6 +20,14 @@ * filter is active (caller falls back to * `nodeData.size`). * + * useRawFilterState() — FilterState | null: the whole pre- + * computed bundle. For callers that need + * to check many candidate paths from a + * closure (e.g. the Tab-viability + * predicate in ValueNodeWrapper, where + * hooks-per-candidate would violate the + * rules of hooks). + * * Pattern note: this is a dumb provider — `JsonEditor` owns the * `useMemo` that builds the state, and passes it as a `value` prop. * Keeps the algorithm out of React-land for testability (see @@ -56,6 +64,8 @@ const useFilterStateContext = (): FilterState | null => { export const useFilterActive = (): boolean => useFilterStateContext() !== null +export const useRawFilterState = (): FilterState | null => useFilterStateContext() + export const useNodeVisible = (path: CollectionKey[]): boolean => { const fs = useFilterStateContext() if (fs === null) return true diff --git a/src/contexts/index.ts b/src/contexts/index.ts index 619097d5..6741a7ed 100644 --- a/src/contexts/index.ts +++ b/src/contexts/index.ts @@ -8,4 +8,5 @@ export { useFilterActive, useNodeVisible, useVisibleChildCount, + useRawFilterState, } from './FilterStateProvider' diff --git a/src/hooks/useCommon.ts b/src/hooks/useCommon.ts index e0a138ad..8d07150b 100644 --- a/src/hooks/useCommon.ts +++ b/src/hooks/useCommon.ts @@ -4,11 +4,17 @@ */ import React, { useCallback, useRef, useState } from 'react' -import { useEditingSelector, useEditingStore, useVisibleChildCount } from '../contexts' +import { + useEditingSelector, + useEditingStore, + useRawFilterState, + useVisibleChildCount, +} from '../contexts' import { type CollectionNodeProps, type ErrorString, type JerError, + type NodeData, type ThemeableElement, type ValueData, type ValueNodeProps, @@ -113,11 +119,35 @@ export const useCommon = ({ props, collapsed }: CommonProps) => { [onErrorCallback, showErrorMessages, getLatestData, errorDisplayTime] ) + // Tab-viability predicate: a candidate leaf is a valid Tab target only + // if it would be visible AND editable. Visibility comes from the + // precomputed `filterState.visiblePaths` Set; editability is the + // existing `allowEditFilter(nodeData)`. Closures over the live filter + // state, so a search keystroke that re-builds the state implicitly + // re-builds the predicate (via useCallback identity), but only when + // those inputs actually change — the §16 memo invariants hold. + // + // Hooks-rules note: we read the WHOLE filter state via + // `useRawFilterState` (not per-path `useNodeVisible`) because the + // predicate is invoked for many candidate paths during a single Tab, + // and we can't call hooks inside a loop. + const filterState = useRawFilterState() + const isViableTarget = useCallback( + (candidate: NodeData) => { + const visible = + filterState === null || filterState.visiblePaths.has(toPathString(candidate.path)) + return visible && allowEditFilter(candidate) + }, + [filterState, allowEditFilter] + ) + // Stable wrapper around `getNextOrPrevious` against the LIVE document for // this node's `path`, so callers (`KeyDisplay`, value-node `tabForward` / // `tabBack`) don't need to re-thread `getLatestData` / `path` / `sort`. + // Threads `isViableTarget` so Tab skips filtered-out / non-editable + // leaves up front instead of landing on them and bouncing. const getNextOrPreviousAtPath = (type: 'next' | 'prev') => - getNextOrPrevious(getLatestData(), path, type, sort) + getNextOrPrevious(getLatestData(), path, type, sort, isViableTarget) // Commits a key rename through the store's commit engine. The no-op / // duplicate-key checks stay client-side and close the session as a cancel. diff --git a/src/utils/buildNodeData.ts b/src/utils/buildNodeData.ts new file mode 100644 index 00000000..b3d63120 --- /dev/null +++ b/src/utils/buildNodeData.ts @@ -0,0 +1,62 @@ +import { type CollectionKey, type JsonData, type NodeData } from '../types' +import { extract } from './extract' +import { isCollection } from './misc' + +/** + * Builds a `NodeData` snapshot for an arbitrary path against `fullData`. + * + * Reused by: + * - `JsonEditor` (root and bridge construction — onCollapse / onEditEvent / + * editorRef handle reads). + * - `getNextOrPrevious` (synthesising the candidate `NodeData` the + * viability predicate is called with during Tab navigation). + * + * `index` derivation respects the same sort the renderer uses, so a custom + * `searchFilter` / `allowEdit` callback sees the same `index` it would during + * render. `rootName` is the editor's `rootName` prop value — only used when + * `path` is empty; defaults to `''` for callers that never target the root + * (Tab leaves, internal walk steps). + */ +export const buildNodeData = ( + fullData: JsonData, + path: CollectionKey[], + rootName = '', + sort?: (arr: T[], nodeMap: (input: T) => [string | number, unknown]) => void +): NodeData => { + if (path.length === 0) { + return { + key: rootName, + path: [], + level: 0, + index: 0, + value: fullData, + size: isCollection(fullData) ? Object.keys(fullData).length : 1, + parentData: null, + fullData, + } + } + + const key = path[path.length - 1] + const value = extract(fullData, path) as JsonData + const parentData = (extract(fullData, path.slice(0, -1)) ?? null) as object | null + + let index = 0 + if (Array.isArray(parentData)) { + index = typeof key === 'number' ? key : Number(key) + } else if (parentData && typeof parentData === 'object') { + const entries = Object.entries(parentData) as Array<[string | number, unknown]> + sort?.(entries, (entry) => entry) + index = entries.findIndex(([k]) => k === key) + } + + return { + key, + path, + level: path.length, + index, + value, + size: isCollection(value) ? Object.keys(value as object).length : null, + parentData, + fullData, + } +} diff --git a/src/utils/keyboard.ts b/src/utils/keyboard.ts index 14c7986e..74e9b9e0 100644 --- a/src/utils/keyboard.ts +++ b/src/utils/keyboard.ts @@ -5,9 +5,11 @@ import { type KeyboardControls, type KeyboardControlsFull, type KeyEvent, + type NodeData, type SortFunction, type TabDirection, } from '../types' +import { buildNodeData } from './buildNodeData' import { extract } from './extract' import { isCollection } from './misc' @@ -153,7 +155,16 @@ export const getNextOrPrevious = ( fullData: JsonData, path: CollectionKey[], nextOrPrev: TabDirection = 'next', - sort: SortFunction + sort: SortFunction, + // Viability predicate. Candidate leaves whose synthesized `NodeData` + // fails the predicate are skipped: the function recurses with the + // failed candidate as the new starting point, continuing in the same + // direction until a viable leaf or `null` is reached. Lets Tab + // navigation skip filtered-out or non-editable nodes up front instead + // of relying on a downstream redirect to bounce them. Pass `() => true` + // for a pure structural walk (tests; never in production — the editor + // always supplies a real predicate via `useCommon`). + isViable: (nodeData: NodeData) => boolean ): CollectionKey[] | null => { const parentPath = path.slice(0, path.length - 1) const thisKey = path.slice(-1)[0] @@ -173,15 +184,24 @@ export const getNextOrPrevious = ( if (!destination) { if (parentPath.length === 0) return null - return getNextOrPrevious(fullData, parentPath, nextOrPrev, sort) + return getNextOrPrevious(fullData, parentPath, nextOrPrev, sort, isViable) } + let candidate: CollectionKey[] | null if (isCollection(destination.value)) { if (Object.keys(destination.value).length === 0) { - return getNextOrPrevious(fullData, [...parentPath, destination.key], nextOrPrev, sort) + return getNextOrPrevious(fullData, [...parentPath, destination.key], nextOrPrev, sort, isViable) } - return getChildRecursive(fullData, [...parentPath, destination.key], nextOrPrev, sort) - } else return [...parentPath, destination.key] + candidate = getChildRecursive(fullData, [...parentPath, destination.key], nextOrPrev, sort) + } else { + candidate = [...parentPath, destination.key] + } + if (!candidate) return null + + if (!isViable(buildNodeData(fullData, candidate, '', sort))) { + return getNextOrPrevious(fullData, candidate, nextOrPrev, sort, isViable) + } + return candidate } // If the node at "path" is a collection, tries the first/last child of that diff --git a/test/JsonEditor.test.tsx b/test/JsonEditor.test.tsx index 6a1426b5..680edd2b 100644 --- a/test/JsonEditor.test.tsx +++ b/test/JsonEditor.test.tsx @@ -2075,6 +2075,71 @@ describe('JsonEditor — search and filter', () => { expect(seen.length).toBeGreaterThan(0) seen.forEach((v) => expect(v).toBeNull()) }) + + test('Tab past a non-editable node fires NO transient startEdit/cancelEdit pair', async () => { + // With the redirect useLayoutEffect retired (#334), Tab calls now + // skip non-viable nodes inside `getNextOrPrevious` instead of opening + // them and bouncing reactively. So the event stream for a Tab from + // `a` past a non-editable `b` to `c` should be: + // startEdit(a) → submitEdit(a) → commitEdit(a) → startEdit(c) + // — no startEdit(b) or cancelEdit(b) sneaking in. + const user = userEvent.setup() + const onEditEvent = jest.fn() + render( + key !== 'b'} + /> + ) + + await user.dblClick(screen.getByText('"one"')) + await user.keyboard('{Tab}') + + const seq = onEditEvent.mock.calls.map(([e]) => ({ event: e.event, key: e.key })) + // No event ever references `b` — Tab walked past it without opening it. + expect(seq.some((e) => e.key === 'b')).toBe(false) + // The transition lands directly on `c`. + expect(seq.map((e) => e.event)).toEqual([ + 'startEdit', + 'submitEdit', + 'commitEdit', + 'startEdit', + ]) + expect(seq[seq.length - 1].key).toBe('c') + }) + + test('Live search hiding the actively-edited node unmounts the input without error', async () => { + // The redirect's secondary role (cancel-when-active-becomes-invisible) + // is intentionally dropped — see plan §"Reactive cancel deliberately + // omitted". A search keystroke that filters out the currently-editing + // node leaves the editing record in the store; the node simply + // returns null from its render (its `!isVisible` early-return path), + // so the input unmounts cleanly with no errors. + const user = userEvent.setup() + const data = { apple: 'red-fruit', banana: 'yellow-fruit' } + // `searchDebounceTime={0}` so the rerender's filter applies immediately. + const { rerender } = render( + + ) + // Open an edit on `apple`. + await user.dblClick(screen.getByText('"red-fruit"')) + expect((screen.getByRole('textbox') as HTMLTextAreaElement).value).toBe('red-fruit') + // Apply a search that matches only `banana` — `apple` becomes hidden. + // The debounced setSearchText still uses a setTimeout(0), so let the + // microtask flush via `waitFor`. + expect(() => { + rerender( + + ) + }).not.toThrow() + await waitFor(() => expect(screen.queryByText('"red-fruit"')).toBeNull()) + // No input is rendered, and the matching sibling is still on screen. + expect(screen.queryByRole('textbox')).toBeNull() + expect(screen.getByText('"yellow-fruit"')).toBeInTheDocument() + }) }) describe('JsonEditor — textarea character insertion via keyboard', () => { diff --git a/test/keyboard.test.ts b/test/keyboard.test.ts index e6c81458..90e856da 100644 --- a/test/keyboard.test.ts +++ b/test/keyboard.test.ts @@ -98,10 +98,15 @@ describe('getFullKeyboardControlMap', () => { }) describe('getNextOrPrevious', () => { + // Tests in this block exercise the pure STRUCTURAL walk — pass an + // always-true `isViable` predicate so every candidate is accepted and + // the result reflects raw document order. The viability-skipping + // behaviour itself is tested separately under `describe('isViable predicate')` below. + const acceptAll = () => true const getNext = (data: object, path: (string | number)[]) => - getNextOrPrevious(data, path, 'next', () => {}) + getNextOrPrevious(data, path, 'next', () => {}, acceptAll) const getPrevious = (data: object, path: (string | number)[]) => - getNextOrPrevious(data, path, 'prev', () => {}) + getNextOrPrevious(data, path, 'prev', () => {}, acceptAll) const data = { a: 1, @@ -197,4 +202,61 @@ describe('getNextOrPrevious', () => { expect(getNext(d, ['styles', 'container', 'fontFamily'])).toEqual(['styles', 'lastOne']) expect(getNext(d, ['styles', 'lastOne'])).toEqual(null) }) + + describe('isViable predicate', () => { + // The predicate receives a synthesized NodeData per candidate leaf and + // returns true to accept that leaf, false to skip onward. This is how + // Tab navigation now skips filtered-out / non-editable nodes up front, + // replacing the redirect useLayoutEffect that previously bounced edits + // reactively in ValueNodeWrapper. + const sample = { a: 1, b: 2, c: 3, d: 4, e: 5 } + + test('skips a single non-viable leaf forward and back', () => { + // Skip `c` going forward and back. + const isViable = (nd: { key: string | number }) => nd.key !== 'c' + expect(getNextOrPrevious(sample, ['b'], 'next', () => {}, isViable)).toEqual(['d']) + expect(getNextOrPrevious(sample, ['d'], 'prev', () => {}, isViable)).toEqual(['b']) + }) + + test('skips a contiguous run of non-viable leaves', () => { + // Skip b, c, d going forward; should land on e. + const isViable = (nd: { key: string | number }) => + !(['b', 'c', 'd'] as Array).includes(nd.key) + expect(getNextOrPrevious(sample, ['a'], 'next', () => {}, isViable)).toEqual(['e']) + // And reverse: from e back, skip d, c, b — land on a. + expect(getNextOrPrevious(sample, ['e'], 'prev', () => {}, isViable)).toEqual(['a']) + }) + + test('returns null when nothing viable remains in the search direction', () => { + const isViable = (nd: { key: string | number }) => nd.key === 'a' + // From `a`, no more `a` siblings forward → null. + expect(getNextOrPrevious(sample, ['a'], 'next', () => {}, isViable)).toEqual(null) + // From `a`, no `a` siblings before either → null. + expect(getNextOrPrevious(sample, ['a'], 'prev', () => {}, isViable)).toEqual(null) + }) + + test('viability check sees the candidate NodeData shape, not the source', () => { + // The predicate inspects the CANDIDATE leaf (the proposed Tab + // destination), not the source. Asserts the synthesized shape + // carries path, level, value, parentData, fullData so custom + // searchFilter / allowEdit functions get what they expect. + const seen: Array<{ key: unknown; path: unknown; level: unknown; value: unknown }> = [] + const isViable = (nd: { key: unknown; path: unknown; level: unknown; value: unknown }) => { + seen.push({ key: nd.key, path: nd.path, level: nd.level, value: nd.value }) + return true + } + getNextOrPrevious(sample, ['a'], 'next', () => {}, isViable) + // First candidate from ['a'] going next is ['b'] (a leaf, value 2). + expect(seen[0]).toEqual({ key: 'b', path: ['b'], level: 1, value: 2 }) + }) + + test('descends into a collection and skips a non-viable first leaf', () => { + // Tab from a leaf into a nested collection — viability is checked on + // the deepest first leaf reached by getChildRecursive, and skipped + // onward if non-viable. + const nested = { a: 1, b: { x: 'skip-me', y: 'ok' }, c: 3 } + const isViable = (nd: { value: unknown }) => nd.value !== 'skip-me' + expect(getNextOrPrevious(nested, ['a'], 'next', () => {}, isViable)).toEqual(['b', 'y']) + }) + }) })