docs: Hide nav and add hamburger to collapse side nav #2386

edison-cy-yang · 2025-02-19T17:58:06Z

Motivations

We want to make the Atlantis doc site mobile responsive

Changes

TopNav layout changes during narrow viewport
Hamburger button to open nav menu during narrow viewport
NavMenu layout - move Jobber logo next to the close button in narrow viewport
Attach toggleMobileMenu onClick handler to every react-router-dom Link in NavMenu to close nav when clicked. The state update is only triggered if viewport is below medium.

Added

LeftDrawer is a simplified SideDrawer for the purpose of this responsive nav
isMobileMenuOpen state in AtlantisSiteContext

Changed

NavMenu contains both the desktop nav and mobile nav

Deprecated

Removed

Fixed

Security

Testing

Changes can be
tested via Pre-release

The breakpoint is 768px. Test the responsiveness in TopNav, as well as NavMenu (which can be opened from the hamburger menu button)

In Atlantis we use Github's built in pull request reviews.

…1-integrate-triton-to-next-gen-docs

* create header component * accurate header width calculation * ToggleThemeButton into Header * align ToggleThemeButton to the right of the header * make sure nav menu aligns with header * border on the header * Header takes up the whole width * make sure only BaseView.Main scrolls with header on top * move top nav to be inside BaseView.Main * refactor Triton SideDrawer into its own component and created TritonContext * header styling * dont use box as main due to lack of boxShadow * make header responsive in minimal mode * fix eslint * overflow auto on main routed div * ask triton button styling * only rounded top corners of main content * fixed main content not growing * fix BaseView border radius * some layout/styling tweaks * add header to collection pages and make scrollbar inside the content * isolate blending to only content card wrapper * align nav with header * Renamed Header to TopNav * use Box instead of div --------- Co-authored-by: Chris Murray <[email protected]>

cloudflare-workers-and-pages · 2025-02-19T18:01:59Z

Deploying atlantis with Cloudflare Pages

Latest commit:	`cbd9b25`
Status:	✅ Deploy successful!
Preview URL:	https://492881e5.atlantis.pages.dev
Branch Preview URL:	https://job-115212-hide-nav-and-add.atlantis.pages.dev

View logs

edison-cy-yang · 2025-02-21T21:52:30Z

packages/site/src/layout/AnimatedPresenceDisclosure.tsx

