feat(FoRadialProgress): First implementation#135
Conversation
WalkthroughThis pull request adds a new FoRadialProgress component with accompanying TypeScript types (RadialProgressProps, RadialProgressBackgroundProps), registers it in the public UI exports and app config, and integrates it with the color utility by centralizing color-to-class mappings and updating the useColor signature to accept the app config. It also adds documentation pages and demo components for RadialProgress and renames the Avatar docs section heading from "Variants" to "Presets". 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/core/src/UI/Components/RadialProgress/Types/RadialProgress.ts (2)
9-9: Document the valid range forvalue.
value: numberaccepts any number, but the underlying--valueCSS variable is meaningful only in the 0–100 range. A JSDoc note prevents misuse.✏️ Proposed addition
- /** The progress' value */ - value: number; + /** + * The progress' value (0–100). + */ + value: number;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/UI/Components/RadialProgress/Types/RadialProgress.ts` at line 9, Add a JSDoc comment to the RadialProgress type's value property (the `value: number` declaration) documenting that the valid range is 0–100 and that this number maps to the underlying `--value` CSS variable; place the note directly above the `value` field in the RadialProgress type so developers know to supply/clamp values to that range.
7-12: Consider makingcolorandbackgroundmutually exclusive at the type level.The comment on line 11 documents that
coloris silently ignored whenbackgroundis set, but the type still permits both to be provided simultaneously. A discriminated union would surface this conflict statically instead of relying on a comment.♻️ Proposed type-safe alternative
-export interface RadialProgressProps extends Colorable { - /** The progress' value */ - value: number; - - /** The progress' background style, if this is set, the color prop will be ignored */ - background?: RadialProgressBackgroundProps; -} +type RadialProgressBaseProps = { + /** The progress' value */ + value: number; +}; + +export type RadialProgressProps = + | (RadialProgressBaseProps & Colorable & { background?: never }) + | (RadialProgressBaseProps & { color?: never; background: RadialProgressBackgroundProps });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/UI/Components/RadialProgress/Types/RadialProgress.ts` around lines 7 - 12, RadialProgressProps currently allows both color (from Colorable) and background (RadialProgressBackgroundProps) simultaneously even though color is ignored when background is present; change RadialProgressProps to a discriminated union so the two are mutually exclusive — one branch extends Colorable and allows color but sets background?: never, and the other branch requires background: RadialProgressBackgroundProps and excludes color (or sets color?: never); update any uses or helpers accordingly (or create an XOR<T,U> utility and use XOR<Colorable, { background: RadialProgressBackgroundProps }>) so TypeScript surfaces the conflict at compile time.packages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vue (1)
38-64: Consider narrowingcolorClassesto only supported presets.The
Record<Preset, ...>type requires entries for all 7 presets, but onlysolidandsoftproduce meaningful classes. The other 5 map to empty strings, which silently degrades to an unstyled state. SinceRadialProgressBackgroundPresetis reportedly'solid' | 'soft', you could typecolorClassesasRecord<RadialProgressBackgroundPreset, Record<Color, string>>and avoid the need for theemptyColorClassesplaceholder entirely.#!/bin/bash # Verify RadialProgressBackgroundPreset type rg -n "RadialProgressBackgroundPreset" --type ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vue` around lines 38 - 64, The colorClasses map is typed as Record<Preset, Record<Color, string>> but only 'solid' and 'soft' are meaningful; change its type to Record<RadialProgressBackgroundPreset, Record<Color, string>> and remove the emptyColorClasses placeholders so only solid and soft entries are defined. Update the declaration of colorClasses (symbol: colorClasses) and the referenced types (Preset, Color) to use RadialProgressBackgroundPreset instead, and delete or stop using emptyColorClasses in this file (symbol: emptyColorClasses) so the type matches the actual supported presets.
🤖 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/Components/RadialProgress/UI/FoRadialProgress.vue`:
- Around line 1-8: The progressbar div in FoRadialProgress.vue is missing
required ARIA attributes; update the element with role="progressbar" to include
aria-valuenow bound to the component's value prop and
aria-valuemin/aria-valuemax bound to min and max props (use sensible defaults
like 0 and 100 if min/max props don't exist—add them to the component props as
needed), ensuring the attributes reflect numeric values so screen readers can
convey progress state.
In `@packages/docs/Next/Components/RadialProgress.md`:
- Line 39: Update the section header "## Api" to use the correct capitalization
for the initialism by changing the heading text from "## Api" to "## API" in the
RadialProgress markdown (look for the heading line "## Api").
In
`@packages/docs/Next/Components/RadialProgress/RadialProgressSoftBackground.vue`:
- Around line 1-47: The soft-background showcase is missing the neutral variant;
add another FoRadialProgress instance using :background="{ color: 'neutral',
preset: 'soft' }" with the same :value="70" and slot content "70%" so the
component (FoRadialProgress) demonstrates the neutral color alongside
primary/secondary/accent/etc.; place it alongside the other soft-background
examples to maintain ordering and consistency.
---
Nitpick comments:
In `@packages/core/src/UI/Components/RadialProgress/Types/RadialProgress.ts`:
- Line 9: Add a JSDoc comment to the RadialProgress type's value property (the
`value: number` declaration) documenting that the valid range is 0–100 and that
this number maps to the underlying `--value` CSS variable; place the note
directly above the `value` field in the RadialProgress type so developers know
to supply/clamp values to that range.
- Around line 7-12: RadialProgressProps currently allows both color (from
Colorable) and background (RadialProgressBackgroundProps) simultaneously even
though color is ignored when background is present; change RadialProgressProps
to a discriminated union so the two are mutually exclusive — one branch extends
Colorable and allows color but sets background?: never, and the other branch
requires background: RadialProgressBackgroundProps and excludes color (or sets
color?: never); update any uses or helpers accordingly (or create an XOR<T,U>
utility and use XOR<Colorable, { background: RadialProgressBackgroundProps }>)
so TypeScript surfaces the conflict at compile time.
In `@packages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vue`:
- Around line 38-64: The colorClasses map is typed as Record<Preset,
Record<Color, string>> but only 'solid' and 'soft' are meaningful; change its
type to Record<RadialProgressBackgroundPreset, Record<Color, string>> and remove
the emptyColorClasses placeholders so only solid and soft entries are defined.
Update the declaration of colorClasses (symbol: colorClasses) and the referenced
types (Preset, Color) to use RadialProgressBackgroundPreset instead, and delete
or stop using emptyColorClasses in this file (symbol: emptyColorClasses) so the
type matches the actual supported presets.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/components-radial-progress-background-color-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/components-radial-progress-color-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/components-radial-progress-custom-size-and-thickness-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/components-radial-progress-default-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/components-radial-progress-soft-background-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/components-radial-progress-with-text-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/components-radial-progress-with-value-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (21)
packages/core/src/Lib/UseColor/Internal/Lib/UseColor.tspackages/core/src/Lib/UseColor/Types/Color.tspackages/core/src/Lib/UseFlyonUIVueAppConfig/Types/FlyonUIVueAppConfig.tspackages/core/src/UI/Components/RadialProgress/Types/RadialProgress.tspackages/core/src/UI/Components/RadialProgress/Types/index.tspackages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vuepackages/core/src/UI/Components/RadialProgress/UI/index.tspackages/core/src/UI/Components/RadialProgress/index.tspackages/core/src/UI/Components/index.tspackages/docs/.vitepress/theme/Components/Layout/Features/Sidebar/Lib/UseSidebarItems.tspackages/docs/.vitepress/theme/index.tspackages/docs/Next/Components/Avatar.mdpackages/docs/Next/Components/RadialProgress.mdpackages/docs/Next/Components/RadialProgress/CustomRadialProgressSizeAndThickness.vuepackages/docs/Next/Components/RadialProgress/DefaultRadialProgress.vuepackages/docs/Next/Components/RadialProgress/RadialProgressBackgroundColor.vuepackages/docs/Next/Components/RadialProgress/RadialProgressColor.vuepackages/docs/Next/Components/RadialProgress/RadialProgressDocs.vuepackages/docs/Next/Components/RadialProgress/RadialProgressSoftBackground.vuepackages/docs/Next/Components/RadialProgress/RadialProgressWithText.vuepackages/docs/Next/Components/RadialProgress/RadialProgressWithValue.vue
packages/docs/Next/Components/RadialProgress/RadialProgressSoftBackground.vue
Show resolved
Hide resolved
Deploying flyonui-vue-v3 with
|
| Latest commit: |
663a50c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c00ed973.flyonui-vue-v3.pages.dev |
| Branch Preview URL: | https://feat-radial-progress.flyonui-vue-v3.pages.dev |
Deploying flyonui-vue with
|
| Latest commit: |
663a50c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f69b1a4e.flyonui-vue.pages.dev |
| Branch Preview URL: | https://feat-radial-progress.flyonui-vue.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vue (1)
5-7:⚠️ Potential issue | 🟠 Major
aria-valueminandaria-valuemaxare still missing from the progressbar element.
aria-valuenowwas added, but the WCAG-requiredaria-valueminandaria-valuemaxremain absent. Screen readers cannot convey relative progress without all three.♿ Proposed fix
role="progressbar" :aria-valuenow="value" + aria-valuemin="0" + aria-valuemax="100"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vue` around lines 5 - 7, The progressbar element in FoRadialProgress.vue sets :aria-valuenow="value" but is missing aria-valuemin and aria-valuemax; add bound attributes (e.g., :aria-valuemin="min" and :aria-valuemax="max") on the same element and ensure the component exposes props or computed values named min and max (with sensible defaults, e.g., 0 and 100) so the screen reader can infer the range relative to :aria-valuenow="value".
🧹 Nitpick comments (1)
packages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vue (1)
28-65: Static lookup tables are reconstructed on everycomputedevaluation.
emptyColorClassesandcolorClassesare pure constants with no dependency on reactive state. Defining them inside the computed getter causes fresh object allocation on every recompute. Hoist them to module-level constants.♻️ Proposed refactor
+const emptyColorClasses: Record<Color, ''> = { + neutral: '', primary: '', secondary: '', accent: '', + info: '', success: '', warning: '', error: '', +}; + +const colorClasses: Record<Preset, Record<Color, string>> = { + solid: { + neutral: 'bg-neutral text-neutral-content border-4 border-transparent', + primary: 'bg-primary text-primary-content border-4 border-transparent', + secondary: 'bg-secondary text-secondary-content border-4 border-transparent', + accent: 'bg-accent text-accent-content border-4 border-transparent', + info: 'bg-info text-info-content border-4 border-transparent', + success: 'bg-success text-success-content border-4 border-transparent', + warning: 'bg-warning text-warning-content border-4 border-transparent', + error: 'bg-error text-error-content border-4 border-transparent', + }, + outline: emptyColorClasses, + dash: emptyColorClasses, + soft: { + neutral: 'bg-neutral/10 text-neutral border-4 border-transparent', + primary: 'bg-primary/10 text-primary border-4 border-transparent', + secondary: 'bg-secondary/10 text-secondary border-4 border-transparent', + accent: 'bg-accent/10 text-accent border-4 border-transparent', + info: 'bg-info/10 text-info border-4 border-transparent', + success: 'bg-success/10 text-success border-4 border-transparent', + warning: 'bg-warning/10 text-warning border-4 border-transparent', + error: 'bg-error/10 text-error border-4 border-transparent', + }, + gradient: emptyColorClasses, + text: emptyColorClasses, + dot: emptyColorClasses, +}; const backgroundClass = computed((): string => { - const emptyColorClasses: Record<Color, ''> = { ... }; - const colorClasses: Record<Preset, Record<Color, string>> = { ... }; // ... });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vue` around lines 28 - 65, The two lookup objects emptyColorClasses and colorClasses are being recreated inside a computed getter; hoist them to module scope as true constants to avoid reallocation on each computed evaluation. Move the const declarations for emptyColorClasses: Record<Color, ''> and colorClasses: Record<Preset, Record<Color, string>> out of the component/computed block to the top-level of the module (keeping their exact names and type annotations), then update any references in the computed/getter to use these module-level constants.
🤖 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/Components/RadialProgress/UI/FoRadialProgress.vue`:
- Line 70: The direct double-index access
colorClasses[defaultPreset][defaultColor] can throw if
colorClasses[defaultPreset] is undefined; change the return to perform a guarded
lookup and provide a safe fallback: first check colorClasses and
colorClasses[defaultPreset] exist (or use optional chaining) before indexing
with defaultColor, and return a sensible default class (e.g., an empty string or
a known fallback key) when either defaultPreset or defaultColor is
missing/invalid; update the logic around the colorClasses, defaultPreset and
defaultColor variables (and any use of useFlyonUIVueAppConfigProperty) to use
this defensive lookup so runtime TypeError is avoided.
- Around line 67-68: Move the two useFlyonUIVueAppConfigProperty calls out of
the computed getter and up to the top level of setup: create defaultPreset =
useFlyonUIVueAppConfigProperty(config, componentName, 'preset', () =>
props.background?.preset) and defaultColor =
useFlyonUIVueAppConfigProperty(config, componentName, 'color', () =>
props.background?.color) inside setup, then reference defaultPreset.value and
defaultColor.value inside your computed getter instead of calling the composable
there; this avoids creating nested computed refs and preserves reactivity by
passing callbacks for props.
---
Duplicate comments:
In `@packages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vue`:
- Around line 5-7: The progressbar element in FoRadialProgress.vue sets
:aria-valuenow="value" but is missing aria-valuemin and aria-valuemax; add bound
attributes (e.g., :aria-valuemin="min" and :aria-valuemax="max") on the same
element and ensure the component exposes props or computed values named min and
max (with sensible defaults, e.g., 0 and 100) so the screen reader can infer the
range relative to :aria-valuenow="value".
---
Nitpick comments:
In `@packages/core/src/UI/Components/RadialProgress/UI/FoRadialProgress.vue`:
- Around line 28-65: The two lookup objects emptyColorClasses and colorClasses
are being recreated inside a computed getter; hoist them to module scope as true
constants to avoid reallocation on each computed evaluation. Move the const
declarations for emptyColorClasses: Record<Color, ''> and colorClasses:
Record<Preset, Record<Color, string>> out of the component/computed block to the
top-level of the module (keeping their exact names and type annotations), then
update any references in the computed/getter to use these module-level
constants.
Summary by CodeRabbit
New Features
Documentation