refactor(ui-rnative-visualization): align axis API with react-visualization + fix tick values#730
Open
gamegee wants to merge 3 commits into
Open
refactor(ui-rnative-visualization): align axis API with react-visualization + fix tick values#730gamegee wants to merge 3 commits into
gamegee wants to merge 3 commits into
Conversation
When `xAxis.data` is provided without explicit `ticks`, `buildTicksData` now derives ticks from the data itself (numeric values for numeric data, indices for string data) instead of falling back to `scale.ticks()`. This removes d3-invented intermediate ticks (e.g. `1, 3` for `[0, 2, 4]`, or `0.5, 1.5` between `'Jan', 'Feb', 'Mar'`). The existing `ticks: []` short-circuit (passing an empty array to render no labels) is preserved. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Consolidate axis configuration onto a single `BaseAxisProps` type (merging the former `AxisConfigProps` fields: `scaleType`, `data`, `domain`), add a `nice` opt-out on numeric scales (default `true`; set `false` to keep the domain exactly as provided), and cap `toScaledPoints` iteration at `xData.length` so a series with more points than the axis has labels no longer overflows the right edge of the chart. Co-authored-by: Cursor <cursoragent@cursor.com>
Group the tick-value source-of-truth fix and the axis API alignment refactor into a single patch version plan for @ledgerhq/lumen-ui-rnative-visualization. Co-authored-by: Cursor <cursoragent@cursor.com>
gamegee
force-pushed
the
dls-/761/fix-xaxis-rnative
branch
from
5c8f421 to
d1e2350
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the React Native visualization axis configuration to better align with the react-visualization API, while also fixing x-axis tick generation so ticks match xAxis.data exactly (no intermediate “invented” d3 ticks). It also adjusts chart rendering behavior to avoid series overflowing past the x-axis label range and updates the RN sandbox demo accordingly.
Changes:
- Derive tick values from
xAxis.datawhen provided (preserving theticks: []short-circuit) and add tests for the new tick priority behavior. - Consolidate axis configuration into
BaseAxisProps, add numeric-scaleniceopt-out, and captoScaledPointsiteration toxData.length. - Update CartesianChart overflow handling and refresh the
app-sandbox-rnativeLineCharts examples.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/ui-rnative-visualization/src/lib/utils/types.ts | Switches axis config types to BaseAxisProps in chart context types. |
| libs/ui-rnative-visualization/src/lib/utils/ticks/ticks.ts | Updates tick resolution priority to prefer axis.data over scale.ticks(). |
| libs/ui-rnative-visualization/src/lib/utils/ticks/ticks.test.ts | Adds coverage for tick derivation from numeric/string axis.data and priority rules. |
| libs/ui-rnative-visualization/src/lib/utils/scales/scales.ts | Adds nice parameter support to numeric scale creation and updates scale-type typing. |
| libs/ui-rnative-visualization/src/lib/utils/scales/scales.test.ts | Adds tests verifying default nice behavior and explicit nice: false. |
| libs/ui-rnative-visualization/src/lib/utils/index.ts | Removes exports for the old axis config types from utils barrel. |
| libs/ui-rnative-visualization/src/lib/utils/domain/domain.ts | Migrates axis config typing to BaseAxisProps. |
| libs/ui-rnative-visualization/src/lib/Components/Scrubber/utils.ts | Migrates scrubber axis config typing to BaseAxisProps. |
| libs/ui-rnative-visualization/src/lib/Components/Scrubber/types.ts | Adds an optional wrapper style prop to support overflow/offset layout. |
| libs/ui-rnative-visualization/src/lib/Components/Scrubber/ScrubberProvider.tsx | Applies wrapper style and updates memo dependencies. |
| libs/ui-rnative-visualization/src/lib/Components/ReferenceLine/utils.ts | Migrates reference line axis config typing to BaseAxisProps. |
| libs/ui-rnative-visualization/src/lib/Components/LineChart/types.ts | Updates xAxis/yAxis prop types to the new axis props shape. |
| libs/ui-rnative-visualization/src/lib/Components/LineChart/LineChart.tsx | Uses default axis props and passes unified axis configs through to chart + axes. |
| libs/ui-rnative-visualization/src/lib/Components/LineChart/LineChart.stories.tsx | Adds stories demonstrating numeric/string xAxis.data tick behavior. |
| libs/ui-rnative-visualization/src/lib/Components/Line/utils.ts | Caps point iteration to xData.length to prevent right-edge overflow. |
| libs/ui-rnative-visualization/src/lib/Components/Line/utils.test.ts | Adds tests for capped iteration, numeric xData, categorical centering, etc. |
| libs/ui-rnative-visualization/src/lib/Components/CartesianChart/utils.ts | Reworks overflow buffer documentation and replaces negative-margin approach with offset approach. |
| libs/ui-rnative-visualization/src/lib/Components/CartesianChart/types.ts | Migrates CartesianChart axis config typing to BaseAxisProps. |
| libs/ui-rnative-visualization/src/lib/Components/CartesianChart/context/useBuildChartContext.ts | Threads nice through scale building using BaseAxisProps. |
| libs/ui-rnative-visualization/src/lib/Components/CartesianChart/context/useBuildChartContext.test.ts | Adds integration tests for default nice behavior and opt-out. |
| libs/ui-rnative-visualization/src/lib/Components/CartesianChart/CartesianChart.tsx | Updates SVG sizing/offset handling and passes wrapper style to scrubber provider. |
| libs/ui-rnative-visualization/src/lib/Components/Axis/index.ts | Introduces an Axis barrel export for types/constants. |
| libs/ui-rnative-visualization/src/lib/Components/Axis/Axis.types.ts | Defines AxisBounds and consolidates axis config into BaseAxisProps (incl. nice). |
| libs/ui-rnative-visualization/src/lib/Components/Axis/Axis.constants.ts | Adds default axis prop objects for x/y axis. |
| apps/app-sandbox-rnative/src/app/blocks/LineCharts.tsx | Updates the sandbox LineCharts demo to match the refactored chart API and sizing behavior. |
| .nx/version-plans/version-plan-1780320400001-rnative-visualization.md | Adds a patch version plan describing the tick fix + axis API refactor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+11
to
+13
| scaleType: 'linear', | ||
| nice: false, | ||
| }; |
Comment on lines
+69
to
+73
| getXAxisConfig: (id?: string) => BaseAxisProps | undefined; | ||
| /** | ||
| * Returns the y-axis config. Accepts an optional axis ID for future multi-axis support. | ||
| */ | ||
| getYAxisConfig: (id?: string) => AxisConfigProps | undefined; | ||
| getYAxisConfig: (id?: string) => BaseAxisProps | undefined; |
Comment on lines
+45
to
+47
| // The SVG canvas is enlarged by the overflow buffer on every side so edge | ||
| // content (labels, points, ticks) is not clipped, then shifted back by the | ||
| // negative margin so the drawing area still spans the container footprint. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Split out from #728 so the react-native-visualization axis work lands independently.
fix(charts): respectxAxis.dataas the source of truth for tick values —buildTicksDataderives ticks from the data itself instead ofscale.ticks(), removing d3-invented intermediate ticks. Theticks: []short-circuit is preserved.refactor(charts): align the axis API withreact-visualization— consolidate axis config onto a singleBaseAxisPropstype, add aniceopt-out on numeric scales (defaulttrue), and captoScaledPointsiteration atxData.lengthso a series with more points than the axis has labels no longer overflows the right edge.app-sandbox-rnativeLineCharts demo accordingly.Test plan
nx test ui-rnative-visualizationapp-sandbox-rnativerenders ticks fromxAxis.dataand clips series that exceed the axis lengthMade with Cursor