✨ feat(slider): update to reflect latest shadcn variant#625
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/zard/src/lib/shared/components/avatar/avatar.variants.ts (1)
41-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWrap
avatarBadgeVariantsin CVA to match variants-module pattern.All other exports in this file (
avatarVariants,fallbackVariants,imageVariants,avatarGroupVariants) use CVA. Line 41–46 should too for consistency. Update the single usage inavatar.component.tsline 96 fromavatarBadgeVariantstoavatarBadgeVariants().Suggested fix
export const avatarBadgeVariants = cva( mergeClasses( 'absolute right-0 bottom-0 z-10 inline-flex items-center justify-center rounded-full bg-primary text-primary-foreground bg-blend-color ring-2 ring-background select-none', 'group-data-[size=sm]/avatar:size-2 group-data-[size=sm]/avatar:[&>svg]:hidden', 'group-data-[size=default]/avatar:size-2.5 group-data-[size=default]/avatar:[&>svg]:size-2', 'group-data-[size=lg]/avatar:size-3 group-data-[size=lg]/avatar:[&>svg]:size-2', ), );And in
avatar.component.tsline 96:-protected readonly badgeClasses = computed(() => mergeClasses(avatarBadgeVariants, this.zBadgeClass())); +protected readonly badgeClasses = computed(() => mergeClasses(avatarBadgeVariants(), this.zBadgeClass()));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/zard/src/lib/shared/components/avatar/avatar.variants.ts` around lines 41 - 46, The avatarBadgeVariants constant is currently a plain merged class string; convert it to a CVA-style variant by replacing the mergeClasses(...) export with export const avatarBadgeVariants = cva(...) so it matches avatarVariants/fallbackVariants/imageVariants/avatarGroupVariants; then update the single usage in avatar.component.ts to call the function (change avatarBadgeVariants to avatarBadgeVariants()). Ensure you import cva where needed and keep the same class strings as the base argument so existing styling remains unchanged.libs/zard/src/lib/shared/components/slider/slider.variants.ts (1)
1-27: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRestore CVA variant-property type exports to match project convention.
The file violates the coding guideline for
**/src/lib/**/*.variants.tsfiles. WhilesliderOrientationVariantsdefines avariantsobject with properties (zOrientation), it is missing the required exported type alias. Across the repository (tree, tooltip, tabs, skeleton, sheet, separator, popover, pagination, dialog, dropdown, breadcrumb, button-group, alert-dialog, accordion), every CVA variant with properties has a correspondingZardComponentNameVariantsexport. The slider file should follow the same pattern.Proposed minimal diff
-import { cva } from 'class-variance-authority'; +import { cva, type VariantProps } from 'class-variance-authority'; @@ export const sliderOrientationVariants = cva('absolute', { @@ }); + +export type ZardSliderOrientationVariants = VariantProps<typeof sliderOrientationVariants>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/zard/src/lib/shared/components/slider/slider.variants.ts` around lines 1 - 27, Export a matching CVA variant-property type for the sliderOrientationVariants constant so the file follows the project convention; import VariantProps from 'class-variance-authority' (if not already imported) and add an exported type alias like SliderOrientationVariants = VariantProps<typeof sliderOrientationVariants> (exported) to accompany the existing sliderOrientationVariants export. This mirrors the pattern used elsewhere (e.g., TooltipVariants -> TooltipVariants type) and keeps the variant type available to consumers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/domain/pages/themes/services/theme-generator.service.ts`:
- Line 86: The exported CSS string in exportCss() contains an incorrect Tailwind
import path ('./src/app/shared/core/css/tailwind.css'); update that import to
the correct workspace path used elsewhere (e.g.
'../../../libs/zard/src/lib/shared/core/css/tailwind.css') so the pasted CSS
resolves correctly, or replace it with a package alias (e.g.
'`@zard/core/css/tailwind.css`') if the library is intended to be consumed
externally; change the import text inside the exportCss() output in
theme-generator.service.ts accordingly.
In `@libs/zard/src/lib/shared/components/field/demo/slider.ts`:
- Line 33: The demo's reactive value is a one-element array signal (protected
readonly value = signal([40])) but the UI renders value() (which stringifies the
array); update the demo text to render the scalar explicitly by using value()[0]
wherever the percent/number is displayed (e.g., in the component/template that
references value()), so the demo shows a numeric percent rather than an array
string.
In `@libs/zard/src/lib/shared/components/slider/demo/controlled.ts`:
- Around line 22-23: The demo is incorrectly using [zDefault]="value()" which
only initializes the slider instead of making it controlled; change the binding
to [zValue]="value()" so the slider reflects the signal updates, and ensure the
(zSlideIndexChange)="onSlide($event)" handler still updates the underlying
signal used by value() so the component remains fully controlled.
In `@libs/zard/src/lib/shared/components/slider/slider.component.ts`:
- Line 407: The onChange emission is inconsistent: when there's one thumb it
emits a number but for multi-thumbs it emits number[]; update the emissions to
always emit number[] to match OnChangeType and zSlideIndexChange. Specifically,
change the call sites (e.g., the line using this.onChange(currentValues.length >
1 ? currentValues : newValue) and the analogous emission in
updateThumbFromPercentage) to wrap single values as arrays (e.g.,
this.onChange(Array.isArray(currentValues) ? currentValues : [newValue]) or
always emit [newValue/currentValue]) so onChange always receives number[];
ensure any callers expecting a single number are adjusted accordingly.
- Line 467: The current sentinel check uses raw !== -1 which can incorrectly
reject valid negative slider values; update the logic in the slider component
where value is computed (the expression assigning to value that references raw,
def, min, max) to treat "unset" explicitly with null/undefined instead of
-1—i.e., consider raw == null (or raw === null || raw === undefined) as the
unset case and otherwise validate raw against min/max; adjust any callers or API
docs of SliderComponent/slider.value to stop using -1 as a sentinel or document
the change if backwards compatibility must be preserved.
- Around line 476-478: getMinMax currently returns a number[] but always yields
exactly two values; change its return type to a tuple [number, number] to
capture that invariant and improve type safety. Update the method signature of
getMinMax(): [number, number] and keep the implementation returning
[Math.max(this.zMin(), 0), Math.min(this.zMax(), 100)] (now typed as a tuple);
then update any call sites that destructure or assign its result to expect a
[number, number] tuple (e.g., const [min, max] = this.getMinMax()) so TypeScript
will enforce the two-element structure.
- Around line 241-247: The percentages computed in the protected readonly
percentages computed currently short-circuits to this.values() when this.zMax()
> 1, which yields wrong positions for custom ranges; update the computation in
the percentages computed to always derive percentages from the current range by
reading const [min, max] = [this.zMin(), this.zMax()] and mapping this.values()
through convertValueToPercentage(v, min, max); also add a small guard for
edge-case min === max (e.g., return an array of 0 or normalized values) to avoid
division-by-zero.
- Around line 229-239: The current linkedSignal for values falls back to
getMinMax() which returns [0,100], producing a two-thumb range by default;
change the fallback so when neither zValue() nor zDefault() provide values you
produce a single-thumb value (e.g., the minimum). Update the values computation
(the readonly values linkedSignal) so after calling getMinMax() you check the
returned array and, if it has two elements, return an array with only the first
element (e.g., [min]) to ensure a single-thumb slider; reference zValue(),
zDefault(), getMinMax(), and the readonly values linkedSignal when making the
change.
- Around line 102-104: Guard against an empty percent array before using p[0]:
update the logic around percent() (variable p) in slider.component.ts so that
when p.length === 0 you return a full-range segment (e.g., use make('0',
'100%')), and keep the existing single-value branch for p.length === 1 (use
make('0', 100 - p[0] + '%')). Ensure you check p.length explicitly rather than
relying on p[0] being defined to avoid producing NaN for the CSS value.
In `@libs/zard/src/lib/shared/core/css/tailwind.css`:
- Around line 1-89: Add a linting override so the custom Tailwind v4 at-rules
used here (`@theme`, `@custom-variant`, `@utility`) are allowed by Stylelint/Biome:
update your Stylelint config (e.g., .stylelintrc) to include "ignoreAtRules":
["theme","custom-variant","utility"] or install/enable the Tailwind plugin that
recognizes v4 directives, or add a Biome CSS rule exception to allow these
at-rules; this ensures the declarations (accordion-down/up keyframes and the
custom-variant/utility blocks) won’t be flagged if CSS linting is enabled later.
---
Outside diff comments:
In `@libs/zard/src/lib/shared/components/avatar/avatar.variants.ts`:
- Around line 41-46: The avatarBadgeVariants constant is currently a plain
merged class string; convert it to a CVA-style variant by replacing the
mergeClasses(...) export with export const avatarBadgeVariants = cva(...) so it
matches avatarVariants/fallbackVariants/imageVariants/avatarGroupVariants; then
update the single usage in avatar.component.ts to call the function (change
avatarBadgeVariants to avatarBadgeVariants()). Ensure you import cva where
needed and keep the same class strings as the base argument so existing styling
remains unchanged.
In `@libs/zard/src/lib/shared/components/slider/slider.variants.ts`:
- Around line 1-27: Export a matching CVA variant-property type for the
sliderOrientationVariants constant so the file follows the project convention;
import VariantProps from 'class-variance-authority' (if not already imported)
and add an exported type alias like SliderOrientationVariants =
VariantProps<typeof sliderOrientationVariants> (exported) to accompany the
existing sliderOrientationVariants export. This mirrors the pattern used
elsewhere (e.g., TooltipVariants -> TooltipVariants type) and keeps the variant
type available to consumers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3949cf93-59d2-4503-a157-71f4c6598e04
⛔ Files ignored due to path filters (20)
apps/web/public/r/avatar.jsonis excluded by!apps/web/public/**and included byapps/**apps/web/public/r/core.jsonis excluded by!apps/web/public/**and included byapps/**apps/web/public/r/empty.jsonis excluded by!apps/web/public/**and included byapps/**apps/web/public/r/field.jsonis excluded by!apps/web/public/**and included byapps/**apps/web/public/r/progress.jsonis excluded by!apps/web/public/**and included byapps/**apps/web/public/r/registry.jsonis excluded by!apps/web/public/**and included byapps/**apps/web/public/r/slider.jsonis excluded by!apps/web/public/**and included byapps/**apps/web/src/generated/components/field/demo/slider.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/components/progress/demo/controlled.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/components/slider/demo/controlled.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/components/slider/demo/default.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/components/slider/demo/disabled.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/components/slider/demo/min-max.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/components/slider/demo/multiple.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/components/slider/demo/range.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/components/slider/demo/vertical.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/installation/manual/alert.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/installation/manual/avatar.tsis excluded by!**/generated/**and included byapps/**apps/web/src/generated/installation/manual/slider.tsis excluded by!**/generated/**and included byapps/**packages/cli/src/core/registry/registry-data.tsis excluded by!packages/**and included by none
📒 Files selected for processing (23)
apps/web/src/app/domain/pages/themes/components/theme-preview/theme-preview.component.htmlapps/web/src/app/domain/pages/themes/components/theme-sidebar/theme-sidebar.component.htmlapps/web/src/app/domain/pages/themes/components/theme-sidebar/theme-sidebar.component.tsapps/web/src/app/domain/pages/themes/services/theme-generator.service.tsapps/web/src/app/widget/components/zard-code-box/zard-code-box.component.htmlapps/web/src/styles.csslibs/zard/src/lib/shared/components/avatar/avatar.component.tslibs/zard/src/lib/shared/components/avatar/avatar.variants.tslibs/zard/src/lib/shared/components/field/demo/slider.tslibs/zard/src/lib/shared/components/progress/demo/controlled.tslibs/zard/src/lib/shared/components/slider/demo/controlled.tslibs/zard/src/lib/shared/components/slider/demo/default.tslibs/zard/src/lib/shared/components/slider/demo/disabled.tslibs/zard/src/lib/shared/components/slider/demo/min-max.tslibs/zard/src/lib/shared/components/slider/demo/multiple.tslibs/zard/src/lib/shared/components/slider/demo/range.tslibs/zard/src/lib/shared/components/slider/demo/slider.tslibs/zard/src/lib/shared/components/slider/demo/vertical.tslibs/zard/src/lib/shared/components/slider/doc/api.tslibs/zard/src/lib/shared/components/slider/slider.component.spec.tslibs/zard/src/lib/shared/components/slider/slider.component.tslibs/zard/src/lib/shared/components/slider/slider.variants.tslibs/zard/src/lib/shared/core/css/tailwind.css
💤 Files with no reviewable changes (1)
- libs/zard/src/lib/shared/components/slider/demo/min-max.ts
9 tasks
Luizgomess
approved these changes
May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was done? 📝
This branch makes these changes:
1. Slider multi-thumb support — Refactored
ZardSliderComponentfrom single-value to array-based values.zDefaultandzValueinputs now acceptnumber[], enabling single thumb ([50]), range ([20, 80]), and multi-thumb sliders. Added thumb collision prevention (thumbs can't cross), segment-based range rendering inZSliderRangeComponent, and click-to-nearest-thumb logic.2. Extracted shared Tailwind CSS — Moved custom variants (
data-open,data-checked,data-disabled,data-horizontal,data-vertical, etc.) and keyframe animations fromapps/web/src/styles.cssintolibs/zard/src/lib/shared/core/css/tailwind.cssso they can be reused by the CLI registry for consumers installing components.3. Avatar
SafeUrlsupport — WidenedzSrcinput type fromstringtostring | SafeUrlto accept sanitized URLs.4. Demo/doc updates — Added
controlledandmultipledemos, renamedmin-maxtomultiple, updated all existing demos to use array syntax, fixed API doc type forzSlideIndexChange(number[]), and updatedzard-code-boxwith[&>*]:size-fullfor full-width slider display.Screenshots or GIFs 📸
|-----Figma-----|
|-----Implementation-----|
Link to Issue 🔗
Solves #547
Type of change 🏗
Breaking change 🚨
zDefaultchanged fromnumber→number[]—[zDefault]="50"breaks, must be[zDefault]="[50]"zValuechanged fromnumber→number[]— Same,[zValue]="x()"must be[zValue]="[x()]"zSlideIndexChangeoutput changed fromnumber→number[]— Handlers expectingnumberwill get an array insteadZSliderRangeComponent.percentchanged fromnumber→number[]— If consumed directly (internal sub-component, unlikely external usage)Checklist 🧐
Summary by CodeRabbit
Release Notes
New Features
Documentation