sts-sponsors team mailing list archive

[Jones Ogolo <mp+442211@xxxxxxxxxxxxxxxxxx>]

Jones Ogolo has proposed merging ~jonesogolo/maas-site-manager:1561-update-mobile-menu into maas-site-manager:main.

Commit message:
Added a close menu button on smaller screens
Close nav menu on navlink click

Requested reviews:
  MAAS Committers (maas-committers)

For more details, see:
https://code.launchpad.net/~jonesogolo/maas-site-manager/+git/maas-site-manager/+merge/442211

Q.A:
1. Goto `/sites`
2. Resize the screen to mobile (< 460px)
3. Click the `Menu` button on the navbar
4. Ensure that the 'Close menu' button is displayed.
5. Click any of the navlinks or the 'Close menu' button and ensure that the menu is closed
-- 
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.

diff --git a/frontend/src/_utils.scss b/frontend/src/_utils.scss
index 972f9d4..b02fd0c 100644
--- a/frontend/src/_utils.scss
+++ b/frontend/src/_utils.scss
@@ -47,6 +47,9 @@
 .u-no-padding {
   padding: 0 !important;
 }
+.u-nudge-down--small {
+  padding-top: $sph--small !important;
+}
 // hide elements visually, but keep them accessible to screen readers
 .u-visually-hidden {
   position: absolute !important;
@@ -56,3 +59,18 @@
   clip: rect(1px, 1px, 1px, 1px) !important;
   white-space: nowrap !important;
 }
+.u-display-x-small {
+  display: initial;
+
+  @media (min-width: $breakpoint-x-small) {
+    display: none;
+  }
+}
+
+.u-display-small {
+  display: none;
+
+  @media (min-width: $breakpoint-x-small) {
+    display: initial;
+  }
+}
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;
+
 const Navigation = ({ isLoggedIn }: NavProps): JSX.Element => {
   const [isCollapsed, setIsCollapsed] = useLocalStorageState<boolean>("appSideNavIsCollapsed", { defaultValue: true });
   const location = useLocation();
@@ -48,6 +50,12 @@ const Navigation = ({ isLoggedIn }: NavProps): JSX.Element => {
     }
   }, [isLoggedIn, setIsCollapsed]);
 
