src/chart/Treemap.tsx (2)

122-128: Missing explicit return type on getTreemapRenderHeight.

Suggested fix

-const getTreemapRenderHeight = (height: number, type: Props['type']) => {
+const getTreemapRenderHeight = (height: number, type: Props['type']): number => {

As per coding guidelines: "Type function parameters and return values explicitly."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/chart/Treemap.tsx` around lines 122 - 128, The function
getTreemapRenderHeight lacks an explicit return type; add an explicit return
type of number to its signature (e.g., getTreemapRenderHeight = (height: number,
type: Props['type']): number => ...) so the function's return value is
statically typed; update the declaration in Treemap.tsx where
getTreemapRenderHeight is defined and ensure the return paths (both the height -
NEST_INDEX_HEIGHT and height) conform to number.

---

458-468: JSDoc descriptions for nodePadding and nodeGap are too vague for autogenerated API docs.

nodePadding is described as "The padding between the nodes" but it actually insets the layout area used for a node's children (parent-child inset). nodeGap is described as "The gap between the nodes" but specifically applies between sibling nodes at the same depth. Since these JSDoc comments drive the autogenerated API docs, they should clearly communicate the distinction.

📝 Suggested JSDoc improvement

   /**
-   * The padding between the nodes.
+   * Insets the layout area used for a node's children, creating visual separation between parent and child nodes.
    * `@default` 0
    */
   nodePadding?: number;

   /**
-   * The gap between the nodes.
+   * Spacing between sibling nodes at the same depth.
    * `@default` 0
    */
   nodeGap?: number;

As per coding guidelines: "JSDoc comments and TypeScript definitions in source files must be kept up-to-date as they are used to autogenerate API documentation via 'npm run omnidoc'"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/chart/Treemap.tsx` around lines 458 - 468, Update the JSDoc for the
nodePadding and nodeGap props in Treemap.tsx to clearly differentiate their
behavior: change the nodePadding description (nodePadding?: number) to state
that it insets the layout area used for a node’s children (i.e., a
parent-to-children inset), and change the nodeGap description (nodeGap?: number)
to state that it controls the spacing between sibling nodes at the same depth
(i.e., gap between adjacent children). Keep the default values and type
annotations unchanged, but make the descriptions explicit so autogenerated docs
reflect the parent-child inset vs sibling-gap distinction.

test/chart/Treemap.spec.tsx (1)

13-28: Consider adding a test combining nodePadding and nodeGap together.

Currently padding and gap are tested in isolation. A single test with multiLevelInsetData using both nodePadding={8} and nodeGap={6} would verify no interference between the two features. Not urgent since the story exercises both and the algorithm applies them independently.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/chart/Treemap.spec.tsx` around lines 13 - 28, Add a new test in
Treemap.spec.tsx that renders the Treemap with multiLevelInsetData and both
nodePadding={8} and nodeGap={6} to ensure they don't interfere; use the same
rendering helper as other tests (e.g., the test's renderTreemap or
renderWithProviders) to mount the tree, then assert that expected nodes (like
"Root A", "A1", "A1a") are present and that their bounding boxes/positions
reflect separation (e.g., non-overlapping rects or distinct x/y/top/left values)
to confirm both padding and gap are applied concurrently.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

www/src/docs/exampleComponents/TreeMap/TreemapWithPaddingAndGaps.tsx (1)

88-88: The * 5 fix resolves the out-of-bounds bug, but was previously recommended to use * colors.length instead.

The previously flagged * 6 was changed to * 5, which prevents colors[5] === undefined, but the agreed fix was to use colors.length for resilience against future palette changes.

♻️ Proposed fix

-          fill: depth < 2 ? colors[Math.floor((index / (root?.children?.length ?? 1)) * 5)] : '#ffffff00',
+          fill: depth < 2 ? colors[Math.floor((index / (root?.children?.length ?? 1)) * colors.length)] : '#ffffff00',

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@www/src/docs/exampleComponents/TreeMap/TreemapWithPaddingAndGaps.tsx` at line
88, Replace the hardcoded multiplier 5 with the palette's length to avoid
out-of-bounds access: when computing the color index inside
TreemapWithPaddingAndGaps (the expression using colors[Math.floor((index /
(root?.children?.length ?? 1)) * 5)]), use colors.length instead of 5 so the
computed index is always within colors' bounds (refer to symbols colors, index,
root?.children?.length, and the fill assignment).

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

www/src/docs/exampleComponents/TreeMap/TreemapWithPaddingAndGaps.tsx (1)

88-88: Prefer colors.length over the hardcoded 5 for maintainability.

The previous OOB issue (using * 6 with a 5-entry array) was fixed by changing to * 5, but colors.length would be the more maintainable fix as agreed in the earlier review thread.

♻️ Proposed refactor

-          fill: depth < 2 ? colors[Math.floor((index / (root?.children?.length ?? 1)) * 5)] : '#ffffff00',
+          fill: depth < 2 ? colors[Math.floor((index / (root?.children?.length ?? 1)) * colors.length)] : '#ffffff00',

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@www/src/docs/exampleComponents/TreeMap/TreemapWithPaddingAndGaps.tsx` at line
88, Replace the hardcoded "5" with the actual colors array length to keep the
color selection maintainable: in the TreemapWithPaddingAndGaps component change
the fill calculation that currently uses "Math.floor((index /
(root?.children?.length ?? 1)) * 5)" to use "colors.length" (e.g.,
Math.floor((index / (root?.children?.length ?? 1)) * colors.length)); reference
the variables colors, index, and root to locate the expression and ensure the
colors.length value is used instead of the magic number.

src/chart/Treemap.tsx (2)

191-204: insetRect logic is correct — minor note on redundant safeguard.

clampedPadding = Math.min(nodeInset, rect.width / 2, rect.height / 2) already guarantees width - 2*clampedPadding ≥ 0, so the subsequent Math.max(..., 0) is technically redundant. It's harmless defensive coding, but worth noting if you ever decide to simplify.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/chart/Treemap.tsx` around lines 191 - 204, The insetRect function
currently uses Math.max(..., 0) when computing width/height even though
clampedPadding = Math.min(nodeInset, rect.width / 2, rect.height / 2) guarantees
width - 2*clampedPadding and height - 2*clampedPadding are non-negative;
simplify by removing the redundant Math.max guards and directly return width:
rect.width - clampedPadding * 2 and height: rect.height - clampedPadding * 2 in
insetRect to reduce unnecessary defensive code while keeping the clampedPadding
calculation and early-return behavior intact.

---

120-128: NEST_INDEX_HEIGHT is silently coupled to renderNestIndex's DOM height — add a clarifying comment.

The constant 30 must match whatever the .recharts-treemap-nest-index-wrapper element actually occupies. If the nest-index styles change (font-size, padding, margin), this constant needs to be updated manually. A comment cross-referencing renderNestIndex would prevent future drift.

💬 Suggested comment

-const NEST_INDEX_HEIGHT = 30;
+/** Height reserved for the breadcrumb/nest-index bar rendered by `renderNestIndex()`.
+ * Must be kept in sync with the inline styles on `.recharts-treemap-nest-index-wrapper`. */
+const NEST_INDEX_HEIGHT = 30;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/chart/Treemap.tsx` around lines 120 - 128, NEST_INDEX_HEIGHT (used by
getTreemapRenderHeight) is tightly coupled to the actual DOM height of the
element created by renderNestIndex (the .recharts-treemap-nest-index-wrapper);
add a concise comment above the NEST_INDEX_HEIGHT declaration stating that the
numeric value must match the rendered height of
.recharts-treemap-nest-index-wrapper (and reference renderNestIndex) and note
that changes to its font-size/padding/margin require updating this constant or
switching to a measured/ref-based approach.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

src/chart/Treemap.tsx (1)

120-128: Good refactoring, but add explicit return type.

Extracting the magic number 30 into NEST_INDEX_HEIGHT improves readability. Per coding guidelines, add an explicit return type annotation.

Suggested fix

-const getTreemapRenderHeight = (height: number, type: Props['type']) => {
+const getTreemapRenderHeight = (height: number, type: Props['type']): number => {
   if (type === 'nest') {
     return height - NEST_INDEX_HEIGHT;
   }

   return height;
 };

As per coding guidelines: "Type function parameters and return values explicitly."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/chart/Treemap.tsx` around lines 120 - 128, The helper
getTreemapRenderHeight currently lacks an explicit return type; add a return
type annotation of number to the function signature (i.e., change
getTreemapRenderHeight = (height: number, type: Props['type']) => { ... } to
explicitly return number) so the function and its use of NEST_INDEX_HEIGHT
remain properly typed and satisfy the coding guideline requiring explicit
parameter and return types.