Choose implicit Scatter ErrorBar direction based on chart layout#6159
Choose implicit Scatter ErrorBar direction based on chart layout#6159
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the ErrorBar direction logic to use chart layout context instead of prop drilling. The change removes the SetErrorBarPreferredDirection wrapper component and moves the direction determination logic directly into the useErrorBarDirection hook using the chart layout context.
- Removes the
SetErrorBarPreferredDirectioncontext provider and related prop drilling - Updates
useErrorBarDirectionhook to use chart layout context for determining direction - Simplifies Bar and Line components by removing ErrorBar direction wrapper usage
Reviewed Changes
Copilot reviewed 4 out of 25 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/cartesian/ErrorBar.tsx | Replaces context-based direction setting with direct chart layout usage |
| src/cartesian/Line.tsx | Removes SetErrorBarPreferredDirection wrapper and layout prop usage |
| src/cartesian/Bar.tsx | Removes SetErrorBarPreferredDirection wrapper and layout prop usage |
| DEVELOPING.md | Adds clarification about VR test report viewing |
| */ | ||
| import * as React from 'react'; | ||
| import { Component, createContext, SVGProps, useContext } from 'react'; | ||
| import { Component, SVGProps } from 'react'; |
There was a problem hiding this comment.
The useContext import was removed but createContext is still being removed from the same import statement. Consider cleaning up the import to only include what's actually used.
| import { Component, SVGProps } from 'react'; | |
| import { SVGProps } from 'react'; |
There was a problem hiding this comment.
This is still used, irrelevant comment
There was a problem hiding this comment.
I think the new variant makes more sense - it follows the numerical axis. Same as the Scatter points.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6159 +/- ##
==========================================
- Coverage 96.51% 96.50% -0.01%
==========================================
Files 217 217
Lines 19839 19825 -14
Branches 4083 4081 -2
==========================================
- Hits 19147 19133 -14
Misses 686 686
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will decrease total bundle size by 1.65kB (-0.07%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
|
|
Yeah we'll call this a bug fix |
Description
Bar and Line already did that, Scatter we forgot.
There is some visual diff with numerical XAxis. Technically this is a breaking change if we think of it as a feature. If we think of it as a bugfix then it's a patch ¯_(ツ)_/¯
Related Issue
Fixes #6152