one first idea inline

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;

It feels like breakpoints should go to some central place so we can easily 
reuse them in different components. Also 460 seems a bit on the low end. See 
e.g. https://www.browserstack.com/guide/responsive-design-breakpoints or 
https://bulma.io/documentation/customize/variables/ for some best practices in 
breakpoint definitions.

> +
>  const Navigation = ({ isLoggedIn }: NavProps): JSX.Element => {
>    const [isCollapsed, setIsCollapsed] = 
> useLocalStorageState<boolean>("appSideNavIsCollapsed", { defaultValue: true 
> });
>    const location = useLocation();


-- 
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

Reply via email to