Skip to content

Remove Cell examples from storybooks, fix RadialBar shape type and selector stability#6917

Merged
ckifer merged 7 commits intomainfrom
cell
Jan 26, 2026
Merged

Remove Cell examples from storybooks, fix RadialBar shape type and selector stability#6917
ckifer merged 7 commits intomainfrom
cell

Conversation

@PavelVanecek
Copy link
Collaborator

@PavelVanecek PavelVanecek commented Jan 22, 2026

Description

In this PR:

  • I have removed all remaining Cell examples from storybook
  • Most of them were already covered on the website, except RadialBar + Cell
  • So I added a new website example
  • In that process I found that RadialBar.shape type is awkward to work with so I fixed that
  • Also I figured why not have some CSS transitions on the clicks to make it look nice
  • Found out that wasn't working because Recharts was re-mounting the RadialBar on each render due to un-stable selector
  • So I fixed that too and added a test

Related Issue

#6645

#6668

#6734

Summary by CodeRabbit

  • New Features

    • Added a RadialBar Chart example demonstrating interactive legend-based focus selection.
  • Documentation

    • Updated RadialBar storybook entry (simplified API story).
    • Removed several legacy Pie and Bar example stories.
  • Performance & UX

    • Reduced render overhead and improved stability in radial bar rendering.
  • Tests

    • Added selector stability and visual tests for RadialBar examples.
  • Chores

    • Cleaned up TypeScript annotations and refined exported typings.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Walkthrough

Refactors RadialBar typing and rendering, introduces memoized RadialBar computations, adds replacePolarGraphicalItem and memoized SetPolarGraphicalItem, tightens TypeScript checks in tests, removes several Storybook examples, and adds a new interactive RadialBarChartClickToFocusLegendExample.

Changes

Cohort / File(s) Summary
Legend & Payload Types
src/component/DefaultLegendContent.tsx
Broadened LegendPayload.payload typing to allow unknown/object; added getStrokeDasharray helper and adjusted icon rendering to safely extract strokeDasharray.
Public Exports
src/index.ts
Added RadialBarSectorProps type export from ./util/RadialBarUtils.
RadialBar Core
src/polar/RadialBar.tsx, src/util/RadialBarUtils.tsx
Switched sector generics to RadialBarSectorProps; made index and isActive required on sector props; removed extra Layer wrappers; added useMemo for cells and radialBarSettings; coerced isActive to boolean.
Polar State Management
src/state/SetGraphicalItem.ts, src/state/graphicalItemsSlice.ts, src/state/polarOptionsSlice.ts
Wrapped SetPolarGraphicalItem with memoized impl using prevPropsRef and layout effect; added replacePolarGraphicalItem reducer/action; changed updatePolarOptions to in-place conditional update.
Type-checking/Test Cleanups
src/state/selectors/pieSelectors.ts, src/state/selectors/radialBarSelectors.ts, test/chart/PieChart.spec.tsx, test/component/Tooltip/Tooltip.visibility.spec.tsx, test/component/Legend.spec.tsx
Removed several @ts-expect-error suppressions; adjusted comments for legend payload typing; replaced test wrapper usage with direct component invocation.
Storybook Examples Removed
storybook/stories/Examples/Pie/PieWithCells.stories.tsx, storybook/stories/Examples/Pie/PieWithPatterns.stories.tsx, storybook/stories/Examples/Pie/PieWithTooltip.stories.tsx, storybook/stories/Examples/cartesian/Bar/CustomizedEvent.stories.tsx
Deleted four example story files and their exports.
RadialBar Examples & Stories
storybook/stories/API/chart/RadialBarChart.stories.tsx, www/src/docs/exampleComponents/RadialBarChart/RadialBarChartClickToFocusLegendExample.tsx, www/src/docs/exampleComponents/RadialBarChart/index.ts
Simplified API story (renamed/exported as API with name "Simple"); added new interactive RadialBarChartClickToFocusLegendExample and registered it in examples index.
Tests & VR
test/state/selectors/radialBarSelectors.spec.tsx, test-vr/tests/www/RadialBarChartApiExamples.spec-vr.tsx
Added unit test for selectRadialBarSectors stability; added VR test mounting the new interactive example.

Sequence Diagram(s)

sequenceDiagram
participant User as "User"
participant Legend as "LegendItem (click)"
participant Context as "SelectedLabelContext (provider)"
participant Chart as "RadialBarChart"
participant Sector as "RadialBarSector"

User->>Legend: click label
Legend->>Context: update selected label
Context-->>Chart: provides updated selection
Chart->>Sector: re-render with isActive (index-aware)
Sector->>Chart: visual update (highlight/focus)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

refactor

Suggested reviewers

  • ckifer
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: removing Cell examples, fixing RadialBar shape type, and addressing selector stability issues.
Description check ✅ Passed The PR description includes motivation, related issues, and a clear summary of changes, but lacks sections on testing details and component checklist items.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

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: 1

🤖 Fix all issues with AI agents
In
`@www/src/docs/exampleComponents/RadialBarChart/RadialBarChartClickToFocusLegendExample.tsx`:
- Around line 42-59: LegendItem renders a clickable <li> without keyboard
handlers; make it keyboard-accessible by adding a focusable role and key
handling: give the element a tabIndex={0} and role="button", remove the
eslint-disable comments, and implement an onKeyDown handler on LegendItem that
listens for Enter or Space and calls setSelectedLabel(l) (same action as
onClick); keep existing visual styles and use the same selectedLabel logic from
SelectedLabelContext to preserve behavior.

