Skip to content

feat(slider)!: support bidirectional fill#764

Merged
netchampfaris merged 5 commits into
mainfrom
slider-bidirectional-fill
Jun 17, 2026
Merged

feat(slider)!: support bidirectional fill#764
netchampfaris merged 5 commits into
mainfrom
slider-bidirectional-fill

Conversation

@GursheenK

@GursheenK GursheenK commented Jun 8, 2026

Copy link
Copy Markdown
Member

Bidirectional fill

When min is negative, the slider fill now originates from the zero rather than always from the left edge, making positive and negative values visually distinct. Added docs and test for the same.

Fix

SliderRoot was missing an explicit height, so the thumb (size-4 / size-5) overflowed the component's bounding box. Added h-4 for sm and h-5 for md to match the thumb dimensions so that we no longer need to compensate for the overflow in our own layouts.

Breaking change

Slider root element now has an explicit h-4 / h-5 Tailwind class. Previously the root had no height class and sized naturally to its content. Consumers who applied a custom height to the Slider container or overrode its height via CSS may see a layout shift.

Migration: if your Slider container has a fixed height smaller than h-4 (16px), increase it or remove the constraint.

Docs preview: https://ui.frappe.io/pr-preview/pr-764/

Coverage: 56.10% (+0.01% vs main)

@netchampfaris netchampfaris marked this pull request as ready for review June 17, 2026 13:51
@barista-for-frappe

barista-for-frappe Bot commented Jun 17, 2026

Copy link
Copy Markdown

Minor nits — clean change, nothing blocking.

Good calls: bidirectional fill is driven by the existing min/max props rather than a new bidirectional flag, so no public API surface is added. Tests, story, and docs all included, and the h-4/h-5 breaking change is flagged honestly in the description.

  • src/components/Slider/Slider.vue:108-114 — the hasLabeling block got re-indented (operands dropped from 6 to 4 spaces) but isn't otherwise touched by this PR. Looks like a stray Prettier reversal; revert it to keep the diff focused on the slider fill.
  • src/components/Slider/Slider.vue:151-157 — the bidirectional case hand-rolls a <div> with data-orientation/data-disabled to replace <SliderRange>. Fine, since reka's SliderRange can't fill from a midpoint, but worth a one-line comment saying why the plain div exists so a future reader doesn't 'fix' it back to SliderRange.

Logic checks out: isBidirectional correctly requires min<0 && max>0, single-thumb fills between zero-crossing and the thumb, two-thumb keeps standard between-thumbs behaviour, and the zero-width case is covered by a test.

barista · claude-opus-4-8 · 11k in / 4.2k out · 444k cached · 69s · $0.601

@netchampfaris netchampfaris merged commit 2b61876 into main Jun 17, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants