Diff comments:
> diff --git a/frontend/src/components/Navigation/Navigation.tsx
> b/frontend/src/components/Navigation/Navigation.tsx
> index 701e93c..1b39a24 100644
> --- a/frontend/src/components/Navigation/Navigation.tsx
> +++ b/frontend/src/components/Navigation/Navigation.tsx
> @@ -37,6 +37,8 @@ type NavProps = {
> isLoggedIn: boolean;
> };
>
> +const MOBILE_BREAKPOINT_WIDTH = 460;
Agree with Thorsten here. We should at least try and keep them in sync with CSS
breakpoints.
breaktpoints.ts file with breakpoints referring to each scss value, clearly
marked by a comment should be enough to begin with.
FYI we use https://vanillaframework.io/docs/settings/breakpoint-settings
> +
> const Navigation = ({ isLoggedIn }: NavProps): JSX.Element => {
> const [isCollapsed, setIsCollapsed] =
> useLocalStorageState<boolean>("appSideNavIsCollapsed", { defaultValue: true
> });
> const location = useLocation();
> @@ -82,18 +90,38 @@ const Navigation = ({ isLoggedIn }: NavProps):
> JSX.Element => {
> <NavigationBanner>
> <div className="l-navigation__controls">
> <NavigationCollapseToggle isCollapsed={isCollapsed}
> setIsCollapsed={setIsCollapsed} />
> + <Button
> + appearance="base"
> + className="is-dark u-display-x-small b-btn-transparent"
No need to create custom classes for this. You could use vanilla framework
visibility util classes instead, e.g. u-hide--small u-hide--medium
u-hide--large in this case: https://vanillaframework.io/docs/utilities/hide
> + onClick={(e) => {
> + setIsCollapsed(!isCollapsed);
> + // Make sure the button does not have focus
> + // .l-navigation remains open with :focus-within
> + e.stopPropagation();
> + e.currentTarget.blur();
> + }}
> + >
> + Close menu
> + </Button>
> </div>
> </NavigationBanner>
> </div>
> {isLoggedIn && (
> <div className="p-panel__content">
> - <NavigationList hasIcons isDark items={navItems}
> path={path} />
> - <NavigationList hasIcons isDark items={settingsNavItems}
> path={path} />
> - <NavigationList hasIcons isDark items={navItemsAccount}
> path={path} />
> + <NavigationList hasIcons isDark items={navItems}
> onClick={handleNavlinkClick} path={path} />
> + <NavigationList hasIcons isDark items={settingsNavItems}
> onClick={handleNavlinkClick} path={path} />
> + <NavigationList hasIcons isDark items={navItemsAccount}
> onClick={handleNavlinkClick} path={path} />
> </div>
> )}
> </span>
> - <NavigationList hasIcons hideDivider isDark
> items={navItemsBottom} path={path} />
> + <NavigationList
> + hasIcons
> + hideDivider
> + isDark
> + items={navItemsBottom}
> + onClick={handleNavlinkClick}
> + path={path}
> + />
> </div>
> </div>
> </nav>
--
https://code.launchpad.net/~jonesogolo/maas-site-manager/+git/maas-site-manager/+merge/442211
Your team MAAS Committers is requested to review the proposed merge of
~jonesogolo/maas-site-manager:1561-update-mobile-menu into
maas-site-manager:main.
--
Mailing list: https://launchpad.net/~sts-sponsors
Post to : sts-sponsors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~sts-sponsors
More help : https://help.launchpad.net/ListHelp