Comment on lines +42 to +59
const LegendItem = ({ entry }: { entry: LegendPayload }) => {
const { selectedLabel, setSelectedLabel } = useContext(SelectedLabelContext);
// @ts-expect-error label is always present in our case, the typings are just not accurate enough
const l = entry.payload.label;
const isSelected = selectedLabel === l || selectedLabel == null;
return (
// eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-noninteractive-element-interactions
<li
onClick={() => setSelectedLabel(l)}
style={{
color: entry.color,
opacity: isSelected ? 1 : 0.2,
transition: 'opacity 0.3s ease',
}}
>
{l}
</li>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make legend items keyboard-accessible (remove a11y suppressions).
Clickable <li> without keyboard handling blocks non-mouse users.

✅ Proposed fix
-  return (
-    // eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-noninteractive-element-interactions
-    <li
-      onClick={() => setSelectedLabel(l)}
-      style={{
-        color: entry.color,
-        opacity: isSelected ? 1 : 0.2,
-        transition: 'opacity 0.3s ease',
-      }}
-    >
-      {l}
-    </li>
-  );
+  return (
+    <li>
+      <button
+        type="button"
+        onClick={() => setSelectedLabel(l)}
+        style={{
+          color: entry.color,
+          opacity: isSelected ? 1 : 0.2,
+          transition: 'opacity 0.3s ease',
+          background: 'none',
+          border: 'none',
+          padding: 0,
+          cursor: 'pointer',
+        }}
+      >
+        {l}
+      </button>
+    </li>
+  );
🤖 Prompt for AI Agents
In
`@www/src/docs/exampleComponents/RadialBarChart/RadialBarChartClickToFocusLegendExample.tsx`
around lines 42 - 59, LegendItem renders a clickable <li> without keyboard
handlers; make it keyboard-accessible by adding a focusable role and key
handling: give the element a tabIndex={0} and role="button", remove the
eslint-disable comments, and implement an onKeyDown handler on LegendItem that
listens for Enter or Space and calls setSelectedLabel(l) (same action as
onClick); keep existing visual styles and use the same selectedLabel logic from
SelectedLabelContext to preserve behavior.

@codecov
Copy link

codecov bot commented Jan 22, 2026

Codecov Report

❌ Patch coverage is 99.37107% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.28%. Comparing base (bc2c877) to head (82a5641).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...rChart/RadialBarChartClickToFocusLegendExample.tsx 98.63% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6917      +/-   ##
==========================================
+ Coverage   94.26%   94.28%   +0.01%     
==========================================
  Files         569      570       +1     
  Lines       55728    55854     +126     
  Branches     5185     5205      +20     
==========================================
+ Hits        52532    52661     +129     
+ Misses       3187     3184       -3     
  Partials        9        9              

☔ 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.

@github-actions
Copy link
Contributor

Staging Deployment Details

These deployments will remain available for 30 days.

To update snapshots: Comment /update-snapshots on this PR to automatically update the baseline screenshots.

strokeDasharray?: number | string;
value?: any;
};
payload?: unknown;
Copy link
Member

Choose a reason for hiding this comment

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

can we keep value? This is a breaking change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm the trouble is that this type was a lie - some graphical elements are passing its own props, some other elements are passing the data entry. unknown or even any is the best we can do here. Unless we unify the behaviour across all graphical items which is even more breaking change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suppose we can make it an object because that much is true.

@github-actions
Copy link
Contributor

Staging Deployment Details

These deployments will remain available for 30 days.

To update snapshots: Comment /update-snapshots on this PR to automatically update the baseline screenshots.

@codecov
Copy link

codecov bot commented Jan 24, 2026

Bundle Report

Changes will increase total bundle size by 1.58MB (130.73%) ⬆️⚠️, exceeding the configured threshold of 5%.

Bundle name Size Change
recharts/bundle-cjs 1.21MB 1.73kB (0.14%) ⬆️
recharts/bundle-es6 1.05MB 1.05MB (100%) ⬆️⚠️
recharts/bundle-umd 530.2kB 530.2kB (100%) ⬆️⚠️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
polar/RadialBar.js 167 bytes 21.48kB 0.78%
state/selectors/radialBarSelectors.js 29 bytes 12.75kB 0.23%
component/DefaultLegendContent.js 89 bytes 7.41kB 1.22%
state/selectors/pieSelectors.js 29 bytes 4.61kB 0.63%
state/graphicalItemsSlice.js 530 bytes 3.38kB 18.62% ⚠️
state/SetGraphicalItem.js 566 bytes 2.81kB 25.25% ⚠️
state/polarOptionsSlice.js 319 bytes 926 bytes 52.55% ⚠️

* Different graphical items put different information in the payload object
* so double check in runtime what are you getting here.
*/
payload?: object;
Copy link
Member

Choose a reason for hiding this comment

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

I suppose:

{
   [string]: any
}

is roughly the same type

@ckifer ckifer merged commit 0e971fd into main Jan 26, 2026
47 of 48 checks passed
@ckifer ckifer deleted the cell branch January 26, 2026 17:11
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