-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various Components: Rename DisableElevation
to DropShadow
#8592
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #8592 +/- ##
==========================================
+ Coverage 89.82% 90.00% +0.17%
==========================================
Files 412 418 +6
Lines 11878 12029 +151
Branches 2364 2371 +7
==========================================
+ Hits 10670 10827 +157
+ Misses 681 667 -14
- Partials 527 535 +8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great choice on the new name
@@ -206,7 +206,7 @@ | |||
</div> | |||
</div> | |||
<div class="d-flex align-end flex-grow-1 pa-4"> | |||
<MudButton Variant="Variant.Filled" Color="Color.Primary" FullWidth="true" DisableElevation="true">Pay now</MudButton> | |||
<MudButton Variant="Variant.Filled" Color="Color.Primary" FullWidth="false" DropShadow="true">Pay now</MudButton> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did fullwidth change? Looks better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oopsie, good catch, I think I inverted the wrong property here
@namespace MudBlazor.Docs.Examples | ||
|
||
<MudButtonGroup Color="Color.Primary" Variant="Variant.Filled" DisableElevation="@_disableElevation"> | ||
<MudButtonGroup Color="Color.Primary" Variant="Variant.Filled" DropShadow="@_dropShadow"> | ||
<MudButton>One</MudButton> | ||
<MudButton>Two</MudButton> | ||
<MudButton>Three</MudButton> | ||
</MudButtonGroup> | ||
|
||
<MudSwitch @bind-Value="_disableElevation" Label="Disable elevation" Color="Color.Primary" /> | ||
<MudSwitch @bind-Value="_dropShadow" Label="Drop shadow" Color="Color.Primary" /> | ||
|
||
@code { | ||
private bool _disableElevation = true; | ||
private bool _dropShadow = true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the goal of that doc is to show deviation from the default behavior, drop shadow should be false, right?
Are you going to keep the |
Yes, Elevation makes sense with an int value. That is the main reason why I renamed DisableElevation to DropShadow. |
would it make sense if the elevation property could enable/disable it without the need for an extra boolean property? honestly not super familiar with the elevation stuff |
I think it is rare that elevation can be controlled directly. Mostly you set the elevation in the theme and it applies to all equally. That's why this would not be easy to change. |
Description
We want to move from
DisableSomething
to justSomething
orEnableSomething
depending on the property. This PR tackles onlyDisableElevation
and renames it toDropShadow
. Of course default value and all invocations have been inverted.See #6131
How Has This Been Tested?
Types of changes
Checklist:
dev
).