Skip to content

Add support for left-aligned Sankey nodes#6568

Merged
ckifer merged 3 commits intorecharts:mainfrom
dbnlAI:sankey-alignment
Nov 5, 2025
Merged

Add support for left-aligned Sankey nodes#6568
ckifer merged 3 commits intorecharts:mainfrom
dbnlAI:sankey-alignment

Conversation

@dbnl-renaud
Copy link
Contributor

@dbnl-renaud dbnl-renaud commented Nov 4, 2025

Description

Add support for left-aligned Sankey nodes.

Related Issue

#6567

Motivation and Context

This helps with user-journey like diagrams by aligning all nodes with the same logical depth.

How Has This Been Tested?

Tested locally with Storybook.

Screenshots (if appropriate):

justify (default)

Screenshot 2025-11-04 at 9 36 30 AM

left

Screenshot 2025-11-04 at 9 36 40 AM

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have added a storybook story or VR test, or extended an existing story or VR test to show my changes

Summary by CodeRabbit

  • New Features
    • Added alignment control to Sankey diagrams. The new align prop enables 'left' mode (nodes aligned to the left edge) or 'justify' mode (start nodes aligned left, end nodes aligned right) for customizable node positioning.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

Adds an align prop ('left' | 'justify') to the Sankey component to control node layout alignment. The prop is threaded through getNodesTree and computeData functions, affecting whether root depth is forced to maxDepth. Includes visual regression tests and API documentation updates.

Changes

Cohort / File(s) Summary
Sankey component alignment feature
src/chart/Sankey.tsx
Introduces new align prop to SankeyProps. Updates getNodesTree to accept align parameter and conditionally force root depth only when align === 'justify'. Extends computeData and SankeyImpl to propagate align throughout computation pipeline. Updates sankeyDefaultProps to include align: 'justify' and memoization dependencies.
Visual regression tests
test-vr/tests/SankeyChart.spec-vr.tsx
Adds new VR test file with two test cases: one for align="justify" and one for align="left", verifying Sankey rendering with different alignment configurations via screenshot assertions.
API documentation
www/src/docs/api/Sankey.ts
Adds align prop to SankeyAPI documentation with type 'left' | 'justify', default 'justify', and descriptive text explaining alignment behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Primary changes consist of consistent parameter threading through function signatures and memoization dependencies
  • Single conditional logic addition in getNodesTree to control behavior based on align value
  • VR tests follow established patterns for the test suite
  • Review should focus on verifying the conditional logic in getNodesTree and confirming memoization dependencies are correctly updated

Suggested reviewers

  • ckifer

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main feature addition: support for left-aligned Sankey nodes.
Description check ✅ Passed The PR description comprehensively covers all required template sections: description, related issue, motivation/context, testing approach, screenshots, change type selection, and a completed checklist confirming documentation updates, tests, and VR tests were added.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4142f89 and 4f463e0.

