Skip to content
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

NavLink and NavGroup: IconOnly Mode #8809

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

esda98
Copy link

@esda98 esda98 commented Apr 25, 2024

This PR adds a boolean property to the MudNavLink and MudNavGroup components that controls the conditional rendering of the child elements of MudNavLink and the Title of MudNavGroup.

Description

The PR adds an if around the <div> tags that wrap @ChildContent for MudNavLink and @Title for MudNavGroup controlled by the new boolean parameter for the components named IconOnly.

This is to enable functionality of collapsed sidebars, common in vertical navigation pane paradigms, to save horizontal space but still allow navigation through the icons on the sidebar.

How Has This Been Tested?

  • New Component Specific Unit Tests
  • Visually through Docs

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Examples

Screenshot 2024-04-24 231740

IconOnlyModeExample.mp4

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

…de for MudNavLink and MudNavGroup components.
@github-actions github-actions bot added docs Related to docs enhancement New feature or request PR: needs review labels Apr 25, 2024
Copy link

codecov bot commented Apr 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.14%. Comparing base (28bc599) to head (d2b71dd).
Report is 116 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #8809      +/-   ##
==========================================
+ Coverage   89.82%   90.14%   +0.31%     
==========================================
  Files         412      421       +9     
  Lines       11878    12215     +337     
  Branches     2364     2411      +47     
==========================================
+ Hits        10670    11011     +341     
+ Misses        681      663      -18     
- Partials      527      541      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielchalmers
Copy link
Contributor

danielchalmers commented Apr 27, 2024

IconOnly should probably be a cascading value or be forced on all children. In what situation would some be icon-only and not other?


Should it collapse the title too? Not sure how that would work

image

Side note: the hamburger menu icon should really align with the rest


Titles in the new example are too far to the right and don't look right

image

@henon
Copy link
Collaborator

henon commented Apr 27, 2024

Should it collapse the title too? Not sure how that would work

image

Side note: the hamburger menu icon should really align with the rest

I agree with this

@henon
Copy link
Collaborator

henon commented Apr 27, 2024

@esda98 this example isn't intuitive. Nobody will figure out that they have to click the burger icon to collapse. Just add a MudSwitch at the bottom of the example that says IconOnly instead

@henon
Copy link
Collaborator

henon commented Apr 27, 2024

Compare with the Variant="DrawerVariant.Mini" example in the drawer docs, this would be also a good way of improving the example

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Related to docs enhancement New feature or request PR: needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants