Tab viability: skip non-viable targets, retire redirect useLayoutEffect (#334)

[CarlosNZ]

Closes #334.

Summary

• getNextOrPrevious gains an optional 5th isViable?: (nodeData) => boolean predicate. At every candidate leaf, the function synthesises NodeData for the candidate and calls isViable; on false, it recurses with the failed candidate as the new starting point. Sort-aware index derivation matches buildNodeData semantics so custom searchFilter / allowEdit callbacks see the same shape they get during render.
• useCommon composes isViableTarget from the precomputed filterState.visiblePaths Set (new useRawFilterState hook) and the existing allowEditFilter. Predicate is useCallback-stable across stable inputs. Threads it through getNextOrPreviousAtPath. KeyDisplay's Tab path (key-edit → value-edit) inherits the viability check for free.
• Redirect useLayoutEffect removed from ValueNodeWrapper. Tab no longer fires transient startEdit / cancelEdit observer pairs on dead-end nodes.

User-visible improvements

• Cleaner onEditEvent stream: a Tab past N non-viable nodes used to fire startEdit(badNode_1) → cancelEdit(badNode_1) → startEdit(badNode_2) → cancelEdit(badNode_2) → … → startEdit(goodNode). Now it's just startEdit(goodNode).
• No setState-during-useLayoutEffect pattern in every value node.
• O(1) Set-lookup visibility check instead of per-candidate filterNode calls (the visibility primitive built in Filtered collection counts + filter rewrite (#113) #351).

Behaviour changes worth flagging

• No previouslyEditedElement fallback. Today's redirect tries the previously-edited node before giving up; now when no viable Tab target exists, the editor cancels cleanly. Aligns with the "decision up front" model the issue argued for. recordPreviousEdit / previouslyEditedElement likely become dead state; queued for a follow-up cleanup PR once this is verified stable.
• Live search hiding the actively-edited node. The redirect's secondary role (cancel-when-active-becomes-invisible) is intentionally dropped. The node early-returns from render (its !isVisible guard), the input unmounts, the editing record stays in the store. Clearing the search later resumes the edit. No off-screen-commit footgun because there's no input element to commit through.

Test plan

• pnpm test keyboard — 6 new cases for the isViable predicate (skip single, skip run, return null when none viable, candidate-NodeData shape, descend-then-skip, no-predicate backward-compat). Total 21 passing.
• pnpm test JsonEditor — added 2 cases: Tab-past-non-editable fires NO transient observer events, and live search hiding the open node unmounts cleanly without error. Existing 'Tab and Shift+Tab skip filtered-out nodes' (line 1864) still passes — same user-visible behaviour via different mechanism. Total 107 passing.
• pnpm test — 456 tests across 18 suites passing.
• pnpm lint && pnpm compile && pnpm build — clean.
• Manual: Tab from a node where the next is filtered out → lands on the next visible. Tab past a non-editable node (allowEdit restriction) → skipped, no input flicker. Type into search while a node is being edited → input disappears, no errors. Clear search → input reappears, edit resumes. editorRef.startEdit(path, { force: true }) on a non-editable visible node → edit opens.
• Manual observer-event smoke: open the event-signals demo, Tab through a tree with mixed editable/non-editable nodes; the event stream should be clean.

Follow-up (separate PR)

After this lands and is verified stable, remove the now-unused previouslyEditedElement / recordPreviousEdit plumbing from EditingProvider and audit tabDirection for the same.

🤖 Generated with Claude Code

[github-actions]

Bundle size impact

json-edit-react

Format	Base raw	PR raw	Δ raw	Base gzip	PR gzip	Δ gzip
esm	56.05 KB	55.76 KB	🟢 -299 B (-0.52%)	20.09 KB	20.01 KB	🟢 -82 B (-0.40%)
cjs	57.58 KB	57.29 KB	🟢 -301 B (-0.51%)	20.17 KB	20.10 KB	🟢 -76 B (-0.37%)

_{Measured from build/index.{cjs,esm}.js. Gzip at level 9.}

[CarlosNZ]

Both review points addressed in 81a354e.

isViable made required. Audit confirmed getNextOrPrevious is internal-only (not in index.ts) and the only no-predicate callers were tests verifying the legacy structural walk. Production (useCommon) always supplies one. Tests now pass an explicit acceptAll = () => true for the structural-walk case; the "absence of predicate is byte-equivalent" test went away — trivially true once the param is required.

buildNodeData extracted to src/utils/buildNodeData.ts. The inline buildLeafNodeData was a near-duplicate of the buildNodeData in JsonEditor.tsx — same shape, same index-derivation logic, same fields. Differences were:

• buildNodeData had a root-path branch using rootName; buildLeafNodeData didn't (Tab never targets root)
• buildLeafNodeData required sort; buildNodeData had it optional

Both differences are non-conflicting — the unified buildNodeData keeps rootName (default '') and sort optional. Both call sites now use the same util. Keyboard.ts passes '' for rootName since Tab targets leaves; the root-path branch isn't reachable from that caller.

Net diff: −82 lines across JsonEditor.tsx + keyboard.ts (deleting both copies), +62 lines in the new shared util. Tests, lint, compile, build all clean.