Mob hamburger menu

[funbunch]

Description

Removed buttons on mobile on the /calendar route and added a hamburger menu.

Type of change

Please select everything applicable. Please, do not delete any lines.

• 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)
• This change requires a documentation update
• This change requires an update to testing

Issue

• Is this related to a specific issue? If so, please specify: Hamburger menu on mobile, remove buttons #549

Checklist:

• This PR is up to date with the main branch, and merge conflicts have been resolved
• I have executed npm run test and npm run test:e2e and all tests have passed successfully or I have included details within my PR on the failure.
• I have executed npm run lint and resolved any outstanding errors. Most issues can be solved by executing npm run format
• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings

[funbunch]

New mob menu.

[DevinCLane]

been excited for this!

[geraldiner]

Nice work, and is working as intended! 🥳

I noticed something wonky with the buttons on desktop and had a thought about implementation, so feel free to take or leave that part. 🙂

[geraldiner]

Since we're specifying links as a prop now, could we send the defaultLinks at the parent level? My 2 cents is that the HamburgerNav should only display the links as passed, and not worry about the logic for determining what they should be.

Something like

// CalendarPage.jsx
const CALENDAR_NAV_LINKS = [...list of links here]

<HamburgerNav logo={Logo} logotext="./logotext.png" links={CALENDAR_NAV_LINKS} />

// LandingPage.jsx
const LANDING_NAV_LINKS = [...list of links here, can also do logic for signed in/out]

<HamburgerNav logo={"./logoicon.png"} logotext={"./logotext.png"} links={LANDING_NAV_LINKS} />

[funbunch]

I believe I had something like that before, but I wasn't getting it to work. My limited understanding, I'm sure. So kept it what seemed easiest in HamburgerNav.

I'm open though, and interested in what any other reviewers think!

[DevinCLane]

checking in again on this @funbunch. Did you want to implement this change or not? I think it's fine to merge for now, then we could make a separate issue for this refactor if we want to.

@geraldiner i think that @funbunch wasn't clear on how to implement this. if you have capacity could you help, otherwise we will merge it as is.

thanks!

[geraldiner]

So sorry it's been a while and even I forgot the context of my own comment. 😅

I'd say merge as-is for now. There might be some other things to refactor for the component and don't want to slow this PR down.

[DevinCLane]

Looks great overall.

i noticed that when you click outside the hamburger menu, it doesn't close. I'm going to say let's merge this anyway, and could you create a separate issue just for that, so someone else could take it?

[funbunch]

@DevinCLane @moses-codes Not sure why this is failing since I did run prettier and nothing was changed.

[DevinCLane]

hmm not sure, beyond what that error says

[funbunch]

@DevinCLane Got the formatting errors to pass! However, I see some other tests now fail and require some refactoring. Will continue to work on those next.