MudMenu: Hide Button when Label, Icon and Content are unset#10349
MudMenu: Hide Button when Label, Icon and Content are unset#10349henon merged 6 commits intoMudBlazor:devfrom
Conversation
|
@ScarletKuro do you think this is the right approach? Also I'm not sure about the parameter name If you think this is OK I can add tests and docs tomorrow. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #10349 +/- ##
==========================================
- Coverage 91.48% 91.43% -0.06%
==========================================
Files 415 417 +2
Lines 13053 13222 +169
Branches 2473 2538 +65
==========================================
+ Hits 11942 12089 +147
- Misses 548 554 +6
- Partials 563 579 +16 ☔ View full report in Codecov by Sentry. |
|
Had to get caught up, question- Shouldn't that button always be hidden or visibility collapse or something when not in a popover? I think could probably solve it all with just a css class when laid out in that manner under a popover to not show it and disable it but the popover version would work fine |
|
@versile2 @henon @danielchalmers |
danielchalmers
left a comment
There was a problem hiding this comment.
Might be a stupid question but why can't we add the mud-menu-hidden class if the activator content is null? Why do we need a second property?
|
I guess @danielchalmers has a point. Should we decide to keep the parameter though I'd like it to be named |
When the activator content is null a default button is shown. That's the main use of MudMenu. |
Right, no activator content or label = hidden? <MudMenu>
<MudMenuItem>Enlist</MudMenuItem>
<MudMenuItem>Barracks</MudMenuItem>
<MudMenuItem>Armory</MudMenuItem>
</MudMenu>Do people use the empty button that's shown in your Before example? |
I like this idea.
I hope not. I think this behavior qualifies as unintended / buggy. |
We can do this but we should explicitly document it with an example in the docs. |
| [TestCase(null, "Close menu", "Close menu")] | ||
| [TestCase("x", "Close menu", "Close menu")] | ||
| [TestCase(null, null, null, Description = "Ensures aria-label is not present instead of empty string")] |
There was a problem hiding this comment.
I had to remove these test cases because the button isn't rendered anymore when the label is null.
|
Thanks @meenzen |
Description
see #10122 (comment)
Before:
After:
New docs section:
How Has This Been Tested?
unit, visually
Type of Changes
Checklist
dev).