Fix/range slider align #1222
Fix/range slider align #1222
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1222 +/- ##
==========================================
- Coverage 99.54% 97.35% -2.19%
==========================================
Files 163 214 +51
Lines 6621 9043 +2422
Branches 401 530 +129
==========================================
+ Hits 6591 8804 +2213
- Misses 30 239 +209 ☔ View full report in Codecov by Sentry. |
| <input | ||
| ref={ref} | ||
| type="range" | ||
| className={twMerge(theme.field.input.base, theme.field.input.sizes[sizing], className)} |
There was a problem hiding this comment.
Since you're doing this, it would be helpful to consolidate the theme API here since we're making the component much simpler, as it should be. It seems like you could change the theme to literally just base and sizes keys.
Remember to consult the Flowbite vanilla docs on RangeSlider for the classes in the theme. I think we have a mismatch already, before your PR.
There was a problem hiding this comment.
@tulup-conner Good point, thanks!
remove base wrappers according to vanilla implementation BREAKING CHANGE: className prop applies directly to input
remove tests for field base and add test for className prop
remove unused field theme props BREAKING CHANGE: RangeSlider theme API changed
SutuSebastian
force-pushed
the
fix/range-slider-align
branch
from
08f79f7 to
a897072
Compare
| @@ -34,16 +22,7 @@ export const RangeSlider = forwardRef<HTMLInputElement, RangeSliderProps>( | |||
|
|
|||
| return ( | |||
| <> | |||
There was a problem hiding this comment.
in this case, do we still need the fragment (<>)?
| @@ -5,7 +5,7 @@ import { describe, expect, it, vi } from 'vitest'; | |||
| import { Flowbite, type CustomFlowbiteTheme } from '../Flowbite'; | |||
| import { RangeSlider } from './RangeSlider'; | |||
|
|
|||
| describe('Components / Button', () => { | |||
| describe('Components / RangeSlider', () => { | |||
There was a problem hiding this comment.
good catch & cleanup
Removed field base wrapper from RangeSlider according to vanilla Flowbite range slider implementation to fix issue with component vertical alignment
Fixes #1202
Breaking API changes:
classNameprops now applies to input instead of component field base wrapper.FlowbiteRangeSliderThemenow acceptsbaseandsizesprops only.