@@ -63,7 +66,7 @@ function AnimatedPresenceDisclosure({
          isTitleSelected ? styles.selected : ""
        } stickySectionHeader`}
      >
-        <Link to={to ?? "/"} tabIndex={0}>
+        <Link to={to ?? "/"} tabIndex={0} onClick={toggleMobileMenu}>


these onClick handlers on Link are responsible for closing mobile menu when a route is clicked on

edison-cy-yang · 2025-02-21T21:53:33Z

packages/site/src/layout/LeftDrawer.tsx

@@ -0,0 +1,37 @@
+import { Box, Button } from "@jobber/components";


LeftDrawer is a simpler version of our SideDrawer, until we can get SideDrawer to slide out from different directions

edison-cy-yang · 2025-02-21T21:55:51Z

packages/site/src/layout/NavMenu.module.css

  min-width: var(--sideBarWidth);
  box-sizing: border-box;
+
+  @media screen and (min-width: 768px) {


--medium-screens-and-up css variable isn't working properly due to missing post-media plugin. There's also a couple other issues with css import that I spotted along the way. Will get another ticket started to deal with it.

edison-cy-yang · 2025-02-21T21:56:51Z

packages/site/src/layout/NavMenu.module.css

@@ -5,10 +5,14 @@
  --navItemHeight: 40px;
  display: flex;
  flex-direction: column;
-  padding: 35px 0 0 0;
-  height: 100dvh;
+  height: 100%;


height of NavMenu in mobile viewport should not be 100dvh since it lives inside the LeftDrawer with header on top

….com:GetJobber/atlantis into JOB-115212-hide-nav-and-add-hamburger-to-collapse-side-nav

ZakaryH · 2025-02-21T22:09:43Z

packages/site/src/layout/LeftDrawer.module.css

+  overflow-y: hidden;
+}
+
+@keyframes slideIn {


I'm thinking we should also do a slide out animation so it feels a bit less abrupt on the way out

that looks & feels much better!

ZakaryH · 2025-02-21T22:17:45Z

packages/site/src/layout/NavMenu.tsx

+      <div className={styles.desktopNavContainer}>{menuContent}</div>
+      <div className={styles.mobileNavContainer}>
+        <LeftDrawer
+          open={isMobileMenuOpen}


I know the LeftDrawer is not a component we're looking to export and re-use much, even still I'm tempted to not have open be a prop on it since the only way it uses open is to return null, and instead let that decision making happen here at the level where we have it in our template

so we'd just do

{isMobileMenuOpen && <LeftDrawer... />

and the LeftDrawer doesn't have to worry about a case where it shouldn't show.

I wouldn't call this a blocker though.

ZakaryH · 2025-02-21T22:46:05Z

packages/site/src/layout/LeftDrawer.tsx

+    <>
+      <div className={styles.overlay} onClick={onClose} />
+      <div className={styles.drawer}>
+        <Box padding="small" direction="row" alignItems="center" gap="small">


if we make this padding="base" the position of the Jobber logo more or less stays in the same place

there's still a 2px difference between the two for some reason.... overall though, it looks nicer.

vs

ZakaryH · 2025-02-21T22:57:30Z

packages/site/src/layout/TopNav.tsx

+      gap="small"
+      justifyContent={mediumAndUp ? "" : "space-between"}
+    >
+      <div className={styles.mobileNavContainer}>


hmm we already have the mediumAndUp out at this level, how about we use!mediumAndUp as the condition to render the mobile nav stuff instead eg. {!mediumAndUp && <Box ...

that way we can get rid of the mobileNavContainer div, and since that's the only style in TopNav.module.css that entire file can go too

then we also have the benefit of our responsive stuff being managed in 1 way rather than split between CSS and JS, plus it's always nice to just not render things we don't want rather than hiding them with CSS when possible

Didn't see this comment before my reply. In this case, it makes sense to only use mediumAndUp and get rid of the css file entirely.

ZakaryH · 2025-02-21T23:01:17Z

packages/site/src/layout/TopNav.tsx

+          width={mediumAndUp ? "grow" : "auto"}
+          padding={mediumAndUp ? { left: "extravagant" } : {}}
+        >
+          <Box width={mediumAndUp ? 200 : "auto"}>


we have a decent amount of ternaries with mediumAndUp

I'm tempted to suggest breaking it into 2 components, and then instead having it be a simpler conditional of which layout to use... but I'm not sure it's really worth it. I wouldn't say the current setup is hard to understand.

what do you think?

overall I wanted to use media query and steer away from this sort of pattern, but also wanted to keep using Box instead of divs. Agree breaking into two components will be nicer.

mkay, if you wanna refactor it that definitely works for me!

again I don't think this is in currently in a state where I find it hard to read or anything, so if sharing 90% of the code with this handful of conditionals is beneficial enough, I don't have any hard objections to leaving it as is.

ZakaryH · 2025-02-21T23:08:52Z

packages/site/src/layout/NavMenu.tsx

-        <Box>
-          <Link to="/">
+        <div className={styles.navMenuHeaderLogo}>
+          <Link to="/" onClick={toggleMobileMenu}>


when does this show up?

I've tried replacing the content with HELLO WORLD and I only ever see it when we're in not-mobile mode, at which point I'd ask if we need it to have an onClick to toggle a menu that doesn't apply to it

ah yeah this used to be the same code for desktop and mobile but not anymore. Good catch

ZakaryH · 2025-02-21T23:35:48Z

packages/site/src/layout/NavMenu.module.css

+  display: none;
+
+  @media screen and (min-width: 768px) {
+    display: block;


do we need to explicitly be setting display: block on divs? they should just do that naturally

then we could reduce our style rules to only be applying the thing we want to change, when we want to apply them

I did this to follow the design guideline to style mobile first, also for when the --medium-screens-and-up css variable is added back.

ZakaryH · 2025-02-21T23:45:58Z

packages/site/src/layout/NavMenu.tsx

+  return (
+    <>
+      <div className={styles.desktopNavContainer}>{menuContent}</div>
+      <div className={styles.mobileNavContainer}>


I'm getting a bit confused with these two classes called mobileNavContainer

what's this one for? if I delete it, as far as I can tell everything still works nicely

in general I'm having not the easiest time following what is showing/when with the combination of CSS display: none and JS mediumAndUp conditionals

This mobileNavContainer is not needed since the visibility is controlled in LeftDrawer, I removed it.

I dont like mixing plain media query with useBreakpoints hook in js either. If we had to pick, is it fine to use div instead of Box in order to get all the responsive logic inside css?

Let me think about the combination of css and JS for a bit.
Overall the pattern I see is

TopNav uses mediumAndUp only

NavMenu uses CSS media query only, except the logic where I dont attach toggleMobileMenu to Link (but this has nothing to do with showing/hiding)

It's not really that convoluted anymore. Thoughts?

definitely improved now that we're using some logic in React to avoid rendering

I think there's a world where we would be able to simply not render anything we don't want to render on a given screen size and it's all managed through one mechanism. however, it's not a huge priority since we're not particularly worried about rendering any of these nav components and the CSS cases that hide them are not too bad to follow.

…nu is set in LeftDrawer

…1-integrate-triton-to-next-gen-docs

….com:GetJobber/atlantis into JOB-115212-hide-nav-and-add-hamburger-to-collapse-side-nav

…2-hide-nav-and-add-hamburger-to-collapse-side-nav

ZakaryH

LGTM

scotttjob and others added 15 commits January 29, 2025 07:43

initial integration

4b1084f

merged with master

79d4323

Merge branch 'master' into scott_t/triton-integration-test

04d144f

Merge branch 'master' of github.com:GetJobber/atlantis into JOB-11016…

05d0f51

…1-integrate-triton-to-next-gen-docs

hide NavMenu below 768

9aa917e

menu button and jobber logo in top nav below 768

441b241

style layout and search button in small viewport

0f7dfb4

shrink search and triton button in small viewport

522f943

simple left drawer for responsive nav menu

a585150

move mobile menu open state into atlantis site context

6a0e476

Layout host separate mobile and desktop nav menu

7261c51

keep responsive menu display inside single NavMenu component

495acc4

close mobile nav menu on navigate

981c75c

one scrollbar

c7c80c4

edison-cy-yang added 2 commits February 19, 2025 15:41

close menu for section header click

374569f

jobber logo in drawer header in mobile viewport:

05c1cfa

edison-cy-yang changed the title ~~Job 115212 hide nav and add hamburger to collapse side nav~~ docs: Hide nav and add hamburger to collapse side nav Feb 20, 2025

edison-cy-yang added 4 commits February 20, 2025 12:23

fixed view in storybook link not showing up in mobile viewport

c6967bd

cleanup

f4f7154

consolidate mobile menu state handler by removing closeMobileMenu

ee51867

only toggleMobileMenu when viewport is medium and up

5478750

edison-cy-yang marked this pull request as ready for review February 21, 2025 17:31

ZakaryH self-requested a review February 21, 2025 19:16

jobbiebot bot added the review requested label Feb 21, 2025

edison-cy-yang commented Feb 21, 2025

View reviewed changes

Merge branch 'JOB-110161-integrate-triton-to-next-gen-docs' of github…

a3e3a2d

….com:GetJobber/atlantis into JOB-115212-hide-nav-and-add-hamburger-to-collapse-side-nav

edison-cy-yang requested a review from a team as a code owner February 21, 2025 22:08

jobbiebot bot added the Team UX Foundations label Feb 21, 2025

ZakaryH reviewed Feb 21, 2025

View reviewed changes

edison-cy-yang added 2 commits February 21, 2025 15:33

remove overlay as the drawer covers the whole viewport

7c30129

one more overlay css

5f55171

ZakaryH reviewed Feb 21, 2025

View reviewed changes

edison-cy-yang added 8 commits February 21, 2025 15:46

padding adjustment

8de864c

remove onClick for Jobber logo that only appears in desktop mode

35a66da

mobileNavContainer class is not needed as the visibility of mobile me…

46c003d

…nu is set in LeftDrawer

get rid of open prop in LeftDrawer

451e98b

dont attach toggleMobileMenu if mobile viewport

0473d6f

use mediumAndUp to display mobile menu button and logo

54b339b

Merge branch 'master' of github.com:GetJobber/atlantis into JOB-11016…

7c97aa2

…1-integrate-triton-to-next-gen-docs

Merge branch 'JOB-110161-integrate-triton-to-next-gen-docs' of github…

4f2d6c3

….com:GetJobber/atlantis into JOB-115212-hide-nav-and-add-hamburger-to-collapse-side-nav

Base automatically changed from JOB-110161-integrate-triton-to-next-gen-docs to master February 24, 2025 18:56

edison-cy-yang added 2 commits February 24, 2025 11:22

Merge branch 'master' of github.com:GetJobber/atlantis into JOB-11521…

8d65d07

…2-hide-nav-and-add-hamburger-to-collapse-side-nav

simple slide out animation for LeftDrawer

cbd9b25

edison-cy-yang requested a review from ZakaryH February 24, 2025 21:28

ZakaryH approved these changes Feb 24, 2025

View reviewed changes

jobbiebot bot added approved and removed review requested labels Feb 24, 2025

edison-cy-yang merged commit a062172 into master Feb 24, 2025
12 checks passed

edison-cy-yang deleted the JOB-115212-hide-nav-and-add-hamburger-to-collapse-side-nav branch February 24, 2025 23:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Hide nav and add hamburger to collapse side nav #2386

docs: Hide nav and add hamburger to collapse side nav #2386

edison-cy-yang commented Feb 19, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Feb 19, 2025 •

edited

Loading

edison-cy-yang Feb 21, 2025

edison-cy-yang Feb 21, 2025

edison-cy-yang Feb 21, 2025

edison-cy-yang Feb 21, 2025

ZakaryH Feb 21, 2025

edison-cy-yang Feb 24, 2025

ZakaryH Feb 24, 2025

ZakaryH Feb 21, 2025

edison-cy-yang Feb 22, 2025

ZakaryH Feb 21, 2025

ZakaryH Feb 21, 2025

edison-cy-yang Feb 22, 2025

ZakaryH Feb 21, 2025

edison-cy-yang Feb 21, 2025

ZakaryH Feb 21, 2025

ZakaryH Feb 21, 2025

edison-cy-yang Feb 21, 2025

ZakaryH Feb 21, 2025

edison-cy-yang Feb 21, 2025

ZakaryH Feb 21, 2025

edison-cy-yang Feb 22, 2025

edison-cy-yang Feb 22, 2025

ZakaryH Feb 24, 2025

ZakaryH left a comment

		@@ -0,0 +1,37 @@
		import { Box, Button } from "@jobber/components";

docs: Hide nav and add hamburger to collapse side nav #2386

docs: Hide nav and add hamburger to collapse side nav #2386

Conversation

edison-cy-yang commented Feb 19, 2025 • edited Loading

Motivations

Changes

Added

Changed

Deprecated

Removed

Fixed

Security

Testing

The breakpoint is 768px. Test the responsiveness in TopNav, as well as NavMenu (which can be opened from the hamburger menu button)

cloudflare-workers-and-pages bot commented Feb 19, 2025 • edited Loading

Deploying atlantis with Cloudflare Pages

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZakaryH left a comment

Choose a reason for hiding this comment

edison-cy-yang commented Feb 19, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Feb 19, 2025 •

edited

Loading