⛔ Files ignored due to path filters (6)
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-justify-1-chromium-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-justify-1-firefox-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-justify-1-webkit-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-left-1-chromium-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-left-1-firefox-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-left-1-webkit-linux.png is excluded by !**/*.png
📒 Files selected for processing (3)
  • src/chart/Sankey.tsx (9 hunks)
  • test-vr/tests/SankeyChart.spec-vr.tsx (1 hunks)
  • www/src/docs/api/Sankey.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • test-vr/tests/SankeyChart.spec-vr.tsx
🧰 Additional context used
📓 Path-based instructions (2)
www/**

📄 CodeRabbit inference engine (DEVELOPING.md)

Use the www directory to add and commit examples for the documentation website (recharts.github.io)

Files:

  • www/src/docs/api/Sankey.ts
src/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not hardcode any user-facing strings or formatting choices in the library code; leave internationalization to library users

Files:

  • src/chart/Sankey.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-25T07:36:02.229Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T07:36:02.229Z
Learning: Recharts aims for simple, declarative, and composable charts; prioritize consistency, usability, performance, and accessibility

Applied to files:

  • www/src/docs/api/Sankey.ts
🧬 Code graph analysis (1)
src/chart/Sankey.tsx (1)
src/util/types.ts (2)
  • SankeyNode (915-929)
  • SankeyLink (931-938)
🔇 Additional comments (6)
src/chart/Sankey.tsx (5)

107-149: LGTM! Clean implementation of alignment logic.

The align parameter is properly threaded through getNodesTree, and the conditional logic correctly preserves the existing "justify" behavior (forcing leaf nodes to maxDepth) while enabling the new "left" behavior (allowing nodes to stay at their natural depth). The implementation is minimal and focused.


308-353: LGTM! Proper parameter propagation.

The align parameter is correctly added to the computeData function signature and properly propagated to getNodesTree. The type annotation is consistent with the rest of the codebase.


508-508: LGTM! Type-safe prop definition.

The align prop is correctly defined as an optional property with a string literal union type, providing good type safety.


824-824: LGTM! Backward-compatible default.

The default value of 'justify' ensures backward compatibility by preserving the existing behavior.


845-892: LGTM! Complete integration with proper memoization.

The align prop is correctly destructured (line 845), passed to computeData (line 867), and included in the useMemo dependency array (line 892). This ensures that changes to the align prop properly trigger recomputation of the Sankey layout.

www/src/docs/api/Sankey.ts (1)

224-233: LGTM! Clear and accurate documentation.

The documentation accurately describes the new align prop with the correct type, default value, and a clear explanation of the behavior difference between 'justify' and 'left' alignment modes. The description helps users understand when to use each option.


Comment @coderabbitai help to get the list of available commands and usage tips.

@PavelVanecek
Copy link
Collaborator

Thanks for the PR @dbnl-renaud , I would like to see at least 1 screenshot test please, check out the test-vr directory for examples.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
www/src/docs/api/SankeyChart.ts (1)

224-233: Clarify the description for both alignment modes.

The description for 'left' mode only mentions start nodes, leaving the behavior of end nodes and intermediate nodes unclear. Consider expanding the description to provide a complete picture of both modes.

For example:

  • 'justify': Distributes nodes across the full width—start nodes align to the left edge, end nodes to the right edge, and intermediate nodes are spaced between them.
  • 'left': All nodes align based on their computed depth from the start, resulting in a left-aligned layout where end nodes may not reach the right edge.

This will help users understand the visual difference and choose the appropriate mode for their use case.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 74915d2 and b9cceb3.

📒 Files selected for processing (1)
  • www/src/docs/api/SankeyChart.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
www/**

📄 CodeRabbit inference engine (DEVELOPING.md)

Use the www directory to add and commit examples for the documentation website (recharts.github.io)

Files:

  • www/src/docs/api/SankeyChart.ts
🧠 Learnings (1)
📚 Learning: 2025-10-25T07:36:02.229Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T07:36:02.229Z
Learning: Recharts aims for simple, declarative, and composable charts; prioritize consistency, usability, performance, and accessibility

Applied to files:

  • www/src/docs/api/SankeyChart.ts

@dbnl-renaud
Copy link
Contributor Author

@PavelVanecek I added some screenshot test and updated the documentation.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
test-vr/tests/SankeyChart.spec-vr.tsx (1)

1-19: LGTM! Well-structured test data and imports.

The imports are minimal and correct, and the data structure appropriately represents a user journey flow suitable for testing both alignment modes.

Consider typing the data object for better type safety:

+import type { SankeyData } from '../../src';
+
-const data = {
+const data: SankeyData = {
   nodes: [

However, this is a minor refinement and not critical for VR tests.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b9cceb3 and 4142f89.

⛔ Files ignored due to path filters (6)
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-justify-1-chromium-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-justify-1-firefox-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-justify-1-webkit-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-left-1-chromium-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-left-1-firefox-linux.png is excluded by !**/*.png
  • test-vr/__snapshots__/tests/SankeyChart.spec-vr.tsx-snapshots/Sankey-align-left-1-webkit-linux.png is excluded by !**/*.png
📒 Files selected for processing (1)
  • test-vr/tests/SankeyChart.spec-vr.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Applies to test/**/*.test.{ts,tsx} : Mock getBoundingClientRect using test/helper/MockGetBoundingClientRect.ts when rendering Recharts components (e.g., Tooltip, Legend, charts)

Applied to files:

  • test-vr/tests/SankeyChart.spec-vr.tsx
📚 Learning: 2025-10-25T07:34:46.558Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-10-25T07:34:46.558Z
Learning: Applies to test-vr/__snapshots__/** : Commit newly recorded or updated visual regression snapshot files under test-vr/__snapshots__ to the repository

Applied to files:

  • test-vr/tests/SankeyChart.spec-vr.tsx
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Applies to test/**/*.test.{ts,tsx} : When testing selectors, verify render counts using the spy and rerenderSameComponent from createSelectorTestCase

Applied to files:

  • test-vr/tests/SankeyChart.spec-vr.tsx
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Write unit tests using Vitest and React Testing Library

Applied to files:

  • test-vr/tests/SankeyChart.spec-vr.tsx
🔇 Additional comments (2)
test-vr/tests/SankeyChart.spec-vr.tsx (2)

21-28: Excellent test coverage for default justify alignment.

This test correctly validates the existing default behavior with a screenshot, directly addressing the reviewer's request for visual regression testing.


30-37: Perfect coverage for the new left alignment feature.

This test validates the new align="left" functionality with a screenshot. Together with the justify test, you've provided complete visual coverage of both alignment modes as requested by the reviewer.

The test structure is clean and consistent. The code duplication between the two tests is appropriate for VR tests, as it maintains clarity and makes each test independently understandable.

@codecov
Copy link

codecov bot commented Nov 4, 2025

Bundle Report

Changes will increase total bundle size by 123 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
recharts/bundle-cjs 1.12MB 123 bytes (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
chart/Sankey.js 123 bytes 26.66kB 0.46%

@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.27%. Comparing base (debce07) to head (4f463e0).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6568   +/-   ##
=======================================
  Coverage   91.27%   91.27%           
=======================================
  Files         490      490           
  Lines       40911    40928   +17     
  Branches     4581     4581           
=======================================
+ Hits        37341    37358   +17     
  Misses       3554     3554           
  Partials       16       16           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ckifer ckifer merged commit 7fb128b into recharts:main Nov 5, 2025
23 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Nov 6, 2025
7 tasks
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.

3 participants