feat(FoRange): First implementation#134
Conversation
WalkthroughAdds a new Range form component FoRange with numeric v-model, props min/max/step and isDisabled, and an optional steps slot. Introduces RangeProps (extends Colorable, Sizable, Disableable). Registers FoRange in color and size mappings and adds 'FoRange' to ColorableComponentName and SizableComponentName types. Adds FoRange to the app config configurable props. Adds barrel exports for Range Types and UI, registers and documents Range in the docs site (components, pages, examples), and updates the docs sidebar. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Deploying flyonui-vue with
|
| Latest commit: |
303059b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8bd171e4.flyonui-vue.pages.dev |
| Branch Preview URL: | https://feat-range.flyonui-vue.pages.dev |
Deploying flyonui-vue-v3 with
|
| Latest commit: |
303059b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://41571587.flyonui-vue-v3.pages.dev |
| Branch Preview URL: | https://feat-range.flyonui-vue-v3.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/core/src/Lib/UseFlyonUIVueAppConfig/Types/FlyonUIVueAppConfig.ts (1)
18-19: ConsolidateRangePropsinto the existing@/UI/Formsimport.Line 18 imports all other form props from the
'@/UI/Forms'barrel.RangePropsis re-exported through that same barrel (@/UI/Forms→@/UI/Forms/Range→@/UI/Forms/Range/Types), so the separate import on Line 19 is unnecessary and inconsistent.♻️ Proposed consolidation
-import type { CheckboxProps, InputTextProps, SelectProps, SwitchProps, TextareaProps } from '@/UI/Forms'; -import type { RangeProps } from '@/UI/Forms/Range'; +import type { CheckboxProps, InputTextProps, RangeProps, SelectProps, SwitchProps, TextareaProps } from '@/UI/Forms';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/Lib/UseFlyonUIVueAppConfig/Types/FlyonUIVueAppConfig.ts` around lines 18 - 19, Consolidate the duplicate import by removing the separate import of RangeProps and adding RangeProps to the existing named import list from '@/UI/Forms'; update the import line that currently imports CheckboxProps, InputTextProps, SelectProps, SwitchProps, TextareaProps to also include RangeProps so all form prop types (including RangeProps) come from the single '@/UI/Forms' barrel export.packages/docs/Next/Forms/Range/DisabledRange.vue (1)
2-5: Open TODO:isDisabledprop rename tracked for 3.x.The inline
todocomment notes thatisDisabledwill be renamed in v3.x. Consider opening a dedicated issue to track this rename so it doesn't get lost in a doc-only file.Would you like me to draft the issue description for tracking the
isDisabled→ (new name) prop rename for v3.x?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/Next/Forms/Range/DisabledRange.vue` around lines 2 - 5, The TODO in DisabledRange.vue that notes the prop rename of isDisabled for v3.x should be turned into a tracked issue: create a new issue titled like "Rename FoRange prop isDisabled → <newName> for v3.x", reference the file DisabledRange.vue and the FoRange component, describe the current prop (is-disabled/isDisabled), the desired new prop name and desired deprecation timeline (v3.x), add migration steps/examples (how to replace isDisabled with the new prop and any compatibility shim), link any related components or docs, and assign labels/milestones (v3.x, breaking-change) so the rename isn't lost in a doc-only file.packages/docs/Next/Forms/Range/RangeDocs.vue (1)
44-97:computed()is unnecessary for a static map — thepreviewsmap has no reactive dependencies; none of its values change after mount. A plainconstwould be equivalent without the overhead.♻️ Proposed simplification
-const previews = computed(() => { - return new Map<Section, ComponentDocsPreview>([ +const previews = new Map<Section, ComponentDocsPreview>([ [ 'default', ... ], - ]); -}); +]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/Next/Forms/Range/RangeDocs.vue` around lines 44 - 97, The previews map is created with computed(...) despite having no reactive dependencies; replace the computed wrapper by declaring previews directly as a plain constant (i.e., remove computed and assign new Map<Section, ComponentDocsPreview>(...)) so previews is a non-reactive const; update any imports/usages if they expect a Ref, but keep the same Map shape and the same keys/components (references: previews, computed, Map<Section, ComponentDocsPreview>, and the preview entries like DefaultRangeRaw/DefaultRange, RangeSizeRaw/RangeSize, etc.).packages/core/src/UI/Forms/Range/UI/FoRange.vue (1)
14-14: Prefer an explicit ternary over short-circuit&&forv-bind
v-bind="slots.steps === undefined && $attrs"producesv-bind="false"when the slot is defined. Although Vue 3 handles falsyv-bindgracefully (vianormalizeProps), the intent is clearer and more idiomatic with a ternary.♻️ Proposed refactor
- v-bind="slots.steps === undefined && $attrs" + v-bind="slots.steps === undefined ? $attrs : {}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/UI/Forms/Range/UI/FoRange.vue` at line 14, Replace the short-circuit v-bind usage so intent is explicit: change the binding expression that currently reads v-bind="slots.steps === undefined && $attrs" to use a ternary based on slots.steps (e.g., v-bind="slots.steps === undefined ? $attrs : {}"), referencing the same slots.steps and $attrs identifiers in the template so when the slot exists you bind an empty object instead of producing a falsy v-bind.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/UI/Forms/Range/UI/FoRange.vue`:
- Around line 3-5: The wrapper div and input currently only receive $attrs.style
and $attrs.class when slots.steps is present, causing other attributes (id,
aria-*, data-*, etc.) to be dropped; update the component (which uses
inheritAttrs: false and checks slots.steps) to forward the remaining attributes
to the input by computing an attrsWithoutClassStyle = omit($attrs,
['class','style','steps']) (or similar) and using
v-bind="attrsWithoutClassStyle" on the <input> (keep the existing conditional
v-bind for the slots-absent case), ensuring the wrapper still gets only style
when steps exist and the input receives class plus the omitted remaining
attributes for accessibility and testability; add an inline omit helper or
import it and reference slots.steps, $attrs, the wrapper <div> and the <input>
elements when making the change.
- Line 13: The template binds :disabled="isDisabled" but the RangeProps defaults
don't initialize isDisabled; update the defineProps call for RangeProps (which
extends Disableable) in FoRange.vue to include an isDisabled default (e.g.,
isDisabled: false or undefined) consistent with other form components
(FoTextarea, FoSelect, FoInputText) so the component has a defined prop value at
runtime.
- Line 6: The range input's v-model binding in FoRange.vue is missing the
.number modifier, causing the browser's string value to be assigned to the model
declared with defineModel<number>; update the v-model on the range input
(v-model="input") to use v-model.number so the value is coerced to a number, and
ensure the corresponding defineModel<number> usage (the component's model
declaration) remains consistent with the numeric type; apply the same
v-model.number change to the other range binding referenced near the
defineModel<number> declaration.
In `@packages/docs/Next/Forms/Range.md`:
- Around line 3-19: The headings under the page title use incorrect levels and
invert nesting; change "Default", "Sizes", and "Colors" from ### to ## so they
are proper h2 sections, and change the "Solid" and "Custom" headings from ## to
### so they are nested under the "Colors" h2; keep the RangeDocs calls
(RangeDocs section="default", "size", "color", "custom-color") unchanged and
only adjust the heading markers to restore a correct h1 → h2 → h3 hierarchy.
---
Nitpick comments:
In `@packages/core/src/Lib/UseFlyonUIVueAppConfig/Types/FlyonUIVueAppConfig.ts`:
- Around line 18-19: Consolidate the duplicate import by removing the separate
import of RangeProps and adding RangeProps to the existing named import list
from '@/UI/Forms'; update the import line that currently imports CheckboxProps,
InputTextProps, SelectProps, SwitchProps, TextareaProps to also include
RangeProps so all form prop types (including RangeProps) come from the single
'@/UI/Forms' barrel export.
In `@packages/core/src/UI/Forms/Range/UI/FoRange.vue`:
- Line 14: Replace the short-circuit v-bind usage so intent is explicit: change
the binding expression that currently reads v-bind="slots.steps === undefined &&
$attrs" to use a ternary based on slots.steps (e.g., v-bind="slots.steps ===
undefined ? $attrs : {}"), referencing the same slots.steps and $attrs
identifiers in the template so when the slot exists you bind an empty object
instead of producing a falsy v-bind.
In `@packages/docs/Next/Forms/Range/DisabledRange.vue`:
- Around line 2-5: The TODO in DisabledRange.vue that notes the prop rename of
isDisabled for v3.x should be turned into a tracked issue: create a new issue
titled like "Rename FoRange prop isDisabled → <newName> for v3.x", reference the
file DisabledRange.vue and the FoRange component, describe the current prop
(is-disabled/isDisabled), the desired new prop name and desired deprecation
timeline (v3.x), add migration steps/examples (how to replace isDisabled with
the new prop and any compatibility shim), link any related components or docs,
and assign labels/milestones (v3.x, breaking-change) so the rename isn't lost in
a doc-only file.
In `@packages/docs/Next/Forms/Range/RangeDocs.vue`:
- Around line 44-97: The previews map is created with computed(...) despite
having no reactive dependencies; replace the computed wrapper by declaring
previews directly as a plain constant (i.e., remove computed and assign new
Map<Section, ComponentDocsPreview>(...)) so previews is a non-reactive const;
update any imports/usages if they expect a Ref, but keep the same Map shape and
the same keys/components (references: previews, computed, Map<Section,
ComponentDocsPreview>, and the preview entries like
DefaultRangeRaw/DefaultRange, RangeSizeRaw/RangeSize, etc.).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/core/src/UI/Forms/Range/UI/FoRange.vue (1)
3-5:⚠️ Potential issue | 🟠 MajorAttrs other than
class/styleare still silently dropped when thestepsslot is usedWhen
slots.stepsis provided,v-bind="slots.steps === undefined && $attrs"evaluates tov-bind="false"(Line 14), so only$attrs.style(wrapper, Line 4) and$attrs.class(input, Line 8) are forwarded. Every other attribute —id,aria-label,aria-describedby,data-testid,name, etc. — is lost.🐛 Proposed fix — forward remaining attrs to the input
Add the
omithelper in the<script setup>:+const omit = <T extends object>(obj: T, keys: string[]): Partial<T> => + Object.fromEntries(Object.entries(obj).filter(([k]) => !keys.includes(k))) as Partial<T>;Then update the input's
v-bind:- v-bind="slots.steps === undefined && $attrs" + v-bind="slots.steps ? omit($attrs, ['class', 'style']) : $attrs"Also applies to: 8-8, 14-14
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/UI/Forms/Range/UI/FoRange.vue` around lines 3 - 5, The wrapper currently only forwards class/style when slots.steps is present, causing other $attrs (id, aria-*, name, data-*, etc.) to be dropped; in FoRange.vue add or import an omit helper in the <script setup> and change the input element's v-bind so it forwards the remaining attributes when slots.steps exists (e.g. use v-bind="slots.steps ? omit($attrs, ['class','style']) : $attrs"), while keeping the wrapper's :class="slots.steps && 'w-full'" and :style="slots.steps && $attrs.style" behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/UI/Forms/Range/UI/FoRange.vue`:
- Line 46: Update the JSDoc comment in the FoRange.vue component that currently
reads "The step indicators to show for each steps" to the correct grammar "The
step indicators to show for each step" (look for the JSDoc above the
steps/step-indicator prop or variable in FoRange.vue and correct the text).
---
Duplicate comments:
In `@packages/core/src/UI/Forms/Range/UI/FoRange.vue`:
- Around line 3-5: The wrapper currently only forwards class/style when
slots.steps is present, causing other $attrs (id, aria-*, name, data-*, etc.) to
be dropped; in FoRange.vue add or import an omit helper in the <script setup>
and change the input element's v-bind so it forwards the remaining attributes
when slots.steps exists (e.g. use v-bind="slots.steps ? omit($attrs,
['class','style']) : $attrs"), while keeping the wrapper's :class="slots.steps
&& 'w-full'" and :style="slots.steps && $attrs.style" behavior unchanged.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/core/src/UI/Forms/Range/UI/FoRange.vue (1)
3-5:⚠️ Potential issue | 🟠 MajorNon-
class/stylefallthrough attrs are still silently dropped when thestepsslot is used.When
slots.stepsis provided:
- Line 14 evaluates to
v-bind="false"(a no-op), soid,aria-*,data-*,tabindex, and any other attribute are never forwarded to<input>.- Line 4 applies only
$attrs.styleto the wrapper div; Line 8 applies only$attrs.classto the input.This remains an accessibility blocker — consumers cannot associate a
<label for="...">with the input via a passedid, andaria-describedby/aria-labelare dropped entirely.🐛 Proposed fix — forward remaining attrs to the input via `omit`
+const omit = <T extends object>(obj: T, keys: string[]): Partial<T> => + Object.fromEntries(Object.entries(obj).filter(([k]) => !keys.includes(k))) as Partial<T>;<input v-model.number="input" class="range" :class="[colorClass, sizeClass, slots.steps && $attrs.class]" type="range" :min="min" :max="max" :step="step" :disabled="isDisabled" - v-bind="slots.steps === undefined && $attrs" + v-bind="slots.steps ? omit($attrs, ['class', 'style']) : $attrs" >Also applies to: 8-8, 14-14
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/UI/Forms/Range/UI/FoRange.vue` around lines 3 - 5, In FoRange.vue the presence of slots.steps causes $attrs to be mishandled so non-class/style attributes (id, aria-*, data-*, tabindex, etc.) are dropped; fix this by forwarding all remaining attrs to the <input> by binding an omitted-$attrs object (e.g. v-bind="omit($attrs, ['class','style'])" or equivalent) on the input element while keeping existing :class and :style bindings on the wrapper/input as-is; ensure the omit helper or manual destructuring is used so only class/style are excluded and all other $attrs (id, aria-*, data-*, tabindex) are forwarded when slots.steps is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/UI/Forms/Range/UI/FoRange.vue`:
- Around line 3-5: In FoRange.vue the presence of slots.steps causes $attrs to
be mishandled so non-class/style attributes (id, aria-*, data-*, tabindex, etc.)
are dropped; fix this by forwarding all remaining attrs to the <input> by
binding an omitted-$attrs object (e.g. v-bind="omit($attrs, ['class','style'])"
or equivalent) on the input element while keeping existing :class and :style
bindings on the wrapper/input as-is; ensure the omit helper or manual
destructuring is used so only class/style are excluded and all other $attrs (id,
aria-*, data-*, tabindex) are forwarded when slots.steps is present.
Summary by CodeRabbit
New Features
Documentation