Skip to content

Add validation, traversal utilities and additional placement strategies#12

Closed
mass2527 wants to merge 1 commit intomainfrom
claude/implement-high-impact-work-euNkw
Closed

Add validation, traversal utilities and additional placement strategies#12
mass2527 wants to merge 1 commit intomainfrom
claude/implement-high-impact-work-euNkw

Conversation

@mass2527
Copy link
Owner

  • Add layout validation utilities (validateLayout, isValidLayoutNode,
    isValidPanelNode, isValidSplitNode) for validating layouts loaded from
    external sources like localStorage
  • Add layout traversal utilities (traverseLayout, findAllPanels,
    findAllSplits, countPanels, findNodeById, findPanelById, getPanelIds,
    getTreeDepth) for common layout operations
  • Add additional placement strategies:
    • lastPanelRightStrategy: adds panels to the right of the rightmost panel
    • lastPanelBottomStrategy: adds panels below the bottommost panel
    • equalHeightBottomStrategy: maintains equal heights when adding panels
    • createFixedPlacementStrategy: factory for custom fixed placement
  • Add comprehensive tests for all new utilities (96 new tests)

https://claude.ai/code/session_016RbWKu8BS3dCvPnovr6Ysc

- Add layout validation utilities (validateLayout, isValidLayoutNode,
  isValidPanelNode, isValidSplitNode) for validating layouts loaded from
  external sources like localStorage
- Add layout traversal utilities (traverseLayout, findAllPanels,
  findAllSplits, countPanels, findNodeById, findPanelById, getPanelIds,
  getTreeDepth) for common layout operations
- Add additional placement strategies:
  - lastPanelRightStrategy: adds panels to the right of the rightmost panel
  - lastPanelBottomStrategy: adds panels below the bottommost panel
  - equalHeightBottomStrategy: maintains equal heights when adding panels
  - createFixedPlacementStrategy: factory for custom fixed placement
- Add comprehensive tests for all new utilities (96 new tests)

https://claude.ai/code/session_016RbWKu8BS3dCvPnovr6Ysc
@claude
Copy link

claude bot commented Jan 23, 2026

Code review

Found 1 issue that needs attention:


src/validation.ts (line 355-360)

Issue: The isValidSplitNode function has a type safety bug - it accepts split nodes with null children, even though SplitNode.left and SplitNode.right are typed as LayoutNode (not nullable).

Root cause: isValidLayoutNode returns true for null values (see line 277-279 where it returns node is LayoutNode | null). This means a split node like this would incorrectly pass validation:

{
  type: "split",
  id: "split-1",
  orientation: "horizontal",
  ratio: 0.5,
  left: null,  // Invalid! Should be LayoutNode, not null
  right: { type: "panel", id: "panel-1" }
}

Suggested fix: Add explicit null checks before validating with isValidLayoutNode:

if (!("left" in nodeObj) || nodeObj.left === null || !isValidLayoutNode(nodeObj.left)) {
  return false;
}

if (!("right" in nodeObj) || nodeObj.right === null || !isValidLayoutNode(nodeObj.right)) {
  return false;
}

This ensures the type guard correctly narrows to SplitNode only when both children are non-null LayoutNode values.

Reference: src/validation.ts#L354-L361

@mass2527 mass2527 closed this Jan 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments