feat(chart): add responsive prop - #31
Conversation
…tsWrapper Follows Recharts 3.3+ by adding a `responsive` prop to chart components: the chart root fills its parent via CSS and self-measures with a ResizeObserver, so no ResponsiveContainer wrapper is needed. Non-responsive behavior is unchanged. Also renames the misnamed RechartsWrapper (React library name) to ChartsWrapper, matching the ChartsSurface convention. - generateCategoricalChart: new `responsive` prop, measured size drives layout - ChartsWrapper: `responsive`/`onResize` props, 100% CSS sizing + ResizeObserver - update Sankey/Sunburst/Treemap consumers to ChartsWrapper - add responsive-prop tests and Examples/ResponsiveProp story - document the `responsive` prop in the chart-size guide
rick-hup
changed the title
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduces a ChangesResponsive Chart Sizing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChartComponent
participant ChartsWrapper
participant ResizeObserver
User->>ChartComponent: mount chart with responsive prop
ChartComponent->>ChartsWrapper: render(responsive=true, onResize=handleResize)
ChartsWrapper->>ChartsWrapper: measure via getBoundingClientRect
ChartsWrapper->>ResizeObserver: observe(wrapperEl)
ChartsWrapper->>ChartComponent: onResize(width, height)
ChartComponent->>ChartComponent: update effectiveWidth/effectiveHeight
ChartComponent-->>User: render Surface when size valid
ResizeObserver-->>ChartsWrapper: size change detected
ChartsWrapper->>ChartComponent: onResize(newWidth, newHeight)
ChartComponent-->>User: re-render with updated dimensions
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Deploying vue-charts with
|
| Latest commit: |
7ce723b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://79765609.vue-charts.pages.dev |
| Branch Preview URL: | https://sour-quiet.vue-charts.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/content/2.guides/05.chart-size.md (1)
20-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a live
::chart-demo::for the newresponsivesection.The
responsiveprop is presented as the recommended approach, but only code snippets are shown here, while the "alternative"ResponsiveContainersection below still includes a live::chart-demo{}::embed. Adding a live demo would keep the docs consistent and better showcase the recommended path.As per coding guidelines, "Use MDC syntax
::chart-demo{src=\"...\"}::to embed live demos in documentation."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/2.guides/05.chart-size.md` around lines 20 - 47, Add a live chart demo to the new responsive sizing section so the recommended `responsive` prop is showcased with the same MDC `::chart-demo{src="..."}::` pattern used elsewhere in the docs. Update the markdown near the `responsive` heading to include a demo embed alongside the existing examples, keeping it consistent with the `ResponsiveContainer` section below.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/vue/src/chart/ChartsWrapper.tsx`:
- Around line 58-73: The ResizeObserver setup in ChartsWrapper is tied to a
one-time onMounted check, so changes to props.responsive after mount are
ignored. Move the observer lifecycle logic into a watch on props.responsive (and
wrapperEl if needed) so it starts when responsive becomes true and disconnects
when it becomes false, keeping the existing onMounted/onUnmounted cleanup
behavior aligned with the reactive prop.
- Around line 62-67: The initial measurement in ChartsWrapper.tsx is using
getBoundingClientRect() while the ResizeObserver callback reads contentRect, so
the first onResize value can differ from later updates when padding or borders
are present. Update the resize logic in the same flow that creates
resizeObserver so both measurements use the same box model, preferably by
observing wrapperEl.value with box: 'border-box' or by changing the initial read
to match contentRect.
---
Nitpick comments:
In `@docs/content/2.guides/05.chart-size.md`:
- Around line 20-47: Add a live chart demo to the new responsive sizing section
so the recommended `responsive` prop is showcased with the same MDC
`::chart-demo{src="..."}::` pattern used elsewhere in the docs. Update the
markdown near the `responsive` heading to include a demo embed alongside the
existing examples, keeping it consistent with the `ResponsiveContainer` section
below.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 634e7cda-2190-45ce-9391-cf0e4ab1b2e5
📒 Files selected for processing (9)
docs/content/2.guides/05.chart-size.mdpackages/vue/src/chart/ChartsWrapper.tsxpackages/vue/src/chart/Sankey.tsxpackages/vue/src/chart/SunburstChart.tsxpackages/vue/src/chart/Treemap.tsxpackages/vue/src/chart/__stories__/ResponsiveProp.stories.tsxpackages/vue/src/chart/__tests__/Treemap.spec.tsxpackages/vue/src/chart/__tests__/responsive-prop.spec.tsxpackages/vue/src/chart/generateCategoricalChart.tsx
| onMounted(() => { | ||
| if (!props.responsive || !wrapperEl.value) { | ||
| return | ||
| } | ||
| const { width, height } = wrapperEl.value.getBoundingClientRect() | ||
| props.onResize?.(width, height) | ||
| resizeObserver = new ResizeObserver((entries) => { | ||
| const { width: w, height: h } = entries[0].contentRect | ||
| props.onResize?.(w, h) | ||
| }) | ||
| resizeObserver.observe(wrapperEl.value) | ||
| }) | ||
| onUnmounted(() => { | ||
| resizeObserver?.disconnect() | ||
| }) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
responsive prop is only checked once at mount, not reactive.
The ResizeObserver setup/teardown is gated by props.responsive inside onMounted's closure, which runs a single time. If a consumer toggles responsive after the component mounts, the inline style will correctly switch (it's computed in the render function), but the observer will never start (if it was initially false) or never stop (if it was initially true), leaving the chart's measured size permanently stale. Guidelines recommend mapping effects with reactive dependencies to watch, not a one-shot lifecycle hook.
♻️ Suggested fix using `watch`
- let resizeObserver: ResizeObserver | null = null
- onMounted(() => {
- if (!props.responsive || !wrapperEl.value) {
- return
- }
- const { width, height } = wrapperEl.value.getBoundingClientRect()
- props.onResize?.(width, height)
- resizeObserver = new ResizeObserver((entries) => {
- const { width: w, height: h } = entries[0].contentRect
- props.onResize?.(w, h)
- })
- resizeObserver.observe(wrapperEl.value)
- })
- onUnmounted(() => {
- resizeObserver?.disconnect()
- })
+ let resizeObserver: ResizeObserver | null = null
+ function startObserving() {
+ if (!wrapperEl.value) return
+ const { width, height } = wrapperEl.value.getBoundingClientRect()
+ props.onResize?.(width, height)
+ resizeObserver = new ResizeObserver((entries) => {
+ const { width: w, height: h } = entries[0].contentRect
+ props.onResize?.(w, h)
+ })
+ resizeObserver.observe(wrapperEl.value)
+ }
+ function stopObserving() {
+ resizeObserver?.disconnect()
+ resizeObserver = null
+ }
+ watch(() => props.responsive, (responsive) => {
+ stopObserving()
+ if (responsive) startObserving()
+ }, { immediate: true })
+ onUnmounted(stopObserving)As per coding guidelines, packages/vue/src/**/*.{ts,tsx} should "map React useState/useEffect to Vue ref/watch" when porting Recharts patterns; a prop-dependent effect like this should be reactive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/vue/src/chart/ChartsWrapper.tsx` around lines 58 - 73, The
ResizeObserver setup in ChartsWrapper is tied to a one-time onMounted check, so
changes to props.responsive after mount are ignored. Move the observer lifecycle
logic into a watch on props.responsive (and wrapperEl if needed) so it starts
when responsive becomes true and disconnects when it becomes false, keeping the
existing onMounted/onUnmounted cleanup behavior aligned with the reactive prop.
Source: Coding guidelines
| const { width, height } = wrapperEl.value.getBoundingClientRect() | ||
| props.onResize?.(width, height) | ||
| resizeObserver = new ResizeObserver((entries) => { | ||
| const { width: w, height: h } = entries[0].contentRect | ||
| props.onResize?.(w, h) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does ResizeObserver.observe support a {box: 'border-box'} option to get border-box measurements directly?
💡 Result:
Yes, the ResizeObserver.observe method supports the box option, which allows you to specify whether the observer should track changes to the element's content-box (the default) or its border-box [1][2][3]. You can use it by passing an options object as the second argument to the observe method [2][3]: resizeObserver.observe(element, { box: 'border-box' }); When this option is set, the observer monitors the size of the element's border box as defined in CSS [1][3]. This option specifically determines which box the observer monitors for layout changes [3]. Note that the ResizeObserver API also provides additional options such as 'device-pixel-content-box' for tracking sizes in device pixels [2][4][3].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver/observe
- 2: https://github.com/mdn/content/blob/main/files/en-us/web/api/resizeobserver/observe/index.md
- 3: https://drafts.csswg.org/resize-observer/
- 4: https://www.w3.org/TR/resize-observer-1/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target file with line numbers
sed -n '1,220p' packages/vue/src/chart/ChartsWrapper.tsx | cat -nRepository: unovue/vue-charts
Length of output: 7935
Align the resize observer with the initial measurement. getBoundingClientRect() returns the border box, but ResizeObserver defaults to contentRect; if props.style adds padding or a border, the first onResize value will differ from later updates. Use resizeObserver.observe(wrapperEl.value, { box: 'border-box' }) or make both paths measure the same box.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/vue/src/chart/ChartsWrapper.tsx` around lines 62 - 67, The initial
measurement in ChartsWrapper.tsx is using getBoundingClientRect() while the
ResizeObserver callback reads contentRect, so the first onResize value can
differ from later updates when padding or borders are present. Update the resize
logic in the same flow that creates resizeObserver so both measurements use the
same box model, preferably by observing wrapperEl.value with box: 'border-box'
or by changing the initial read to match contentRect.
Replace the hand-rolled ResizeObserver + onMounted/onUnmounted lifecycle with @vueuse/core's useResizeObserver, which owns the observer lifecycle and auto- disconnects on unmount. Keep the initial getBoundingClientRect to avoid a first-frame flash. Test targets the active (latest) observer instance since useResizeObserver re-creates its observer when the target ref resolves.
1 participant
What
Two related changes:
1. New
responsiveprop (aligns with Recharts 3.3+)Recharts did not remove
ResponsiveContainer; it added aresponsiveprop on the chart itself as a more flexible, CSS-driven alternative. This PR ports that:responsive?: boolean(defaultfalse) to all charts via the sharedgenerateCategoricalChartfactory.width/height: 100%) and measures itself with aResizeObserver— noResponsiveContainerwrapper needed.ReportMainChartProps → setChartSize → reduxchain, so downstream layout/selectors are untouched.ResponsiveContaineris kept for backward compatibility.Non-responsive charts keep their exact previous behavior (early bail on invalid size); responsive charts render the wrapper first so it can be measured, then gate the inner surface on a valid measured size.
Notes
aspectin responsive mode is delegated to CSSaspectRatio(no custom calc).Testing
responsive-prop.spec.tsx(5 cases: 100% sizing, gated-until-measured, renders at measured size, updates on ResizeObserver, non-responsive regression).pnpm --filter vccs buildpasses; eslint clean on new/edited files.Examples/ResponsivePropstory and aresponsivesection in the chart-size guide.Summary by CodeRabbit
New Features
Documentation
Tests