themesberg / flowbite-react

Official React components built for Flowbite and Tailwind CSS
https://flowbite-react.com
MIT License
1.78k stars 396 forks source link

fix(ButtonGroup): dynamic generated button with group wasn't styled properly #1273

Closed dhavalveera closed 4 months ago

dhavalveera commented 5 months ago

Dynamically generated buttons within a button group are not properly styled.

fix #1269

image

stackblitz[bot] commented 5 months ago

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

vercel[bot] commented 5 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
flowbite-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 12, 2024 0:39am
codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 82.85714% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 97.29%. Comparing base (7461173) to head (9d328a5). Report is 197 commits behind head on main.

Files Patch % Lines
src/components/Button/ButtonGroup.tsx 82.85% 6 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dhavalveera commented 5 months ago

@SutuSebastian - please have a look & review the fix of the bug mentioned in Issue #1269 .

dhavalveera commented 4 months ago

Hello @SutuSebastian , Sorry to bother you, but can you please review this PR?

dhavalveera commented 4 months ago

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

Yeah!. Let's find a proper solution for it

dhavalveera commented 4 months ago

@SutuSebastian - this codecov issue again

chunkerchunker commented 3 months ago

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

Yeah!. Let's find a proper solution for it

Suggested fix in https://github.com/themesberg/flowbite-react/pull/1323