ResponsiveContainer only shrinks in the dimensions where it needs to shrink#6367
ResponsiveContainer only shrinks in the dimensions where it needs to shrink#6367
Conversation
3abd8ce to
26db330
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes an issue with ResponsiveContainer where it was applying overflow styles unconditionally, causing problems when using aspect ratios with fixed dimensions. The fix ensures that overflow styles are only applied when the dimension is percentage-based and needs to shrink.
- Extracted chart dimension calculation logic into a reusable utility function
- Added conditional overflow styling that only applies when dimensions are percentage-based
- Updated type definitions to be more specific about percentage vs number values
Reviewed Changes
Copilot reviewed 10 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/component/responsiveContainerUtils.ts |
New utility file containing extracted dimension calculation and conditional overflow styling logic |
src/component/ResponsiveContainer.tsx |
Refactored to use new utility functions and improved type definitions |
src/util/types.ts |
Added Percent type alias for better type safety |
src/util/DataUtils.ts |
Updated isPercent function to use the new Percent type |
test/component/responsiveContainerUtils.spec.ts |
Comprehensive test coverage for the new utility functions |
test-vr/tests/ResponsiveContainer.spec-vr.tsx |
Updated test descriptions for clarity |
scripts/vitest-build.config.ts |
Added .stryker-tmp to exclude list |
scripts/snapshots/*.txt |
Updated build snapshots to include new utility file |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6367 +/- ##
==========================================
+ Coverage 61.68% 61.74% +0.06%
==========================================
Files 409 410 +1
Lines 37903 37967 +64
Branches 4317 4330 +13
==========================================
+ Hits 23380 23444 +64
Misses 14409 14409
Partials 114 114 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 7.11kB (0.29%) ⬆️. 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-es6Assets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
|
|
Hm okay so this makes things worse than before. Back to the drawing board |
|
Okay much better now |
| width?: number; | ||
| } | ||
|
|
||
| export type Percent = `${number}%`; |
There was a problem hiding this comment.
this is technically a breaking change.
But I think since things were either checked for percentage or casted to a number we should be ok
There was a problem hiding this comment.
Or a bugfix - typescript should have flagged strings other than percentage strings, because those won't work anyway
Description
Only apply the overflow when necessary, not always.
Related Issue
Fixes #6245 I think
Hope it doesn't cause more trouble than it solves!
Types of changes
Checklist: