-
-
Notifications
You must be signed in to change notification settings - Fork 430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(ButtonGroup): dynamic generated button with group wasn't styled properly #1273
fix(ButtonGroup): dynamic generated button with group wasn't styled properly #1273
Conversation
Run & review this pull request in StackBlitz Codeflow. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1273 +/- ##
==========================================
- Coverage 99.54% 97.29% -2.26%
==========================================
Files 163 216 +53
Lines 6621 9245 +2624
Branches 401 542 +141
==========================================
+ Hits 6591 8995 +2404
- Misses 30 250 +220 ☔ View full report in Codecov by Sentry. |
@SutuSebastian - please have a look & review the fix of the bug mentioned in Issue #1269 . |
Hello @SutuSebastian , Sorry to bother you, but can you please review this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a quick fix, we need to find a way to target the Button
component while recursively searching through children, and only inject props into the button, not all children components.
let's merge this until that "final" solution appears
…t styled properly Dynamically generated buttons within a button group are not properly styled. themesberg#1269 fix themesberg#1269
33ae831
to
9d328a5
Compare
Yeah!. Let's find a proper solution for it |
@SutuSebastian - this codecov issue again |
This change fixes the following issue: ButtonGroup was adding positionInGroup to all children rather than just Buttons. For example, in the following, both the Buttons and the child spans get positionInGroup props: ``` <Button.Group><Button><span>b1</span></Button><Button><span>b2</span></Button></Button.Group> ``` This results in a React warning: “Warning: React does not recognize the `positionInGroup` prop on a DOM element...” The review comment in the original code change causing this hints at this issue as well: themesberg#1273 (comment)
Suggested fix in #1323 |
This change fixes the following issue: ButtonGroup was adding positionInGroup to all children rather than just Buttons. For example, in the following, both the Buttons and the child spans get positionInGroup props: ``` <Button.Group><Button><span>b1</span></Button><Button><span>b2</span></Button></Button.Group> ``` This results in a React warning: “Warning: React does not recognize the `positionInGroup` prop on a DOM element...” The review comment in the original code change causing this hints at this issue as well: themesberg#1273 (comment)
This change fixes the following issue: ButtonGroup was adding positionInGroup to all children rather than just Buttons. For example, in the following, both the Buttons and the child spans get positionInGroup props: ``` <Button.Group><Button><span>b1</span></Button><Button><span>b2</span></Button></Button.Group> ``` This results in a React warning: “Warning: React does not recognize the `positionInGroup` prop on a DOM element...” The review comment in the original code change causing this hints at this issue as well: themesberg#1273 (comment)
* Fix React warning in ButtonGroup This change fixes the following issue: ButtonGroup was adding positionInGroup to all children rather than just Buttons. For example, in the following, both the Buttons and the child spans get positionInGroup props: ``` <Button.Group><Button><span>b1</span></Button><Button><span>b2</span></Button></Button.Group> ``` This results in a React warning: “Warning: React does not recognize the `positionInGroup` prop on a DOM element...” The review comment in the original code change causing this hints at this issue as well: #1273 (comment) * fix formatting
* Fix React warning in ButtonGroup This change fixes the following issue: ButtonGroup was adding positionInGroup to all children rather than just Buttons. For example, in the following, both the Buttons and the child spans get positionInGroup props: ``` <Button.Group><Button><span>b1</span></Button><Button><span>b2</span></Button></Button.Group> ``` This results in a React warning: “Warning: React does not recognize the `positionInGroup` prop on a DOM element...” The review comment in the original code change causing this hints at this issue as well: themesberg/flowbite-react#1273 (comment) * fix formatting
Dynamically generated buttons within a button group are not properly styled.
fix #1269