Skip to content

feat(sankey): add left and right props for layout padding control#516

Merged
mattrothenberg merged 1 commit into
cloudflare:mainfrom
lkipke:feat/sankey-left-right-props
May 15, 2026
Merged

feat(sankey): add left and right props for layout padding control#516
mattrothenberg merged 1 commit into
cloudflare:mainfrom
lkipke:feat/sankey-left-right-props

Conversation

@lkipke
Copy link
Copy Markdown
Contributor

@lkipke lkipke commented May 15, 2026

Summary

  • Adds left and right optional props to SankeyChart (number | string) for controlling horizontal padding of the Sankey layout within its container
  • When omitted, ECharts defaults apply ('5%'). Passing left={0} right={0} eliminates whitespace for a full-width layout
  • Includes tests, a full-width demo, and docs updates

Motivation

ECharts defaults left and right to 5%, but in practice the right side ends up with ~20% whitespace because ECharts auto-calculates extra space for right-aligned node labels. Consumers currently have to hack around this by grabbing the ECharts instance from the DOM after mount and calling setOption({ series: [{ left: 0, right: 0 }] }) — which is fragile and races with kumo's own option updates. First-class props are the clean solution.

Changes

File Change
packages/kumo/src/components/chart/SankeyChart.tsx Add left/right to props interface, destructure in component, conditionally spread onto series config, add to useMemo deps
packages/kumo/src/components/chart/SankeyChart.test.tsx 4 new tests: omitted (no keys in config), numeric zero, string percentages, pixel numbers
packages/kumo-docs-astro/src/components/demos/Chart/SankeyChartDemo.tsx Add SankeyChartFullWidthDemo
packages/kumo-docs-astro/src/pages/charts/sankey.astro Add "Full-Width Layout" example section
.changeset/sankey-left-right-props.md Minor changeset
  • Reviews
  • bonk has reviewed the change
  • automated review not possible because: new feature requires human review of API design
  • Tests
  • Tests included/updated

Expose ECharts' series-level left and right options on SankeyChart so
consumers can control horizontal padding without post-mount hacks.
When omitted, ECharts defaults apply (5%). Passing left={0} right={0}
eliminates whitespace for a full-width layout.

- Add left/right to SankeyChartProps (number | string)
- Conditionally spread onto series config
- Add tests for omitted, numeric, and string values
- Add full-width demo and docs section
@lkipke lkipke force-pushed the feat/sankey-left-right-props branch from ada5b9c to 2f2b4a1 Compare May 15, 2026 10:37
@mattrothenberg
Copy link
Copy Markdown
Collaborator

/bonk

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented May 15, 2026

PR #516 Summary: Sankey Chart Layout Padding Props

Author: @lkipke
Status: Open, awaiting review
Scope: Minor feature addition to @cloudflare/kumo


Changes Overview

This PR adds two new optional props (left and right) to the SankeyChart component, allowing consumers to control the horizontal padding of the Sankey layout within its container.

File Change
SankeyChart.tsx Adds left and right props (number | string), conditionally spreads them to the ECharts series config
SankeyChart.test.tsx 4 new test cases covering: omitted props, numeric zero values, percentage strings, and pixel numbers
SankeyChartDemo.tsx New SankeyChartFullWidthDemo component showing left={0} right={0} usage
sankey.astro New "Full-Width Layout" documentation section with live example
.changeset/ Minor changeset for versioning

Technical Details

API Design:

interface SankeyChartProps {
  left?: number | string;   // px value or percentage (e.g., "5%")
  right?: number | string;  // px value or percentage
}

Implementation approach:

  • Uses conditional spreading (...(left !== undefined && { left })) to avoid passing keys when props are omitted, preserving ECharts defaults
  • Props added to the useMemo dependency array for proper reactivity
  • Accepts both numbers (pixels) and strings (percentages), matching ECharts' native API

Test Coverage

The 4 new tests verify:

  1. Props omitted → no left/right keys in series config (ECharts defaults apply)
  2. left={0} right={0} → values passed correctly
  3. String percentages ("10%", "15%") → forwarded as-is
  4. Pixel numbers (20, 30) → forwarded as-is

Documentation & Demos

  • Demo: New SankeyChartFullWidthDemo shows a full-width layout with zero padding
  • Docs: Added "Full-Width Layout" section explaining the use case (eliminating ECharts' default ~20% right-side whitespace caused by auto-calculated label spacing)

Current Activity

  • @mattrothenberg triggered /bonk (likely for automated review/CI)
  • PR includes proper changeset, tests, and documentation per project conventions
  • PR checklist indicates: automated review not possible (new API requires human review), tests included

github run

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 15, 2026

npm i https://pkg.pr.new/@cloudflare/kumo@516

commit: 2f2b4a1

@github-actions
Copy link
Copy Markdown
Contributor

Docs Preview

View docs preview

Commit: 2f2b4a1

@github-actions
Copy link
Copy Markdown
Contributor

Visual Regression Report — 13 changed, 25 unchanged

13 screenshot(s) with visual changes:

Button / Variant: Outline

204 px (0.2%) changed

Before After Diff
Before After Diff

Button / Loading State

4 px (0%) changed

Before After Diff
Before After Diff

Button / Disabled State

117 px (0.12%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Combobox

130 px (0.13%) changed

Before After Diff
Before After Diff

Dialog (Open)

0 px (0%) changed

Before After Diff
Before After Diff

Select / Select Sizes

901 px (0.48%) changed

Before After Diff
Before After Diff

Select / Select Without Label

105 px (0.1%) changed

Before After Diff
Before After Diff

Select / Select With Tooltip

294 px (0.29%) changed

Before After Diff
Before After Diff

Select / Select Loading

0 px (0%) changed

Before After Diff
Before After Diff

Select / Select Complex

1,093 px (0.92%) changed

Before After Diff
Before After Diff

Select / Select Grouped

346 px (0.34%) changed

Before After Diff
Before After Diff

Select / Select Grouped With Disabled

736 px (0.72%) changed

Before After Diff
Before After Diff

Select (Open)

175 px (0%) changed

Before After Diff
Before After Diff
25 screenshot(s) unchanged
  • Button / Basic
  • Button / Variant: Primary
  • Button / Variant: Secondary
  • Button / Variant: Ghost
  • Button / Variant: Destructive
  • Button / Variant: Secondary Destructive
  • Button / Sizes
  • Button / With Icon
  • Button / Icon Only
  • Button / Title
  • Button / Link as Button
  • Dialog / Dialog With Actions
  • Dialog / Dialog Basic
  • Dialog / Dialog Alert
  • Dialog / Dialog Confirmation
  • Dialog / Dialog With Select
  • Dialog / Dialog With Dropdown
  • Select / Select Basic
  • Select / Select With Field
  • Select / Select Placeholder
  • Select / Select Custom Rendering
  • Select / Select Multiple
  • Select / Select Disabled Options
  • Select / Select Disabled Items
  • Select / Select Long List

Generated by Kumo Visual Regression

@mattrothenberg mattrothenberg merged commit 3d80fe7 into cloudflare:main May 15, 2026
16 checks passed
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