feat(mind): navigate selected node with arrow keys#1147
Conversation
Deploying plait with
|
| Latest commit: |
3abcb42
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d0a5f94c.plait.pages.dev |
| Branch Preview URL: | https://nightt5879-mind-arrow-key-na.plait.pages.dev |
Deploying plait-docs with
|
| Latest commit: |
3abcb42
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b3825d69.plait-docs.pages.dev |
| Branch Preview URL: | https://nightt5879-mind-arrow-key-na.plait-docs.pages.dev |
|
Thanks for the update. The overall idea of using geometry to navigate between visible mind nodes looks good to me: collect visible nodes, filter by direction, then sort by cross-axis overlap and distance. That makes the behavior more general than relying on tree structure only. A few suggestions:
|
|
Thanks for the detailed suggestions. I pushed
Validated with:
|
|
One more thought about the navigation model: the current geometry-based lookup is a good generic fallback, but for mind maps it may not always match the user's mental model. In many cases arrow-key navigation is expected to follow the mind-map structure first: left/right should usually move between parent and child depending on the branch direction, while up/down should usually move between visible siblings. The geometry-based nearest-node lookup can then be used as a fallback when there is no clear structural target. This would make the behavior more semantic for standard mind-map navigation and avoid jumping to a visually nearby node from a different branch when a parent/child/sibling target is structurally more appropriate. Could we consider making the current geometry lookup a fallback, and add a structure-first lookup for the common parent/child/sibling navigation cases? |
|
Thanks for pointing this out. We did some research and agree with your direction: mind-map arrow navigation should be structure-first instead of purely geometry-first. I pushed an update that changes the main navigation path to use the mind node relationships first:
We also tightened the fallback so it only considers candidates with cross-axis overlap. This keeps the fallback useful for layout edge cases, but avoids jumping across unrelated branches or to distant nodes when there is no valid structural target. test use this: |
|
I tested the standard layout locally and the current behavior looks correct to me. Moving from the last right-side child to the first left-side child with ArrowDown is expected, so I don't think we need to split standard children by The remaining issue is more about how the structure navigation derives its direction model. Currently
This works for horizontal layouts, but it does not map cleanly to vertical layouts like I think we can make this more generic by reusing the layout-direction concepts already present in the codebase instead of adding more layout-specific branches. For example:
A possible model would be:
That would let horizontal, vertical, standard, and indented layouts share the same conceptual navigation model, instead of encoding |
|
Concretely, 
Maybe we could handle the first two layouts, and handle AbstractNode later. |
|
Thanks for the detailed direction. I reworked the arrow-key navigation to be layout-direction based instead of hard-coding horizontal parent/child and vertical sibling behavior. What changed in the production fix:
I added regression coverage for:
Verified locally:
I also added a separate demo commit for manual review. After checking out this branch, run
That demo commit is optional. If you prefer not to keep manual review data in the repo, we can drop |
There was a problem hiding this comment.
The structure-first approach is a good improvement, but I think the current navigation model still needs adjustment before merging, especially because nested layouts are already supported.
Problem: direction ownership in nested layouts
The effective layout of source does not necessarily describe how source is positioned relative to its parent or siblings.
For example, consider a right layout nested inside a rightTopIndented layout. The nested node's right layout should control how its own descendants expand. However, its incoming parent-child relationship and its position in the sibling sequence are still controlled by the outer layout context. In this case, the next sibling is reached with ArrowUp, but the current implementation derives sibling directions from source and incorrectly treats it as a normal right-layout node.
The direction should be resolved from the owner of each relationship:
parent -> childuses the layout context that arranges that edge;- previous/next sibling directions use the common parent's layout context;
child.layoutcontrolschild -> descendants, but does not redefine the incoming edge or sibling axis.
This also means that a single selected node may legitimately use different layout contexts for its incoming, outgoing, and sibling relationships.
Build layout-aware candidates before selecting a target
The current lookup obscures this relationship:
return (
getVisibleSiblingByDirection(board, source, direction) ||
getParentOrChildByLayoutDirection(board, source, direction, previousElement)
);This reads as "try sibling navigation first, then fall back to parent/child navigation". Parent, child, previous sibling, and next sibling are layout-aware navigation candidates rather than fallback strategies. I suggest collecting and resolving all candidates first:
type LayoutNavigationRelation =
| 'parent'
| 'child'
| 'previous-sibling'
| 'next-sibling';
interface LayoutNavigationCandidate {
element: MindElement;
relation: LayoutNavigationRelation;
direction: LayoutDirection;
}The flat candidate list avoids duplicating the relation in both a model property and the candidate itself, while preserving enough semantic information for tests and debugging.
The resulting navigation flow is:
- Convert the keyboard direction to
LayoutDirection. - Collect the visible parent, children, previous sibling, and next sibling.
- Resolve each relationship from the layout context that owns it.
- Build
LayoutNavigationCandidate[]. - Filter candidates by the requested direction.
- Resolve relation conflicts explicitly: sibling first, then parent, then a remembered child, then the first child.
- Use geometry lookup only when no layout candidate matches.
Resolve relationship directions explicitly
I suggest introducing a focused layout-direction.ts for the new navigation relation logic:
type LayoutRelation =
| {
type: 'parent-child';
parent: MindElement;
child: MindElement;
}
| {
type: 'sibling';
parent: MindElement;
order: 'previous' | 'next';
};
export const resolveLayoutRelationDirection = (
board: PlaitBoard,
relation: LayoutRelation
): LayoutDirection => {
// The implementation resolves direction from relation.parent's
// layout context, rather than from the selected source node.
};The following is an implementation sketch showing the intended ownership rules. The exact helper names can follow the existing conventions:
const isLayoutRoot = (element: MindElement) => {
return PlaitMind.isMind(element) || !!element.layout;
};
const resolveLayoutRelationDirection = (
board: PlaitBoard,
relation: LayoutRelation
): LayoutDirection => {
const layout = MindQueries.getCorrectLayoutByElement(
board,
relation.parent
) as MindLayoutType;
if (relation.type === 'sibling') {
const nextDirection = isIndentedLayout(layout)
? isTopLayout(layout)
? LayoutDirection.top
: LayoutDirection.bottom
: isHorizontalLayout(layout)
? LayoutDirection.bottom
: LayoutDirection.right;
return relation.order === 'next'
? nextDirection
: getLayoutReverseDirection(nextDirection);
}
if (isIndentedLayout(layout) && isLayoutRoot(relation.parent)) {
return isTopLayout(layout)
? LayoutDirection.top
: LayoutDirection.bottom;
}
return getLayoutDirection(
MindElement.getNode(relation.child),
isHorizontalLayout(layout)
);
};The important distinction for indented layouts is:
- the direct children of an indented layout root use the top/bottom branch direction;
- deeper parent-child relationships use the left/right hierarchy direction;
- sibling order is still derived from the common parent's layout context;
- a nested node with an explicit layout becomes the owner of the layout used for its own descendants.
These details can remain private to resolveLayoutRelationDirection; they do not need to become several public direction helpers.
Direction derivation by layout
The following examples show why the relationship owner must be explicit.
Horizontal logic layout
For a normal right layout:
parent direction = left
child direction = right
previous sibling direction = top
next sibling direction = bottom
The parent-child axis is horizontal, so siblings use the cross axis.
Upward layout
For an upward layout:
parent direction = bottom
child direction = top
previous sibling direction = left
next sibling direction = right
The same relationship model works without hard-coding left/right = parent/child and top/bottom = sibling.
Right-top indented layout
For the direct children of a right-top indented layout root:
parent direction = bottom
child direction = top
previous sibling direction = bottom
next sibling direction = top
This layout intentionally creates direction conflicts. For the second root child, ArrowDown matches both its parent and its previous sibling. The expected behavior is:
second child + ArrowDown -> previous sibling
first child + ArrowDown -> parent, because no previous sibling exists
The same applies symmetrically to a bottom-indented layout. This is why the candidate needs a relation field and why target selection must define relation priority instead of returning an arbitrary first match.
Nested rightTopIndented -> right
Consider this tree:
Outer parent: rightTopIndented
Previous sibling
Source: layout = right
Child A
Child B
Next sibling
The candidates for Source are derived from different owners:
parent candidate:
owner = outer parent layout context
previous/next sibling candidates:
owner = outer parent's rightTopIndented context
child candidates:
owner = Source's nested right layout
Therefore, the nested right layout changes navigation from Source to its descendants, but does not change how Source navigates to its parent or siblings. Deriving every direction from getCorrectLayout(board, source) cannot represent this distinction.
Standard layout
For a standard root, outgoing child candidates may point in both horizontal directions:
right-side child candidate = right
left-side child candidate = left
Sibling order is still the visible child order and uses top/bottom navigation. If the visible order is:
[rightA, rightB, leftA, leftB]
then the structural transitions are:
rightB + ArrowDown -> leftA
leftA + ArrowUp -> rightB
The left/right direction remains relevant for parent-child navigation and geometry filtering, but should not split the root sibling sequence.
Construct the candidates
const getLayoutNavigationCandidates = (
board: PlaitBoard,
source: MindElement
): LayoutNavigationCandidate[] => {
const candidates: LayoutNavigationCandidate[] = [];
const parent = getVisibleParent(source);
if (parent) {
const incomingDirection = resolveLayoutRelationDirection(board, {
type: 'parent-child',
parent,
child: source
});
candidates.push({
element: parent,
relation: 'parent',
direction: getLayoutReverseDirection(incomingDirection)
});
}
getVisibleChildren(board, source).forEach((child) => {
candidates.push({
element: child,
relation: 'child',
direction: resolveLayoutRelationDirection(board, {
type: 'parent-child',
parent: source,
child
})
});
});
if (parent) {
const siblings = getVisibleChildren(board, parent);
const sourceIndex = siblings.indexOf(source);
const previousSibling = siblings[sourceIndex - 1];
const nextSibling = siblings[sourceIndex + 1];
if (previousSibling) {
candidates.push({
element: previousSibling,
relation: 'previous-sibling',
direction: resolveLayoutRelationDirection(board, {
type: 'sibling',
parent,
order: 'previous'
})
});
}
if (nextSibling) {
candidates.push({
element: nextSibling,
relation: 'next-sibling',
direction: resolveLayoutRelationDirection(board, {
type: 'sibling',
parent,
order: 'next'
})
});
}
}
return candidates;
};For the standard layout, root children should not be filtered by left/right direction when constructing siblings. Keeping the original visible child order allows the expected transition:
last right-side sibling + ArrowDown -> first left-side sibling
first left-side sibling + ArrowUp -> last right-side sibling
The parent/child candidates still resolve their own left/right direction independently.
Select the target by direction
const resolveLayoutNavigationTarget = (
candidates: LayoutNavigationCandidate[],
direction: LayoutDirection,
previousElement?: MindElement
) => {
const matches = candidates.filter(
(candidate) => candidate.direction === direction
);
const sibling = matches.find(
(candidate) =>
candidate.relation === 'previous-sibling' ||
candidate.relation === 'next-sibling'
);
if (sibling) {
return sibling.element;
}
const parent = matches.find(
(candidate) => candidate.relation === 'parent'
);
if (parent) {
return parent.element;
}
const children = matches.filter(
(candidate) => candidate.relation === 'child'
);
const rememberedChild = children.find(
(candidate) => candidate.element === previousElement
);
return rememberedChild?.element || children[0]?.element;
};This priority is not an implementation accident. It is the explicit navigation policy for layouts where sibling and parent directions overlap:
matching sibling
-> matching parent
-> child matching previousElement
-> first matching child
The old sibling || parent/child expression happened to encode part of this priority, but it hid the reason. The candidate model keeps candidate discovery independent from target selection and makes the conflict policy testable.
The main lookup then becomes a direct pipeline instead of sibling/parent-child fallback ordering:
export const getNextMindElementByDirection = (
board: PlaitBoard,
source: MindElement,
direction: Direction,
previousElement?: MindElement
) => {
const layoutDirection = DirectionToLayoutDirection[direction];
const candidates = getLayoutNavigationCandidates(board, source);
const layoutTarget = resolveLayoutNavigationTarget(
candidates,
layoutDirection,
previousElement
);
return (
layoutTarget ||
getNextMindElementByGeometry(board, source, direction)
);
};For a node using right inside rightTopIndented, this produces the intended separation of contexts:
- parent candidate: resolved from the outer parent-child context;
- previous/next sibling candidates: resolved from the outer
rightTopIndentedcontext; - child candidates: resolved from the node's own nested
rightlayout.
Navigation invariants
The implementation should preserve these invariants:
- Only visible nodes become layout navigation candidates.
- Collapsed descendants are not child candidates.
- An incoming edge is resolved from the parent that owns that edge.
- Sibling order and sibling directions are resolved from the common parent.
- A node's nested layout only owns its outgoing descendants.
- Standard root siblings preserve visible child order across the right/left boundary.
- Sibling candidates win when sibling and parent directions overlap.
previousElementonly chooses among matching child candidates; it does not override sibling or parent navigation.- Geometry is a fallback only after no layout candidate is selected.
This separation also makes failures easier to diagnose:
wrong candidate set
-> visible tree/relation construction problem
correct relation, wrong direction
-> layout ownership/direction resolution problem
multiple correct-direction candidates, wrong target
-> relation priority/history selection problem
no layout candidate, unexpected jump
-> geometry fallback problem
Scope of layout-direction.ts
To keep this PR focused, we do not need to migrate existing direction handling in point-placement.ts, Node More, DnD, or link drawing now. Those callers have related but different semantics:
- link drawing resolves a rendered connector direction;
- Node More resolves an affordance placement direction;
- DnD resolves a hypothetical insertion direction;
- keyboard navigation resolves an existing structural relationship.
This PR can add the relation-oriented resolver for navigation while continuing to call the existing low-level getLayoutDirection internally.
As a follow-up, we should audit point-placement.ts#getLayoutDirection, getNodeMoreLayoutDirection, and the DnD direction handling, extract a shared rendered-direction primitive into layout-direction.ts, and migrate only the callers whose semantics are genuinely compatible. That follow-up should preserve existing behavior rather than turning the new module into a universal direction function with feature-specific modes.
Regression coverage
Please add coverage for:
rightTopIndented -> rightnested layout;- navigating from the outer parent into the nested-layout node;
- navigating from the nested-layout node back to its parent;
- previous/next sibling navigation at the nested-layout boundary;
- navigation among descendants inside the nested layout;
- standard-layout sibling navigation across the right/left branch boundary.
We can review the geometry fallback separately. For now, hasCrossAxisOverlap could be renamed to isInSameNavigationLane so its intent is clearer and it remains isolated from the layout-based candidate model.
|
Thoughts on This PR |
|
Thanks for the detailed review. I pushed
One deliberate refinement from the implementation sketch is that sibling direction reads the common parent's final rendered Regression coverage now includes:
Validation:
I agree that broader rendered-direction reuse and geometry-driven navigation belong in Phase 2. @pubuzhixing8, could you please take another look? |
2 participants
Summary
Why
A selected node can participate in relationships owned by different layout contexts. In a nested
rightTopIndented -> righttree, the outer layout owns the node's incoming edge and sibling sequence, while the nestedrightlayout owns its descendants.Nested subtrees may also be mirrored by an ancestor. Sibling navigation therefore uses the common parent's final rendered
up/leftorientation instead of relying only on the declared layout enum. This prevents loops such asA-1 -> A-2 -> A-1when navigating a mirrored three-sibling subtree.Scope
This PR covers layout-based keyboard navigation and keeps the existing geometry lookup as an isolated fallback. Broader geometry policy changes and shared rendered-direction refactors for Node More, DnD, link drawing, and other callers remain follow-up work.
Validation
npx ng test mind --watch=false --progress=false --browsers=ChromeHeadless --include=packages/mind/src/plugins/with-mind-hotkey.spec.ts— 24 specs passednpx ng test mind --watch=false --progress=false --browsers=ChromeHeadless— 45 specs passednpm run build:mindnpm run build:demogit diff --checkfor the changed fileshttp://localhost:4200/?init=mind-navigation-reviewA-1 ↑ A-2 ↑ 111111 ↓ A-2 ↓ A-1D ↓ E ↑ D