Fix #172 #3908
Fix #172 #3908
Conversation
|
Can you explain more of why this fixes the issue? |
|
Pulled this down - this is definitely a breaking change. Try running the demo project and looking at Height gets quite messed up (though everything is much much faster) |
|
@ckifer I wanted to make non-breaking changes. I've made the changes again. |
|
The test is failing. Please wait for a moment |
HHongSeungWoo
force-pushed
the
fix/flexbox
branch
from
d1dc357
to
6dd44ef
Compare
I've fixed the issue where the flexbox wasn't shrinking due to fixed inner item length I'm not good at English, so I can't provide a long explanation. 😄 😭 |
|
Okay, it's late here so I'll take a look tomorrow. And you are good at English! |
| @@ -162,6 +164,7 @@ export const ResponsiveContainer = forwardRef<HTMLDivElement, Props>( | |||
| style={{ ...style, width, height, minWidth, minHeight, maxHeight }} | |||
| ref={containerRef} | |||
| > | |||
| <style>{`.recharts-wrapper > .recharts-surface {width: 100%; height: 100%;}`}</style> | |||
There was a problem hiding this comment.
I don't think its a good idea to have a style tag in the body of the document. Is there any way we can do this another way?
There was a problem hiding this comment.
I removed the style tags and added inline styles.
|
@HHongSeungWoo https://www.chromatic.com/test?appId=63da8268a0da9970db6992aa&id=653fe597bee4c9fdc1a8f6c2 That diff is concerning and is introduced by this PR. Why would we have one |
|
https://github.com/recharts/recharts/blob/master/src/component/Text.tsx#L213 Seems the Other things are affected by this too |
|
Ah... I hadn't thought of wrapping individual components in a container. |
|
We feel that we are allowing too much, which results in terrible side effects. |
|
https://www.chromatic.com/test?appId=63da8268a0da9970db6992aa&id=653fe597bee4c9fdc1a8f6af |
|
+1, completely agree 😓. This is inherited legacy as I've said before, if I could restart from the ground up I would not allow the "features" that cause issues like this like implicit props and prop spreading |
imo it should be maintained for now yes as changing it would be/is breaking. If there are issues with center alignment we can change that in v3+ |
|
Ok~! I'll make the changes and PR when ready |
<!--- Provide a general summary of your changes in the Title above --> ## Description rendering improvement This is a follow-up PR for #3908. ## Related Issue #3908 #1767 <!--- This project only accepts pull requests related to open issues --> <!--- If suggesting a new feature or change, please discuss it in an issue first --> <!--- If fixing a bug, there should be an issue describing it with steps to reproduce --> <!--- Please link to the issue here: --> ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Screenshots (if appropriate): ## Types of changes <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] My code follows the code style of this project. - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [ ] I have added tests to cover my changes. - [ ] I have added a storybook story or extended an existing story to show my changes - [x] All new and existing tests passed.
<!--- Provide a general summary of your changes in the Title above --> ## Description I have fixed issue recharts#172 ## Related Issue recharts#172 ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Screenshots (if appropriate): ## Types of changes <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] My code follows the code style of this project. - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [ ] I have added tests to cover my changes. - [x] I have added a storybook story or extended an existing story to show my changes - [x] All new and existing tests passed.
<!--- Provide a general summary of your changes in the Title above --> ## Description rendering improvement This is a follow-up PR for recharts#3908. ## Related Issue recharts#3908 recharts#1767 <!--- This project only accepts pull requests related to open issues --> <!--- If suggesting a new feature or change, please discuss it in an issue first --> <!--- If fixing a bug, there should be an issue describing it with steps to reproduce --> <!--- Please link to the issue here: --> ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Screenshots (if appropriate): ## Types of changes <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] My code follows the code style of this project. - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [ ] I have added tests to cover my changes. - [ ] I have added a storybook story or extended an existing story to show my changes - [x] All new and existing tests passed.
Description
I have fixed issue #172
Related Issue
#172
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: