MudStepper: Fix final step completed color#11012
Conversation
Fixed final step's completed color. When the MudStepper is linear, the final step will now show the completed colour once completed.
github-actions
bot
added
bug
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #11012 +/- ##
=======================================
Coverage 91.01% 91.01%
=======================================
Files 429 429
Lines 13943 13946 +3
Branches 2695 2696 +1
=======================================
+ Hits 12690 12693 +3
Misses 648 648
Partials 605 605 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Anu6is
left a comment
Anu6is
left a comment
There was a problem hiding this comment.
There should be a new test to confirm that the final step color is actually updated once completed
|
|
||
| private async Task SetCurrentStepColorAsync(Color color) | ||
| { | ||
| CurrentStepColor = color; |
There was a problem hiding this comment.
Don't set CurrentStepColor directly, calling SetValueAsync should be enough.
There was a problem hiding this comment.
Hi, just took a quick look during my lunch break. Only using SetValueAsync doesn't seem to be working here. Have I implemented it wrong?
Edit: I think I know the solution, will implement when I can
There was a problem hiding this comment.
Hi, just took a quick look during my lunch break. Only using SetValueAsync doesn't seem to be working here. Have I implemented it wrong?
All places that used CurrentStepColor needs to be replaced with _currentStepColor.Value like this place:
if it's parent component then
Parent?.GetState(x => x.CurrentStepColor).
| /// </summary> | ||
| [Parameter] | ||
| [Category(CategoryTypes.List.Behavior)] | ||
| public EventCallback<Color> CurrentStepColorChanged { get; set; } |
There was a problem hiding this comment.
Is this EventCallback necessary?
|
Hi, thanks for the review. This is my first PR so I was mainly trying to follow the existing style. I'll look into implementing what you've said when I can. |
| _currentStepColor = registerScope.RegisterParameter<Color>(nameof(CurrentStepColor)) | ||
| .WithParameter(() => CurrentStepColor) | ||
| .WithEventCallback(() => CurrentStepColorChanged) | ||
| .WithChangeHandler(async args => await SetCurrentStepColorAsync(args.Value)); |
There was a problem hiding this comment.
just make it
.WithChangeHandler(SetCurrentStepColorAsync);
private async Task SetCurrentStepColorAsync(ParameterChangedEventArgs<Color> color)
Modifies CssBuilder instead of changing the current colour parameter
|
I've changed how this works. Instead of changing the current colour parameter, I modified the condition for the CssBuilder. A step will display the completed colour either if it is not currently active, or if all steps have been completed and it is linear. Thoughts on this? Also, should I add a test for this? It says in contributing.md under "What does not need to be tested?" that the appearance of components does not need to be tested. |
Fixed when completed color isn't set but a different current color is set.
|
Sorry, just noticed after pushing that the same bug happens if you don't set a completed color but change the current step color. This new push fixes that |
I think in this case a test would be nice |
Great, I'll look at implementing this. Should I worry about the failing check? It's happening because the Parent? Null check isn't necessary, but it does follow the existing style |
Leave as is |
|
|
Thanks @Lewis-Pitman |
4 participants
Description
When the MudStepper is linear, the final step will now show the completed colour once completed.
Originally, it was impossible for the last step in a linear MudStepper to show the completed step color, as the last step was also the current step. This has been fixed so that if the MudStepper is linear and the last step has been completed, the current step color changes to the completed step color. The current step color is restored if the user resets the progress of the stepper.
This behaviour doesn't happen in a non linear MudStepper, as these allow you to go back and view other steps when all steps have been completed, and so having a different colour representing the current step is necessary.
Resolves #10962
How Has This Been Tested?
No tests have been added. The code does not alter existing code, and functions as expected on MudBlazor.Docs.Server
Type of Changes
Checklist
dev).