MudPopover: Add DropShadow property#8938
Conversation
github-actions
bot
added
enhancement
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #8938 +/- ##
==========================================
+ Coverage 89.82% 90.49% +0.66%
==========================================
Files 412 419 +7
Lines 11878 12191 +313
Branches 2364 2380 +16
==========================================
+ Hits 10670 11032 +362
+ Misses 681 627 -54
- Partials 527 532 +5 ☔ View full report in Codecov by Sentry. |
danielchalmers
left a comment
danielchalmers
left a comment
There was a problem hiding this comment.
Not a fan of having two properties to control the same thing and I don't know any other components that do this, but the others are fine with it and I get why it's useful
src/MudBlazor.UnitTests.Viewer/TestComponents/Popover/PopoverPropertyTest.razor
Outdated
Show resolved
Hide resolved
| [Parameter] | ||
| [Category(CategoryTypes.Popover.Appearance)] | ||
| public int Elevation { set; get; } = 8; | ||
| public int Elevation { set; get; } = MudGlobal.PopoverDefaults.Elevation; |
There was a problem hiding this comment.
We could discuss removing this parameter entirely. You know, just use MudGlobal.PopoverDefaults.Elevation directly and disallow setting individual elevation for each popover. I just don't see why you would need different elevation levels for different popups.
Pretty sure @danielchalmers would agree to this. What about you @ScarletKuro ? @Mr-Technician ? @JonBunator ? @Garderoben ?
There was a problem hiding this comment.
Yes I think that's better. Menu has a PopoverClass so users can add it themselves (mud-elevation-3) along with other specialized customization. For the average person it removes complexity from the library (and maintenance for contributors).
There was a problem hiding this comment.
Disallow setting individual elevation for each popover. I just don't see why you would need different elevation levels for different popups.
Work experience shows that businesses sometimes have strange ideas about UI/UX, so I prefer to have control over everything. That's why I'd ask the community what they think. I don't have a strong opinion on this, as my MudBlazor usage is limited to my personal projects and not related to work.
There was a problem hiding this comment.
I was running into an issue with the Drawer with a simple fix but it became a nightmare because we had to deal with edge cases with the scrollbars (for probably only a handful of users) so I gave up. At some point excessive customization and supporting every combination becomes restricting so it may be good to put your foot down and make people do it "the right way". This isn't specifically about this but more of a general point I want to make.
There was a problem hiding this comment.
I could see possible use cases for setting different elevations across multiple Popovers. I'm not sure I like pulling global configuration with no way to override it (outside of adding a class manually).
There was a problem hiding this comment.
Yeah, it is hard to argue with that.
5 participants
Description
Added
boolpropertyDropShadowto MudPopover (in preparation of a PR for #6995)Added
intpropertyElevationtoMudGlobal.Popoverclass with a default value of:8.DropShadow true (default)
DropShadow false (Elevation 0)
How Has This Been Tested?
Unit: Added two tests
Visually
Type of Changes
Checklist
dev).