feat(components): add Progress component#282
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new LumexProgress component (component + code-behind + slot types), styling variants and animation, documentation pages/examples and interactive previews, navigation entries, and a comprehensive test suite covering rendering, ARIA, clamping/percentage logic, and indeterminate behavior. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (11)
docs/LumexUI.Docs.Client/Pages/Components/ProgressBar/Examples/CustomStyles.razor (1)
2-10: BothLumexProgressBarelements have no child content — prefer self-closing tags.Helps keep markup concise and signals intent.
♻️ Proposed cleanup
- <LumexProgressBar Value="60" - Color="@ThemeColor.Primary" - Class="border-2 border-primary"> - </LumexProgressBar> + <LumexProgressBar Value="60" + Color="@ThemeColor.Primary" + Class="border-2 border-primary" /> - <LumexProgressBar Value="80" - Color="@ThemeColor.Success" - Classes="@(new ProgressBarSlots { Track = "shadow-medium" })"> - </LumexProgressBar> + <LumexProgressBar Value="80" + Color="@ThemeColor.Success" + Classes="@(new ProgressBarSlots { Track = "shadow-medium" })" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LumexUI.Docs.Client/Pages/Components/ProgressBar/Examples/CustomStyles.razor` around lines 2 - 10, Replace the two LumexProgressBar elements with self-closing tags since they contain no child content; update the markup for the components that use the LumexProgressBar (the instances with Value, Color, Class and the one using Classes="@(new ProgressBarSlots { Track = "shadow-medium" })") so they are written as self-closing elements to keep the Razor markup concise and signal no children are intended.src/LumexUI/Components/ProgressBar/ProgressBarContext.cs (2)
39-39: Non-standard whitespace inside constructor parameter list parentheses.♻️ Proposed fix
- public ProgressBarContext( double percentage, double normalizedValue, double value, double maxValue ) + public ProgressBarContext(double percentage, double normalizedValue, double value, double maxValue)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/LumexUI/Components/ProgressBar/ProgressBarContext.cs` at line 39, The constructor signature for ProgressBarContext contains non-standard whitespace characters inside the parameter-list parentheses; update the ProgressBarContext constructor declaration to use normal ASCII spaces and consistent spacing so it reads exactly like: ProgressBarContext(double percentage, double normalizedValue, double value, double maxValue) (i.e., remove any hidden/non-breaking spaces after '(' and around commas/parentheses).
15-30: Use{ get; init; }to enablewith-expression support on record properties.Properties declared with
{ get; }cannot be targeted bywithexpressions (e.g.,context with { Percentage = 75 }will not compile). Using{ get; init; }aligns with standard record semantics, makes properties settable during construction and viawithexpressions, and keeps the type effectively immutable.♻️ Proposed refactor
- public double Percentage { get; } + public double Percentage { get; init; } - public double NormalizedValue { get; } + public double NormalizedValue { get; init; } - public double Value { get; } + public double Value { get; init; } - public double MaxValue { get; } + public double MaxValue { get; init; }The constructor body remains unchanged;
initaccessors are settable during construction and fromwithexpressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/LumexUI/Components/ProgressBar/ProgressBarContext.cs` around lines 15 - 30, Change the four read-only properties on ProgressBarContext (Percentage, NormalizedValue, Value, MaxValue) from "{ get; }" to "{ get; init; }" so they can be set during construction and via "with" expressions; keep the existing constructor logic unchanged so initialization remains the same but the properties support record-style immutability updates.docs/LumexUI.Docs.Client/Pages/Components/ProgressBar/Examples/Position.razor (1)
2-19:Size="@Size.Medium"is redundant — consider removing it
Size.Mediumis the default value perLumexProgressBar.razor.cs. Including it on all three bars in a positioning example implies size affects label positioning, which it doesn't. Removing it would keep the snippet focused on theLabelPositionparameter being demonstrated.🧹 Suggested cleanup
<div class="w-full flex flex-col gap-4"> <LumexProgressBar Value="50" ShowLabel="true" IsLabelInline="true" LabelPosition="@Align.Start" - Size="@Size.Medium" Color="@ThemeColor.Primary" /> <LumexProgressBar Value="50" ShowLabel="true" IsLabelInline="true" LabelPosition="@Align.Center" - Size="@Size.Medium" Color="@ThemeColor.Primary" /> <LumexProgressBar Value="50" ShowLabel="true" IsLabelInline="true" LabelPosition="@Align.End" - Size="@Size.Medium" Color="@ThemeColor.Primary" /> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LumexUI.Docs.Client/Pages/Components/ProgressBar/Examples/Position.razor` around lines 2 - 19, The examples redundantly set Size="@Size.Medium" which is the default in LumexProgressBar (see LumexProgressBar.razor.cs); remove the Size="@Size.Medium" attribute from each LumexProgressBar in this Position.razor example so the snippet focuses on LabelPosition/Align (LabelPosition="@Align.Start", "@Align.Center", "@Align.End") and avoids implying size affects label positioning.docs/LumexUI.Docs.Client/Pages/Components/ProgressBar/Examples/InlineLabel.razor (1)
2-19:LabelPosition="@Align.Center"is redundant — consider removing it
Align.Centeris the default perLumexProgressBar.razor.cs. Since this example's purpose is to show inline labels at varying sizes (not positions), including the explicit default on all three instances may imply it's required for inline labelling when it isn't. Removing it keeps the snippet minimal and on-point.🧹 Suggested cleanup
<div class="w-full flex flex-col gap-4"> <LumexProgressBar Value="40" ShowLabel="true" IsLabelInline="true" - LabelPosition="@Align.Center" Size="@Size.Small" Color="@ThemeColor.Primary" /> <LumexProgressBar Value="60" ShowLabel="true" IsLabelInline="true" - LabelPosition="@Align.Center" Size="@Size.Medium" Color="@ThemeColor.Primary" /> <LumexProgressBar Value="80" ShowLabel="true" IsLabelInline="true" - LabelPosition="@Align.Center" Size="@Size.Large" Color="@ThemeColor.Primary" /> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LumexUI.Docs.Client/Pages/Components/ProgressBar/Examples/InlineLabel.razor` around lines 2 - 19, The LabelPosition="@Align.Center" attribute is redundant because Align.Center is the default in LumexProgressBar (see LumexProgressBar.razor.cs); remove the LabelPosition="@Align.Center" from each LumexProgressBar instance so the examples focus on IsLabelInline and different Size values without implying the position must be set explicitly.tests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs (3)
238-256:NormalizedValueassertion is inconclusive — test cannot distinguish it fromValue.With
Value=60andMaxValue=200,60is fully within[0, MaxValue], soNormalizedValue == Value == 60regardless of clamping. The assertioncontext.NormalizedValue.Should().Be(60.0)provides no evidence thatNormalizedValuebehaves differently from the rawValue. Using an out-of-range value (e.g.,Value = -10) would actually exercise clamping and prove the two properties diverge:♻️ Suggested test adjustment
var cut = RenderComponent<LumexProgressBar>( p => p .Add( p => p.ShowLabel, true ) - .Add( p => p.Value, 60 ) + .Add( p => p.Value, -10 ) // out-of-range → NormalizedValue should be 0 .Add( p => p.MaxValue, 200 ) .Add( p => p.LabelContent, context => { - context.Percentage.Should().Be( 30.0 ); - context.NormalizedValue.Should().Be( 60.0 ); - context.Value.Should().Be( 60.0 ); + context.Percentage.Should().Be( 0.0 ); // clamped + context.NormalizedValue.Should().Be( 0.0 ); // clamped to 0, not -10 + context.Value.Should().Be( -10.0 ); // raw input preserved context.MaxValue.Should().Be( 200.0 ); return "Test"; } ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs` around lines 238 - 256, Update the ProgressBar_LabelContent_ShouldHaveAccessToContext test so NormalizedValue is actually exercised: set LumexProgressBar.Value to an out-of-range value (for example -10) while keeping MaxValue at 200, then assert that context.Value equals the raw -10.0 and context.NormalizedValue equals the clamped value (e.g., 0.0) along with the existing Percentage/MaxValue assertions; modify the render block passed to RenderComponent<LumexProgressBar> and the assertions inside the LabelContent delegate accordingly.
207-222:ProgressBar_LabelPosition_ShouldApplyCorrectPositiononly asserts the label renders, not that the correct alignment class is applied.The test verifies no exception occurs and the label slot is present, but says nothing about whether
Align.Start,Align.Center, orAlign.Endproduces the expected CSS class on the label element. Consider adding a class assertion (e.g.,text-start,text-center,text-end, or whichever utility the style system emits).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs` around lines 207 - 222, Update the test ProgressBar_LabelPosition_ShouldApplyCorrectPosition to assert that the rendered label element receives the expected CSS alignment class based on the LabelPosition value: when LabelPosition is Align.Start assert the label contains the "text-start" (or the project-specific start class), for Align.Center assert "text-center", and for Align.End assert "text-end"; locate the label via cut.FindBySlot("label") and add an assertion against label.ClassList or label.GetAttribute("class") to verify the correct class is present for LumexProgressBar's LabelPosition handling.
172-205:OuterHtmlstring comparison for DOM-containment checks is fragile.
track!.Children.Any(c => c.OuterHtml == label.OuterHtml)relies on exact string equality of the rendered HTML. Any change in attribute ordering, whitespace, or added children by bUnit could produce a false negative without a real bug. Prefer an explicit parent-child DOM relationship check:♻️ Proposed refactor
- track!.Children.Any( c => c.OuterHtml == label.OuterHtml ).Should().BeTrue(); + label!.ParentElement!.GetAttribute( "data-slot" ).Should().Be( "track" );- track!.Children.Any( c => c.OuterHtml == label.OuterHtml ).Should().BeFalse(); + label!.ParentElement!.GetAttribute( "data-slot" ).Should().NotBe( "track" );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs` around lines 172 - 205, The tests ProgressBar_IsLabelInline_ShouldRenderLabelInsideTrack and ProgressBar_IsLabelInlineFalse_ShouldRenderLabelOutsideTrack use fragile OuterHtml string equality; change them to assert DOM parent-child relationships using the found elements (track and label) instead: for the inline case assert that label's parent/ancestor is the track element (e.g., label.Parent equals or is contained within track), and for the non-inline case assert the label is not a child/descendant of track (e.g., label.Parent is not track or track does not contain label), keeping the existing variables track and label to locate the elements.src/LumexUI/Components/ProgressBar/LumexProgressBar.razor.cs (1)
116-128:OnParametersSetrebuilds slot styles on every render, including high-frequencyValuechanges.Style output depends only on
Size,Radius,Color,IsLoadingBar,IsLabelInline, andLabelPosition—none of which change during a typical progress animation. Rebuilding_slotson everyValueupdate is wasteful. Consider tracking the style-relevant parameters and short-circuiting when they are unchanged.♻️ Proposed approach
+private Size _prevSize; +private Radius _prevRadius; +private ThemeColor _prevColor; +private bool _prevIsLoadingBar; +private bool _prevIsLabelInline; +private Align _prevLabelPosition; protected override void OnParametersSet() { + if( Size == _prevSize && Radius == _prevRadius && Color == _prevColor && + IsLoadingBar == _prevIsLoadingBar && IsLabelInline == _prevIsLabelInline && + LabelPosition == _prevLabelPosition ) + { + return; + } + ( _prevSize, _prevRadius, _prevColor, _prevIsLoadingBar, _prevIsLabelInline, _prevLabelPosition ) = + ( Size, Radius, Color, IsLoadingBar, IsLabelInline, LabelPosition ); + var progressBar = Styles.ProgressBar.Style( TwMerge ); _slots = progressBar( new() { ... } ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/LumexUI/Components/ProgressBar/LumexProgressBar.razor.cs` around lines 116 - 128, OnParametersSet currently recomputes _slots via Styles.ProgressBar.Style(TwMerge) on every render (including frequent Value updates); change it to memoize the style-relevant inputs (Size, Radius, Color, IsLoadingBar, IsLabelInline, LabelPosition) and only call Styles.ProgressBar.Style(...) to rebuild _slots when any of those tracked properties actually change. Implement simple cached fields (e.g., _lastSize, _lastRadius, etc. or a single struct/hash) inside the LumexProgressBar class, compare them at the start of OnParametersSet, and return early if unchanged; otherwise update the cached values and recompute _slots as you do now.src/LumexUI/Styles/ProgressBar.cs (1)
104-181: Seven near-identical colorCompoundVariantblocks can be collapsed into a loop.Each block differs only by
ThemeColorvalue. SinceColorVariants.Solidis aDictionary<ThemeColor, string>, all entries can be generated by iterating it.♻️ Proposed refactor
Replace lines 104–181 with a
foreachinside theCompoundVariantscollection initializer, or build the list before theVariantConfig:- // Color variants for fill - new CompoundVariant() - { - Conditions = new() - { - [nameof( LumexProgressBar.Color )] = nameof( ThemeColor.Default ) - }, - Classes = new SlotCollection() - { - [nameof( ProgressBarSlots.Fill )] = ColorVariants.Solid[ThemeColor.Default] - } - }, - // ... (repeated for each ThemeColor) + // Color variants for fill — generated from ColorVariants.Solid + ..ColorVariants.Solid.Select( kvp => new CompoundVariant() + { + Conditions = new() + { + [nameof( LumexProgressBar.Color )] = kvp.Key.ToString() + }, + Classes = new SlotCollection() + { + [nameof( ProgressBarSlots.Fill )] = kvp.Value + } + } ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/LumexUI/Styles/ProgressBar.cs` around lines 104 - 181, The repeated CompoundVariant blocks mapping LumexProgressBar.Color values to ColorVariants.Solid can be collapsed by iterating the ColorVariants.Solid dictionary (or its keys) and creating a CompoundVariant per entry; replace the seven hard-coded CompoundVariant initializers with a loop that for each ThemeColor key builds a CompoundVariant with Conditions = { [nameof(LumexProgressBar.Color)] = nameof(ThemeColor.<key>) } and Classes = new SlotCollection() { [nameof(ProgressBarSlots.Fill)] = ColorVariants.Solid[key] } (either via a foreach when constructing the CompoundVariants collection or by pre-building a List<CompoundVariant> and adding it to the VariantConfig).src/LumexUI/Components/ProgressBar/LumexProgressBar.razor (1)
19-21: Hardcodedanimation-durationinline style bypasses theming.The
1.5sduration is set as an inline style rather than being expressed as a CSS custom property or embedded in theanimate-shimmerclass. If the theme ever needs to adjust this timing (e.g., via--animate-shimmer), this inline value will silently win.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/LumexUI/Components/ProgressBar/LumexProgressBar.razor` around lines 19 - 21, The inline style setting animation-duration: 1.5s on the div in LumexProgressBar.razor (the element with class "absolute inset-0 bg-white/30 animate-shimmer") bypasses theming; remove that inline style and instead drive the duration from a CSS custom property (e.g. --animate-shimmer) or define it inside the .animate-shimmer rule so theme overrides work—update the component to omit the style attribute and modify the .animate-shimmer CSS to use var(--animate-shimmer, 1.5s) (or place the duration directly in the class) so theme values can take precedence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/LumexUI/Components/ProgressBar/LumexProgressBar.razor`:
- Around line 40-52: The external label branch inside the conditional using
ShouldShowLabel && !IsLabelInline incorrectly outputs NormalizedValue:F0%
instead of the computed Percentage, so update the fallback in the div (where
LabelContent and Context are used and GetStyles(nameof(S.Label)) is applied) to
render Percentage:F0% instead of NormalizedValue:F0%; keep the custom
LabelContent path unchanged so consumers still receive Context when provided.
In `@src/LumexUI/Components/ProgressBar/LumexProgressBar.razor.cs`:
- Around line 107-108: The NormalizedValue property uses Math.Clamp(Value, 0,
MaxValue) which throws if MaxValue is negative; update NormalizedValue to bound
the clamp's upper limit to a non-negative value (e.g., use Math.Max(0,
MaxValue)) so the clamp call never receives max < min. Keep the Percentage guard
(MaxValue > 0) as-is; change only the NormalizedValue computation referenced by
the NormalizedValue property to use Math.Max(0, MaxValue).
- Line 110: ShouldShowLabel currently suppresses the default label at exactly 0%
because of the "NormalizedValue > 0" check; change the property on
LumexProgressBar.razor.cs (ShouldShowLabel) to only gate on explicit consumer
intent and loading state by removing the NormalizedValue check so it becomes
ShowLabel && !IsLoadingBar (i.e., always show the label when ShowLabel is true
unless IsLoadingBar is true, and still allow LabelContent to override).
In `@src/LumexUI/Components/ProgressBar/ProgressBarContext.cs`:
- Around line 18-20: The property NormalizedValue and its XML doc
disagree—change the API to match intent by renaming NormalizedValue to
ClampedValue and update its doc to "Gets the value clamped between 0 and <see
cref="MaxValue"/>", then update all references/usages (including any consumers
in LabelContent fragments, bindings, and tests) to use ClampedValue; ensure any
logic that currently computes clamp(Value, 0, MaxValue) remains unchanged and
only the property name and XML comment are updated to avoid breaking semantics.
In `@tests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs`:
- Around line 116-124: The test
ProgressBar_IsLoadingBar_ShouldHaveShimmerAnimation currently only checks for
the shimmer overlay; update it to also locate the Fill element of
LumexProgressBar (the Fill slot rendered when IsLoadingBar=true) and assert that
the fill element has the CSS class "animate-progress-loading" (e.g., find the
fill element and call Should().Contain("animate-progress-loading") or use an
element.HasClass assertion) to ensure the fill itself receives the loading
animation applied in ProgressBar.cs.
---
Nitpick comments:
In
`@docs/LumexUI.Docs.Client/Pages/Components/ProgressBar/Examples/CustomStyles.razor`:
- Around line 2-10: Replace the two LumexProgressBar elements with self-closing
tags since they contain no child content; update the markup for the components
that use the LumexProgressBar (the instances with Value, Color, Class and the
one using Classes="@(new ProgressBarSlots { Track = "shadow-medium" })") so they
are written as self-closing elements to keep the Razor markup concise and signal
no children are intended.
In
`@docs/LumexUI.Docs.Client/Pages/Components/ProgressBar/Examples/InlineLabel.razor`:
- Around line 2-19: The LabelPosition="@Align.Center" attribute is redundant
because Align.Center is the default in LumexProgressBar (see
LumexProgressBar.razor.cs); remove the LabelPosition="@Align.Center" from each
LumexProgressBar instance so the examples focus on IsLabelInline and different
Size values without implying the position must be set explicitly.
In
`@docs/LumexUI.Docs.Client/Pages/Components/ProgressBar/Examples/Position.razor`:
- Around line 2-19: The examples redundantly set Size="@Size.Medium" which is
the default in LumexProgressBar (see LumexProgressBar.razor.cs); remove the
Size="@Size.Medium" attribute from each LumexProgressBar in this Position.razor
example so the snippet focuses on LabelPosition/Align
(LabelPosition="@Align.Start", "@Align.Center", "@Align.End") and avoids
implying size affects label positioning.
In `@src/LumexUI/Components/ProgressBar/LumexProgressBar.razor`:
- Around line 19-21: The inline style setting animation-duration: 1.5s on the
div in LumexProgressBar.razor (the element with class "absolute inset-0
bg-white/30 animate-shimmer") bypasses theming; remove that inline style and
instead drive the duration from a CSS custom property (e.g. --animate-shimmer)
or define it inside the .animate-shimmer rule so theme overrides work—update the
component to omit the style attribute and modify the .animate-shimmer CSS to use
var(--animate-shimmer, 1.5s) (or place the duration directly in the class) so
theme values can take precedence.
In `@src/LumexUI/Components/ProgressBar/LumexProgressBar.razor.cs`:
- Around line 116-128: OnParametersSet currently recomputes _slots via
Styles.ProgressBar.Style(TwMerge) on every render (including frequent Value
updates); change it to memoize the style-relevant inputs (Size, Radius, Color,
IsLoadingBar, IsLabelInline, LabelPosition) and only call
Styles.ProgressBar.Style(...) to rebuild _slots when any of those tracked
properties actually change. Implement simple cached fields (e.g., _lastSize,
_lastRadius, etc. or a single struct/hash) inside the LumexProgressBar class,
compare them at the start of OnParametersSet, and return early if unchanged;
otherwise update the cached values and recompute _slots as you do now.
In `@src/LumexUI/Components/ProgressBar/ProgressBarContext.cs`:
- Line 39: The constructor signature for ProgressBarContext contains
non-standard whitespace characters inside the parameter-list parentheses; update
the ProgressBarContext constructor declaration to use normal ASCII spaces and
consistent spacing so it reads exactly like: ProgressBarContext(double
percentage, double normalizedValue, double value, double maxValue) (i.e., remove
any hidden/non-breaking spaces after '(' and around commas/parentheses).
- Around line 15-30: Change the four read-only properties on ProgressBarContext
(Percentage, NormalizedValue, Value, MaxValue) from "{ get; }" to "{ get; init;
}" so they can be set during construction and via "with" expressions; keep the
existing constructor logic unchanged so initialization remains the same but the
properties support record-style immutability updates.
In `@src/LumexUI/Styles/ProgressBar.cs`:
- Around line 104-181: The repeated CompoundVariant blocks mapping
LumexProgressBar.Color values to ColorVariants.Solid can be collapsed by
iterating the ColorVariants.Solid dictionary (or its keys) and creating a
CompoundVariant per entry; replace the seven hard-coded CompoundVariant
initializers with a loop that for each ThemeColor key builds a CompoundVariant
with Conditions = { [nameof(LumexProgressBar.Color)] = nameof(ThemeColor.<key>)
} and Classes = new SlotCollection() { [nameof(ProgressBarSlots.Fill)] =
ColorVariants.Solid[key] } (either via a foreach when constructing the
CompoundVariants collection or by pre-building a List<CompoundVariant> and
adding it to the VariantConfig).
In `@tests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs`:
- Around line 238-256: Update the
ProgressBar_LabelContent_ShouldHaveAccessToContext test so NormalizedValue is
actually exercised: set LumexProgressBar.Value to an out-of-range value (for
example -10) while keeping MaxValue at 200, then assert that context.Value
equals the raw -10.0 and context.NormalizedValue equals the clamped value (e.g.,
0.0) along with the existing Percentage/MaxValue assertions; modify the render
block passed to RenderComponent<LumexProgressBar> and the assertions inside the
LabelContent delegate accordingly.
- Around line 207-222: Update the test
ProgressBar_LabelPosition_ShouldApplyCorrectPosition to assert that the rendered
label element receives the expected CSS alignment class based on the
LabelPosition value: when LabelPosition is Align.Start assert the label contains
the "text-start" (or the project-specific start class), for Align.Center assert
"text-center", and for Align.End assert "text-end"; locate the label via
cut.FindBySlot("label") and add an assertion against label.ClassList or
label.GetAttribute("class") to verify the correct class is present for
LumexProgressBar's LabelPosition handling.
- Around line 172-205: The tests
ProgressBar_IsLabelInline_ShouldRenderLabelInsideTrack and
ProgressBar_IsLabelInlineFalse_ShouldRenderLabelOutsideTrack use fragile
OuterHtml string equality; change them to assert DOM parent-child relationships
using the found elements (track and label) instead: for the inline case assert
that label's parent/ancestor is the track element (e.g., label.Parent equals or
is contained within track), and for the non-inline case assert the label is not
a child/descendant of track (e.g., label.Parent is not track or track does not
contain label), keeping the existing variables track and label to locate the
elements.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/LumexUI/Components/ProgressBar/ProgressBarContext.cs (1)
18-18:⚠️ Potential issue | 🟡 MinorResidual doc inconsistency:
ClampedValuesummary still says "normalized value".The property was correctly renamed from
NormalizedValuetoClampedValue, but the XML summary on line 18 was not updated and still reads "Gets the normalized value". It should read "Gets the clamped value".📝 Proposed fix
-/// Gets the normalized value (clamped between 0 and <see cref="MaxValue"/>). +/// Gets the clamped value (between 0 and <see cref="MaxValue"/>).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/LumexUI/Components/ProgressBar/ProgressBarContext.cs` at line 18, Update the XML doc comment for the ClampedValue property so it no longer says "normalized value": change the summary to "Gets the clamped value (clamped between 0 and <see cref=\"MaxValue\"/>)." Locate the summary above the ClampedValue property in ProgressBarContext (the XML <summary> for ClampedValue) and replace the word "normalized" with "clamped" while preserving the rest of the tag and cref reference.
🧹 Nitpick comments (1)
tests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs (1)
249-268: Assertions inside theLabelContentrender callback can silently pass.If
ShouldShowLabelever evaluates tofalse(e.g., due to a future regression), the callback is never invoked andcontext.Percentage.Should().Be(30.0)never runs — the test reports green because only the outerlabel.Should().NotBeNull()fires (and finds null, which would fail). But the specific context-value assertions are simply skipped without any diagnostic. Capturing the context outside is safer and makes intent explicit.♻️ Proposed refactor
+ProgressBarContext? capturedCtx = null; var cut = RenderComponent<LumexProgressBar>( p => p .Add( p => p.ShowLabel, true ) .Add( p => p.Value, 60 ) .Add( p => p.MaxValue, 200 ) - .Add( p => p.LabelContent, context => - { - context.Percentage.Should().Be( 30.0 ); - context.ClampedValue.Should().Be( 60.0 ); - context.Value.Should().Be( 60.0 ); - context.MaxValue.Should().Be( 200.0 ); - return "Test"; - } ) + .Add( p => p.LabelContent, context => { capturedCtx = context; return "Test"; } ) ); -var label = cut.FindBySlot( "label" ); -label.Should().NotBeNull(); +cut.FindBySlot( "label" ).Should().NotBeNull(); +capturedCtx.Should().NotBeNull(); +var ctx = capturedCtx!.Value; +ctx.Percentage.Should().Be( 30.0 ); +ctx.ClampedValue.Should().Be( 60.0 ); +ctx.Value.Should().Be( 60.0 ); +ctx.MaxValue.Should().Be( 200.0 );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs` around lines 249 - 268, The test ProgressBar_LabelContent_ShouldHaveAccessToContext currently performs assertions inside the LabelContent render callback which can be silently skipped if the label isn't rendered; change it to capture the LabelContent context into a local variable from the RenderComponent<LumexProgressBar> invocation (e.g., assign the context provided to LabelContent to a local variable), then after rendering assert on context.Percentage, context.ClampedValue, context.Value and context.MaxValue, and finally assert the label exists; ensure you still set ShowLabel, Value and MaxValue when calling RenderComponent so the callback is invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/LumexUI/Components/ProgressBar/ProgressBarContext.cs`:
- Line 18: Update the XML doc comment for the ClampedValue property so it no
longer says "normalized value": change the summary to "Gets the clamped value
(clamped between 0 and <see cref=\"MaxValue\"/>)." Locate the summary above the
ClampedValue property in ProgressBarContext (the XML <summary> for ClampedValue)
and replace the word "normalized" with "clamped" while preserving the rest of
the tag and cref reference.
---
Nitpick comments:
In `@tests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs`:
- Around line 249-268: The test
ProgressBar_LabelContent_ShouldHaveAccessToContext currently performs assertions
inside the LabelContent render callback which can be silently skipped if the
label isn't rendered; change it to capture the LabelContent context into a local
variable from the RenderComponent<LumexProgressBar> invocation (e.g., assign the
context provided to LabelContent to a local variable), then after rendering
assert on context.Percentage, context.ClampedValue, context.Value and
context.MaxValue, and finally assert the label exists; ensure you still set
ShowLabel, Value and MaxValue when calling RenderComponent so the callback is
invoked.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/LumexUI/Components/ProgressBar/LumexProgressBar.razorsrc/LumexUI/Components/ProgressBar/LumexProgressBar.razor.cssrc/LumexUI/Components/ProgressBar/ProgressBarContext.cstests/LumexUI.Tests/Components/ProgressBar/ProgressBarTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/LumexUI/Components/ProgressBar/LumexProgressBar.razor
desmondinho
reviewed
Feb 24, 2026
desmondinho
left a comment
desmondinho
left a comment
Contributor
There was a problem hiding this comment.
Hey @kevinvenclovas,
Thank you very much for this PR! Overall, it looks good -- though there are a couple of points that need attention.
Please make sure that you follow the HeroUI styles and structure:
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Label.razor (1)
2-2: Consider addingAriaLabelfor accessibility completeness.Other examples in this documentation set include
AriaLabelalongside visual properties. If theLabelparameter doesn't automatically provide ARIA labeling, screen reader users won't have context for this progress bar.Suggested improvement
<div class="w-full flex flex-col gap-4"> - <LumexProgress Value="55" Label="Loading..." /> + <LumexProgress Value="55" Label="Loading..." AriaLabel="Loading..." /> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Label.razor` at line 2, The Progress example uses the LumexProgress component with Label="Loading..." but lacks an AriaLabel for screen readers; update the example to pass an AriaLabel (e.g., AriaLabel="Loading progress") to LumexProgress so assistive tech receives an explicit accessible name; ensure you add AriaLabel alongside the existing Label parameter in the LumexProgress invocation and keep the text meaningfully descriptive for screen-reader users.docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Indeterminate.razor (1)
2-2: Consider addingAriaLabelfor indeterminate state.Indeterminate progress indicators especially benefit from accessible labels since they don't convey numeric progress. An
AriaLabelwould help screen reader users understand the purpose.Suggested improvement
- <LumexProgress IsIndeterminate="true" Color="@ThemeColor.Primary" Size="@Size.Small" /> + <LumexProgress IsIndeterminate="true" Color="@ThemeColor.Primary" Size="@Size.Small" AriaLabel="Loading..." />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Indeterminate.razor` at line 2, Add an accessible label to the indeterminate progress so screen readers know its purpose: update the LumexProgress usage (the instance with IsIndeterminate="true", Color="@ThemeColor.Primary", Size="@Size.Small") to include the AriaLabel property with a meaningful string (e.g., "Loading" or "Loading content") or a bound resource/parameter so the indeterminate state is announced to assistive technology.tests/LumexUI.Tests/Components/Progress/ProgressTests.cs (2)
117-127: Add a regression test forIsIndeterminate + DisableAnimation.This interaction is currently untested and would catch animation-leak regressions.
🧪 Suggested test
[Fact] public void Progress_IsIndeterminate_ShouldHaveAnimateProgressLoading() { var cut = RenderComponent<LumexProgress>( p => p .Add( p => p.IsIndeterminate, true ) ); var indicator = cut.FindBySlot( "indicator" ); indicator.ClassList.Should().Contain( "animate-progress-loading" ); } + + [Fact] + public void Progress_IsIndeterminate_WithDisableAnimation_ShouldDisableAnimation() + { + var cut = RenderComponent<LumexProgress>( p => p + .Add( p => p.IsIndeterminate, true ) + .Add( p => p.DisableAnimation, true ) + ); + + var indicator = cut.FindBySlot( "indicator" ); + indicator.ClassList.Should().Contain( "!animate-none" ); + indicator.ClassList.Should().NotContain( "animate-progress-loading" ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/LumexUI.Tests/Components/Progress/ProgressTests.cs` around lines 117 - 127, Add a regression test to verify the interaction between LumexProgress.IsIndeterminate and LumexProgress.DisableAnimation: create a test similar to Progress_IsIndeterminate_ShouldHaveAnimateProgressLoading that renders LumexProgress with both IsIndeterminate = true and DisableAnimation = true, find the indicator slot (indicator) and assert that the "animate-progress-loading" class is NOT present; also add a complementary assertion ensuring when DisableAnimation = false the class is present to cover both behaviors.
253-266: Add coverage for fractional ARIA values.Current ARIA tests only validate integer inputs; the component API accepts
double.🧪 Suggested test
[Fact] public void Progress_ShouldHaveCorrectAriaAttributes() { var cut = RenderComponent<LumexProgress>( p => p .Add( p => p.Value, 50 ) .Add( p => p.MaxValue, 100 ) ); var base_ = cut.FindBySlot( "base" ); base_.GetAttribute( "role" ).Should().Be( "progressbar" ); base_.GetAttribute( "aria-valuenow" ).Should().Be( "50" ); base_.GetAttribute( "aria-valuemin" ).Should().Be( "0" ); base_.GetAttribute( "aria-valuemax" ).Should().Be( "100" ); } + + [Fact] + public void Progress_WithFractionalRange_ShouldExposeFractionalAriaValues() + { + var cut = RenderComponent<LumexProgress>( p => p + .Add( p => p.MinValue, 0.5 ) + .Add( p => p.MaxValue, 1.5 ) + .Add( p => p.Value, 1.25 ) + ); + + var base_ = cut.FindBySlot( "base" ); + base_.GetAttribute( "aria-valuenow" ).Should().Be( "1.25" ); + base_.GetAttribute( "aria-valuemin" ).Should().Be( "0.5" ); + base_.GetAttribute( "aria-valuemax" ).Should().Be( "1.5" ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/LumexUI.Tests/Components/Progress/ProgressTests.cs` around lines 253 - 266, Add a new unit test in ProgressTests.cs (alongside Progress_ShouldHaveCorrectAriaAttributes) that renders LumexProgress with fractional double values (e.g., Value = 12.34 and MaxValue = 100.5) and asserts the base slot's aria attributes: role == "progressbar", aria-valuenow == "12.34" (matching the component's string formatting), aria-valuemin == "0" and aria-valuemax == "100.5"; reference the existing test name Progress_ShouldHaveCorrectAriaAttributes, the LumexProgress component, and the "base" slot to locate where to add the new test.src/LumexUI/Styles/Progress.cs (1)
87-108: Keep indicator radius aligned with track radius.Radius variants currently style only the track; the indicator keeps
rounded-full, which can look off forRadius.None/Small/Medium/Large.♻️ Suggested refactor
[nameof( Radius.None )] = new SlotCollection { [nameof( ProgressSlots.Track )] = "rounded-none", + [nameof( ProgressSlots.Indicator )] = "rounded-none", }, [nameof( Radius.Small )] = new SlotCollection { [nameof( ProgressSlots.Track )] = "rounded-sm", + [nameof( ProgressSlots.Indicator )] = "rounded-sm", }, [nameof( Radius.Medium )] = new SlotCollection { [nameof( ProgressSlots.Track )] = "rounded-md", + [nameof( ProgressSlots.Indicator )] = "rounded-md", }, [nameof( Radius.Large )] = new SlotCollection { [nameof( ProgressSlots.Track )] = "rounded-lg", + [nameof( ProgressSlots.Indicator )] = "rounded-lg", }, [nameof( Radius.Full )] = new SlotCollection { [nameof( ProgressSlots.Track )] = "rounded-full", + [nameof( ProgressSlots.Indicator )] = "rounded-full", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/LumexUI/Styles/Progress.cs` around lines 87 - 108, The radius variants currently only set the track's classes, so the indicator stays rounded-full and mismatches for Radius.None/Small/Medium/Large; update the VariantValueCollection for nameof(LumexProgress.Radius) to include matching SlotCollection entries for both nameof(ProgressSlots.Track) and nameof(ProgressSlots.Indicator) (e.g., "rounded-none", "rounded-sm", "rounded-md", "rounded-lg", "rounded-full") so the indicator radius aligns with the track for each Radius value in the LumexProgress.Radius mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razor`:
- Line 93: The description string in Progress.razor currently reads "Progress
display the progress..." which is grammatically incorrect; update the
description property (the value assigned to description) to use either "Progress
displays the progress of a task or operation, and can also be used as animated
loading indicators." or "Progress bars display the progress of a task or
operation, and can also be used as animated loading indicators." so the subject
and verb agree (edit the string literal used for description).
- Line 11: Update the docs to use the correct parameter name AriaLabel (not
AreaLabel): change both occurrences in Progress.razor documentation text to
reference AriaLabel and mention passing AriaLabel when Label is not provided;
ensure the accessibility note and any examples consistently match the
LumexProgress component parameter AriaLabel defined in LumexProgress.razor.cs.
In `@src/LumexUI/Components/Progress/LumexProgress.razor`:
- Around line 11-15: The ARIA attributes are truncating fractional values and
emitting determinate text in indeterminate mode; update the attributes in
LumexProgress.razor to use the original double values (do not cast ClampedValue,
MinValue, MaxValue to int) and format them as strings preserving fractional
precision (e.g., invariant/culture-aware ToString with an appropriate numeric
format) for aria-valuenow, aria-valuemin and aria-valuemax, and ensure
aria-valuetext is only emitted when IsIndeterminate is false (return null or
empty when IsIndeterminate is true) so indeterminate progressbars do not
announce a determinate percentage; keep references to ClampedValue, MinValue,
MaxValue, IsIndeterminate and ValueText/ComputedAriaLabel when making the
changes.
In `@src/LumexUI/Styles/Progress.cs`:
- Around line 111-125: The DisableAnimation variant currently only negates
transitions but does not remove the indeterminate animation class set by
IsIndeterminate, so add a negation for the animation class: in the
VariantValueCollection for LumexProgress.DisableAnimation update the
SlotCollection entry for ProgressSlots.Indicator to remove the
animate-progress-loading class (e.g. set it to "!animate-progress-loading" or
combine "!animate-progress-loading !transition-none" if you want to keep
disabling transitions too) so the indeterminate animation is actually disabled
when both flags are set.
---
Nitpick comments:
In
`@docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Indeterminate.razor`:
- Line 2: Add an accessible label to the indeterminate progress so screen
readers know its purpose: update the LumexProgress usage (the instance with
IsIndeterminate="true", Color="@ThemeColor.Primary", Size="@Size.Small") to
include the AriaLabel property with a meaningful string (e.g., "Loading" or
"Loading content") or a bound resource/parameter so the indeterminate state is
announced to assistive technology.
In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Label.razor`:
- Line 2: The Progress example uses the LumexProgress component with
Label="Loading..." but lacks an AriaLabel for screen readers; update the example
to pass an AriaLabel (e.g., AriaLabel="Loading progress") to LumexProgress so
assistive tech receives an explicit accessible name; ensure you add AriaLabel
alongside the existing Label parameter in the LumexProgress invocation and keep
the text meaningfully descriptive for screen-reader users.
In `@src/LumexUI/Styles/Progress.cs`:
- Around line 87-108: The radius variants currently only set the track's
classes, so the indicator stays rounded-full and mismatches for
Radius.None/Small/Medium/Large; update the VariantValueCollection for
nameof(LumexProgress.Radius) to include matching SlotCollection entries for both
nameof(ProgressSlots.Track) and nameof(ProgressSlots.Indicator) (e.g.,
"rounded-none", "rounded-sm", "rounded-md", "rounded-lg", "rounded-full") so the
indicator radius aligns with the track for each Radius value in the
LumexProgress.Radius mapping.
In `@tests/LumexUI.Tests/Components/Progress/ProgressTests.cs`:
- Around line 117-127: Add a regression test to verify the interaction between
LumexProgress.IsIndeterminate and LumexProgress.DisableAnimation: create a test
similar to Progress_IsIndeterminate_ShouldHaveAnimateProgressLoading that
renders LumexProgress with both IsIndeterminate = true and DisableAnimation =
true, find the indicator slot (indicator) and assert that the
"animate-progress-loading" class is NOT present; also add a complementary
assertion ensuring when DisableAnimation = false the class is present to cover
both behaviors.
- Around line 253-266: Add a new unit test in ProgressTests.cs (alongside
Progress_ShouldHaveCorrectAriaAttributes) that renders LumexProgress with
fractional double values (e.g., Value = 12.34 and MaxValue = 100.5) and asserts
the base slot's aria attributes: role == "progressbar", aria-valuenow == "12.34"
(matching the component's string formatting), aria-valuemin == "0" and
aria-valuemax == "100.5"; reference the existing test name
Progress_ShouldHaveCorrectAriaAttributes, the LumexProgress component, and the
"base" slot to locate where to add the new test.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.csdocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Colors.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/CustomStyles.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Indeterminate.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Label.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Sizes.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Usage.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Value.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Colors.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/CustomStyles.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Indeterminate.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Label.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Sizes.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Usage.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Value.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razorsrc/LumexUI/Components/Progress/LumexProgress.razorsrc/LumexUI/Components/Progress/LumexProgress.razor.cssrc/LumexUI/Components/Progress/ProgressSlots.cssrc/LumexUI/Styles/Progress.cstests/LumexUI.Tests/Components/Progress/ProgressTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.cs
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razor (1)
11-11:⚠️ Potential issue | 🟡 MinorFix typo:
AreaLabelshould beAriaLabel.The property name
AreaLabelon line 11 is incorrect. It should beAriaLabelto match the actual component parameter and the correct usage on line 41.🐛 Proposed fix
- Note: Make sure to pass the <code>AreaLabel</code> prop when the <code>Label</code> prop is not provided. This is required for accessibility. + Note: Make sure to pass the <code>AriaLabel</code> prop when the <code>Label</code> prop is not provided. This is required for accessibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razor` at line 11, Typo: change the documentation text that references the prop name "AreaLabel" to the correct prop "AriaLabel" so it matches the component parameter and usage of "Label" in the same file; update the sentence that currently reads "pass the AreaLabel prop" to "pass the AriaLabel prop" to ensure consistency with the Progress component's AriaLabel parameter and other references in the file.
🧹 Nitpick comments (1)
docs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Label.razor (1)
3-3: Minor formatting inconsistency.Other PreviewCodes files use
@new(name: null, snippet: "...")without spaces inside the parentheses. Consider aligning for consistency.✨ Suggested fix
-<PreviewCode Code="@new( name: null, snippet: "Progress.Code.Label" )"> +<PreviewCode Code="@new(name: null, snippet: "Progress.Code.Label")">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Label.razor` at line 3, The PreviewCode instantiation currently has inconsistent spacing: replace the spaced form used in PreviewCodes ("@new( name: null, snippet: \"Progress.Code.Label\" )") with the consistent formatting used elsewhere by changing the call to the compact form (e.g., use `@new`(name: null, snippet: "Progress.Code.Label")) so the PreviewCode Code attribute matches other PreviewCodes; update the occurrence in Label.razor where PreviewCode Code="@new( name: null, snippet: "Progress.Code.Label" )" is declared.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razor`:
- Line 11: Typo: change the documentation text that references the prop name
"AreaLabel" to the correct prop "AriaLabel" so it matches the component
parameter and usage of "Label" in the same file; update the sentence that
currently reads "pass the AreaLabel prop" to "pass the AriaLabel prop" to ensure
consistency with the Progress component's AriaLabel parameter and other
references in the file.
---
Nitpick comments:
In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Label.razor`:
- Line 3: The PreviewCode instantiation currently has inconsistent spacing:
replace the spaced form used in PreviewCodes ("@new( name: null, snippet:
\"Progress.Code.Label\" )") with the consistent formatting used elsewhere by
changing the call to the compact form (e.g., use `@new`(name: null, snippet:
"Progress.Code.Label")) so the PreviewCode Code attribute matches other
PreviewCodes; update the occurrence in Label.razor where PreviewCode Code="@new(
name: null, snippet: "Progress.Code.Label" )" is declared.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Colors.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/CustomStyles.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Indeterminate.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Label.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Sizes.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Usage.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Value.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Colors.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/CustomStyles.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Indeterminate.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Label.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Sizes.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Usage.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Value.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razorsrc/LumexUI/Components/Progress/LumexProgress.razorsrc/LumexUI/Styles/Progress.cssrc/LumexUI/Styles/_theme.css
🚧 Files skipped from review as they are similar to previous changes (11)
- docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/CustomStyles.razor
- docs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Colors.razor
- src/LumexUI/Styles/Progress.cs
- docs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Usage.razor
- docs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/CustomStyles.razor
- docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Usage.razor
- docs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Value.razor
- docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Indeterminate.razor
- docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Label.razor
- src/LumexUI/Components/Progress/LumexProgress.razor
- docs/LumexUI.Docs.Client/Pages/Components/Progress/Examples/Value.razor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razor (1)
11-11: Use consistent Blazor terminology: replacepropwithparameter.Lines 11, 27, and 41 use React-style wording (
prop) while the rest of the documentation usesparameter. Keeping this consistent avoids confusion for Blazor developers.Suggested doc wording update
- Note: Make sure to pass the <code>AriaLabel</code> prop when the <code>Label</code> prop is not provided. This is required for accessibility. + Note: Make sure to pass the <code>AriaLabel</code> parameter when the <code>Label</code> parameter is not provided. This is required for accessibility.- You can use the <code>IsIndeterminate</code> prop to display an indeterminate progress bar. This is useful when you don't know how long an operation will take. + You can use the <code>IsIndeterminate</code> parameter to display an indeterminate progress bar. This is useful when you don't know how long an operation will take.- Note: If you pass the <code>Label</code> prop you don't need to pass <code>AriaLabel</code> prop anymore. + Note: If you pass the <code>Label</code> parameter, you don't need to pass the <code>AriaLabel</code> parameter anymore.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razor` at line 11, Replace the React-style word "prop" with Blazor terminology "parameter" in the Progress component docs—update the sentences that reference AriaLabel and Label (e.g., "AriaLabel prop" and "Label prop") to use "parameter" instead so wording is consistent across Progress.razor; ensure all occurrences (notably the instances at the current lines referencing AriaLabel and Label) are changed and run a quick spellcheck to keep documentation consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razor`:
- Line 11: Replace the React-style word "prop" with Blazor terminology
"parameter" in the Progress component docs—update the sentences that reference
AriaLabel and Label (e.g., "AriaLabel prop" and "Label prop") to use "parameter"
instead so wording is consistent across Progress.razor; ensure all occurrences
(notably the instances at the current lines referencing AriaLabel and Label) are
changed and run a quick spellcheck to keep documentation consistent.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Label.razordocs/LumexUI.Docs.Client/Pages/Components/Progress/Progress.razor
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/LumexUI.Docs.Client/Pages/Components/Progress/PreviewCodes/Label.razor
Contributor
Author
|
@desmondinho Reworked the progress component to match the HeroUI. Please let me know if I should adjust anything else |
Contributor
Hey! Sorry, I missed the notification somehow :/ Thank you for addressing the comments. I will check both your PRs tomorrow Edit: I've just realized that the other one has already been reviewed |
desmondinho
requested changes
Mar 5, 2026
Contributor
There was a problem hiding this comment.
I see that a couple of things are missing or slightly off, so I will take a closer look tomorrow. I can also apply the changes in this file myself if you'd like
Contributor
|
Hey @kevinvenclovas, Very good job! We are almost done with this component. Thank you very much for contributing ❤️ |
Contributor
Author
|
@desmondinho I resolved your last requested changes. Please take a look and if you want you can finalize the component by yourself |
Replace the custom `progress-loading` keyframe with HeroUI's `indeterminate-bar` animation: a scaleX/translateX curve from (translateX(-50%) scaleX(0.2)) to (translateX(100%) scaleX(1)) using cubic-bezier(0.65, 0.815, 0.735, 0.395) over 1.5s. The previous keyframe (-100% -> 400% translateX) produced a thin sliding bar that didn't match the HeroUI reference visual and conflicted with the indicator's own inline transform. The class name is also renamed to `animate-indeterminate-bar` to match HeroUI's utility; the variant in Styles/Progress.cs will be updated in Phase 2.
Replace the single `bg-stripe-gradient` utility (which read four
inline CSS variables set from C#) with a functional Tailwind v4
utility `bg-stripe-gradient-*`. Each invocation resolves the suffix
through `--value(--color-*-200)` and `--value(--color-*)` to produce
the classic 4-band 45deg striped gradient using the matching theme
colors. Also adds a `bg-stripe-size` utility for the 1.25rem stripe
size, matching HeroUI's naming.
This eliminates the need for inline `--stripe-color` / `--stripe-color-light`
injection in `LumexProgress.GetIndicatorStyle()` and brings the stripe
mechanism in line with HeroUI's `bg-stripe-gradient-{color}` utilities.
The literal class names (`bg-stripe-gradient-primary`, etc.) will be
emitted from the compound variants in `Styles/Progress.cs` in Phase 2
so that Tailwind's scanner can statically discover them.
Track: drop w-full, rounded-full, bg-default-200, dark:bg-default-100; add z-0, bg-default-300/50, rtl:rotate-180. The radius and width are now owned by the radius variant and the parent layout respectively. Indicator: keep h-full only. The transition (transition-all duration-500 ease-in-out) and the hard-coded rounded-full are removed; both will be re-introduced through the radius variant (Phase 2.3) and the DisableAnimation variant (Phase 3). Label / Value: drop text-small font-medium text-foreground from the base. Typography now comes entirely from the size variant (Phase 2.2), matching HeroUI's empty label/value slots.
Track heights bumped up to match HeroUI: sm h-1 (same), md h-3 (was h-2), lg h-5 (was h-3). Label and Value text sizes shifted up one notch: text-small / text-medium / text-large (was text-tiny / text-small / text-medium).
Each Radius case now adds the matching `rounded-*` class to both the track and the indicator. Previously only the track was rounded; combined with the hard-coded `rounded-full` on the indicator base (removed in Phase 2.1), a non-full Radius left the indicator with no rounding at all. Now `Radius=None` produces a square indicator and `Radius=Full` a pill, matching HeroUI's behavior.
Replace the seven Color CompoundVariants that pulled classes from ColorVariants.Solid (which include text-*-foreground, unused on an empty indicator and polluting twMerge resolution) with a single Variants[Color] block that sets only the indicator background. `Default` now uses `bg-default-400` to match HeroUI (was `bg-default` via ColorVariants.Solid). The other colors map directly: Primary -> bg-primary, Secondary -> bg-secondary, Success -> bg-success, Warning -> bg-warning, Danger -> bg-danger, Info -> bg-info. This matches HeroUI's `variants.color` structure and aligns with sibling Lumex components (e.g. LumexSpinner.Color).
The Indeterminate=true case now sets `absolute w-full origin-left animate-indeterminate-bar` on the indicator (was just `animate-progress-loading`). The position + transform-origin combo is what HeroUI uses to drive the scaleX-based bar without overlapping the indicator's own translateX in the determinate path. The animation class name now matches the keyframe renamed in Phase 1.1; the old `animate-progress-loading` no longer exists.
The disabled state now uses the theme-defined `opacity-disabled` utility (which resolves to `--lumex-opacity-disabled` via the `@utility opacity-*` functional utility in _theme.css). Matches HeroUI's `opacity-disabled` token and lets the disabled opacity be themed instead of hard-coded.
Replace ProgressTests.cs with ProgressTests.razor following the
recently-added test pattern (DateboxTests.razor, SpinnerTests.razor,
AlertTests.razor): inherit TestContext, render with `@<LumexProgress />`
inline, use FindBySlot for assertions.
Fix the six assertions that checked `style.Contain("width: NN%")`
against an implementation that emits `transform: translateX(...)`.
The expected offsets now reflect the indicator's translate convention
(percentage 0% -> offset 100%, percentage 100% -> offset 0%) and the
clamping behavior.
Rename the indeterminate-class assertion from `animate-progress-loading`
to `animate-indeterminate-bar` and replace the
`width: 100%` indeterminate test with two assertions that match the new
implementation: (a) no inline `style` attribute on the indicator,
(b) the `absolute w-full origin-left` positioning classes are present.
Drop the stale `aria-label == "Loading"` assertion from the indeterminate
test — the synthesized fallback was removed in Phase 3.3, and aria-label
now flows in exclusively through splat attributes.
Add coverage for behaviors introduced by the parity refactor:
* data-disabled and data-indeterminate render as lowercase
"true" / "false" via ToAttributeValue() (Phase 4).
* aria-valuetext is omitted when indeterminate (Phase 3.4).
All 41 Progress tests pass (`dotnet test --filter ~Progress`).
Update two alert callouts that referenced the removed `AriaLabel` prop (Phase 3.2) to refer to the `aria-label` HTML attribute that consumers pass via splat. Rename the "ProgressBar" Custom Styles heading to "Progress" — the component itself was renamed in this PR.
With the synthesized "Loading" aria-label fallback removed in Phase 3.3, this example otherwise rendered an indeterminate progressbar with no accessible name. Adding `aria-label="Loading..."` keeps the example accessibility-correct and mirrors the pattern used in the other example files (Usage, Colors, Sizes, Striped).
Phase 3.3 removed UpdateAdditionalAttributes(), which had been
synthesizing `aria-label` from the Label prop. The docs callout
("If you pass the Label prop you don't need to pass aria-label")
became false: a `<LumexProgress Label="Downloading..." Value="50" />`
rendered with no accessible name despite the visible text.
Restore the contract by binding aria-label directly on the root,
guarded so an empty Label leaves the attribute off. Placement before
`@attributes` means a consumer-supplied splat `aria-label` still
overrides (Blazor attribute precedence: later wins).
This is a slight deviation from HeroUI, which uses aria-labelledby
pointing at the label span's id. The functional outcome (screen reader
announces the label) is the same, and no id generator infrastructure
exists in Lumex yet.
Three new cases:
* When Label is provided, it becomes the base's aria-label.
* When neither Label nor a splat aria-label is supplied, the
attribute is absent (no synthesized fallback like the old
UpdateAdditionalAttributes did).
* When both Label and splat aria-label are set, the splat wins
(consumer override).
The four GetAria*() helpers and GetIndicatorStyle() were single-
expression methods with verb names, even though the rest of the file
already uses expression-bodied properties for the same shape of
computed value (ClampedValue, Percentage, ValueText). Convert them to
properties for consistency.
Replace `value.ToString("0.##", CultureInfo.InvariantCulture)` with
`Invariant($"{value:0.##}")` via `using static System.FormattableString;`.
Same culture-invariant formatting, half the noise at the call site, and
the `System.Globalization` import is no longer needed.
The local `offset` variable in the old GetIndicatorStyle is folded into
the interpolation since there's no longer a multi-step body to clarify.
Markup updates: `@GetAriaValueNow()` -> `@AriaValueNow`, etc.
…Style The indeterminate variant adds `animate-indeterminate-bar` to the indicator, and the keyframe's `transform` overrides any regular inline transform during animation frames. The Indeterminate guard was preventing the emission of a `style` attribute that would have been silently shadowed anyway — purely cosmetic. Removing it makes `IndicatorStyle` a plain non-nullable property and removes one branch. The keyframe still owns the visual. Drop the now-obsolete WhenIndeterminate_ShouldOmitInlineTransform test; the inline transform is present but harmless.
Previously a consumer passing `ValueLabel="Uploading..."` together with
`Indeterminate=true` got nothing — neither aria-valuetext (AriaValueText
returned null when indeterminate) nor a visible value span (markup
gated on `ShowValueLabel && !Indeterminate`). That left the ValueLabel
prop useless for the indeterminate case it was arguably best suited for.
Match HeroUI's behavior:
AriaValueText => Indeterminate ? ValueLabel : ValueText;
Drop the `!Indeterminate` gate from the markup, and render
`@AriaValueText` (instead of `@ValueText`) in the value span. Result:
* Indeterminate + ValueLabel -> aria-valuetext = ValueLabel,
value slot shows ValueLabel.
* Indeterminate, no ValueLabel-> aria-valuetext absent,
value slot renders empty (matches
HeroUI's React render of undefined).
* Determinate -> aria-valuetext = ValueText,
value slot shows ValueText.
Tests: rename WhenIndeterminate_ShouldOmitAriaValueText to clarify the
"without ValueLabel" qualifier, add a forwarding test, and replace
ShouldNotRenderValueLabelWhenIndeterminate with one that asserts the
new (inverted) behavior.
The format string was only doing real work when Percentage was a non- terminating fraction (e.g. Value=100, MaxValue=300 -> 33.33333333…), truncating the 17-digit IEEE-754 tail to two decimals. HeroUI doesn't cap precision either — they template-literal the JS number directly. Match that, accept the rare pathological case, and keep the property bodies minimal. `Invariant(...)` still keeps the decimal separator locale-independent.
Move the inline `string.IsNullOrEmpty( Label ) ? null : Label` expression out of the markup and into a private property, grouping it with the other Aria* computed values. Markup binding shrinks to `@AriaLabel`.
Single animated bar instead of two near-duplicates, drop the unused IJSRuntime inject, drop the redundant outer wrapper div (one bar doesn't need a flex column), and pad the Timer/conditional with the project's standard spacing. Add the missing `aria-label="Loading..."` — the docs page explicitly calls out that aria-label is required when Label is not provided, and this was the only example violating that guidance. Also switch `ShowValueLabel="true"` to the attribute-only `ShowValueLabel` form for consistency with the Striped and CustomStyles examples.
Match the convention applied in Value.razor — Blazor bool parameters use the explicit `@true` form rather than `="true"` (string-coerced) or attribute-only (implicit). Consistent across the Progress examples.
Apply the same convention as the example files — every boolean parameter (Indeterminate / Disabled / ShowValueLabel) on the @<LumexProgress ... /> calls now uses `="@true"` instead of the attribute-only implicit-true form. 44/44 Progress tests still pass.
Last attribute-only boolean in the Progress example tree. Brings CustomStyles.razor in line with the rest (Striped, Indeterminate, Value).
Swap System.Threading.Timer for PeriodicTimer with an async loop. The timer fires on the renderer's synchronization context indirectly via `await InvokeAsync(StateHasChanged)`, cancellation is implicit (disposing the PeriodicTimer makes WaitForNextTickAsync return false and the loop exits cleanly), and the class swaps from IAsyncDisposable to plain IDisposable. Functionally identical to the old Timer-based version: every 500 ms, the value cycles 0 -> 100 in steps of 10, then resets.
Replace the "Rainbow Energy" demo with HeroUI's "Lose weight" example verbatim: Size=Small, Radius=Small, Value=65, all five slot classes mapped 1:1 (base / track / indicator / label / value). Drops the outer flex-column wrapper — the example is a single bar and the `max-w-md` on the base slot now constrains the width. Tailwind class names match HeroUI's Tailwind v4 naming (`bg-linear-to-r` rather than the deprecated `bg-gradient-to-r`).
Match the convention used in the Switch, Tooltip, and other CustomStyles examples: the slot bag lives in an `@code` field named `_classes` and the markup binds `Classes="@_classes"`. Keeps the tag short and the slot composition in one readable block.
The previous gradient paired `--color-*-200` with `--color-*` (the base). In dark theme `--lumex-default` aliases to `--lumex-default-200`, so both gradient bands collapsed to the same color and the default striped progress bar had no visible stripes. The same pairing also produced low perceptual contrast for `secondary` and `danger` in dark theme. Switch to a color-mix derivation: one band is the base `--color-*`, the other is `color-mix(in oklch, --color-*, black 25%)`. The base color stays dominant (matching the non-striped indicator) and the stripe band is always a perceptually distinct darker tint regardless of theme. Deviation from HeroUI noted: HeroUI pairs two distinct theme shades per color. We derive the second band programmatically; the visual goal — visible stripes layered over the indicator color — is the same.
25% black blend was leaving danger (and other already-dark base colors) with insufficient lightness delta against the base band. Math check on rose-500 (oklch L=0.645): the 25% pairing only dropped lightness by ~0.16, which was actually less than the previous *-200/* pairing's ~0.19 gap that this commit replaced. 40% black brings the lightness delta to ~0.26, restoring perceptual separation across all hues without crossing into "muddy" territory for the vivid colors.
Match the convention of the Sizes and Colors sections (one-line `<p>Use the <code>X</code> parameter to ...</p>` intro before the example). The Striped section was the only Usage subsection missing this.
desmondinho
force-pushed
the
LumexProgressBar
branch
from
May 18, 2026 20:56
7dbb1fc to
e45d00e
Compare
Wrap the Custom Styles content in `<DocsSlotsSection Slots="@_slots">` to render the named-slots list, matching the pattern used by every other component docs page (Tooltip, Card, Datebox, Checkbox, …). The `_slots` array describes all six progress slots: Base, LabelWrapper, Label, Value, Track, and Indicator.
desmondinho
approved these changes
May 18, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #282 +/- ##
==========================================
- Coverage 96.95% 92.74% -4.22%
==========================================
Files 70 170 +100
Lines 1542 2850 +1308
Branches 150 425 +275
==========================================
+ Hits 1495 2643 +1148
- Misses 28 111 +83
- Partials 19 96 +77 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
desmondinho
changed the title
Contributor
|
Hey @kevinvenclovas, It took me quite a long time to finally get to this PR... I made some minor changes here and there to achieve almost 100% parity with HeroUI, but your core contribution remains untouched. I also made atomic commits so the changes are easier to review. I'd like to thank you once again for your contribution and apologize for being so slow with all of this! Next on the list is #273 :) |
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.
Description
What's been done?
Checklist
Summary by CodeRabbit