+  const handleNavlinkClick = () => {
+    if (window.screen.width <= MOBILE_BREAKPOINT_WIDTH) {
+      setIsCollapsed(true);
+    }
+  };
+
   return (
     <>
       <header aria-label="navigation" className="l-navigation-bar">
@@ -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"
+                      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>
diff --git a/frontend/src/components/Navigation/NavigationCollapseToggle/NavigationCollapseToggle.tsx b/frontend/src/components/Navigation/NavigationCollapseToggle/NavigationCollapseToggle.tsx
index 4645a26..8415700 100644
--- a/frontend/src/components/Navigation/NavigationCollapseToggle/NavigationCollapseToggle.tsx
+++ b/frontend/src/components/Navigation/NavigationCollapseToggle/NavigationCollapseToggle.tsx
@@ -19,7 +19,10 @@ const NavigationCollapseToggle = ({
       <Button
         appearance="base"
         aria-label={`${!isCollapsed ? "collapse" : "expand"} main navigation`}
-        className={classNames("is-dense has-icon is-dark u-no-margin l-navigation-collapse-toggle", className)}
+        className={classNames(
+          "is-dense has-icon is-dark u-no-margin l-navigation-collapse-toggle u-display-small",
+          className,
+        )}
         onClick={(e) => {
           setIsCollapsed(!isCollapsed);
           // Make sure the button does not have focus
diff --git a/frontend/src/components/Navigation/NavigationItem/NavigationItem.tsx b/frontend/src/components/Navigation/NavigationItem/NavigationItem.tsx
index a7161de..02c3d53 100644
--- a/frontend/src/components/Navigation/NavigationItem/NavigationItem.tsx
+++ b/frontend/src/components/Navigation/NavigationItem/NavigationItem.tsx
@@ -11,6 +11,7 @@ import { Link } from "@/router";
 type Props = {
   navLink: NavLink;
   path: RoutePath;
+  onClick: () => void;
 };
 
 const LinkContent = ({ navLink }: { navLink: NavLink }) => (
@@ -26,12 +27,13 @@ const LinkContent = ({ navLink }: { navLink: NavLink }) => (
   </>
 );
 
-const NavigationItem = ({ navLink, path }: Props): JSX.Element => {
+const NavigationItem = ({ navLink, path, onClick }: Props): JSX.Element => {
   const id = useId();
   const linkProps = {
     className: "p-side-navigation__link",
     id: `${navLink.label}-${id}`,
     onClick: (event: React.MouseEvent<HTMLAnchorElement, MouseEvent>) => {
+      onClick();
       // removing the focus from the link element after click
       // this allows the side navigation to collapse on mouseleave
       event.currentTarget.blur();
diff --git a/frontend/src/components/Navigation/NavigationList/NavigationList.tsx b/frontend/src/components/Navigation/NavigationList/NavigationList.tsx
index 9e78639..22f10b5 100644
--- a/frontend/src/components/Navigation/NavigationList/NavigationList.tsx
+++ b/frontend/src/components/Navigation/NavigationList/NavigationList.tsx
@@ -14,9 +14,10 @@ type Props = {
   hasIcons?: boolean;
   items: NavItem[];
   path: RoutePath;
+  onClick: () => void;
 };
 
-const NavigationItemGroup = ({ group, path }: { group: NavGroup } & Pick<Props, "path">) => {
+const NavigationItemGroup = ({ group, path, onClick }: { group: NavGroup } & Pick<Props, "path" | "onClick">) => {
   const id = useId();
   const hasActiveChild = useMemo(() => {
     for (const navLink of group.navLinks) {
@@ -44,7 +45,7 @@ const NavigationItemGroup = ({ group, path }: { group: NavGroup } & Pick<Props, 
         </span>
         <ul aria-labelledby={`${group.groupTitle}-${id}`} className="p-side-navigation__list">
           {group.navLinks.map((navLink, i) => (
-            <NavigationItem key={i} navLink={navLink} path={path} />
+            <NavigationItem key={i} navLink={navLink} onClick={onClick} path={path} />
           ))}
         </ul>
       </li>
@@ -52,14 +53,14 @@ const NavigationItemGroup = ({ group, path }: { group: NavGroup } & Pick<Props, 
   );
 };
 
-const NavigationList = ({ hideDivider = false, items, path, isDark, hasIcons }: Props): JSX.Element => {
+const NavigationList = ({ hideDivider = false, items, path, isDark, hasIcons, onClick }: Props): JSX.Element => {
   return (
     <div className={classNames({ "is-dark": isDark, "p-side-navigation--icons": hasIcons })}>
       <ul className={classNames("p-side-navigation__list", { "no-divider": hideDivider })}>
         {items.map((item, i) => {
           if (isNavGroup(item)) {
-            return <NavigationItemGroup group={item} key={`${i}-${item.groupTitle}`} path={path} />;
-          } else return <NavigationItem key={`${i}-${item.label}`} navLink={item} path={path} />;
+            return <NavigationItemGroup group={item} key={`${i}-${item.groupTitle}`} onClick={onClick} path={path} />;
+          } else return <NavigationItem key={`${i}-${item.label}`} navLink={item} onClick={onClick} path={path} />;
         })}
       </ul>
     </div>
diff --git a/frontend/src/components/Navigation/_Navigation.scss b/frontend/src/components/Navigation/_Navigation.scss
index 5b97ae3..6a2eff0 100644
--- a/frontend/src/components/Navigation/_Navigation.scss
+++ b/frontend/src/components/Navigation/_Navigation.scss
@@ -12,6 +12,9 @@
       visibility: visible;
       button {
         background-color: rgba(255, 255, 255, 0.05);
+        &.b-btn-transparent {
+          background-color: transparent;
+        }
       }
       @media only screen and (min-width: ($breakpoint-small + 1)) {
         transform: translateX(#{$application-layout--side-nav-width-expanded - 3rem}) translateY(0.8rem);