Remove "Home" item / tidy up navigation overlay - #246
Merged
Conversation
The expandable navigation menu grows too tall on desktop. Removing the redundant "Home" entry from the main menu shortens it. The homepage stays reachable via the logo in the top-left, and the / route is unchanged.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The spacing-class change shortened a div's class string so it now fits on one line; run Prettier to collapse the multi-line attribute and satisfy the prettier:check CI step.
…scroll when menu open
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Nuxt 3 Header.vue navigation overlay to reduce menu height and remove the redundant homepage entry from the main navigation.
Changes:
- Removed the
{ label: 'Home', href: '/' }entry frommainMenuItems. - Adjusted navigation overlay spacing/layout classes to shorten the expanded menu.
- Added body scroll-locking while the menu is open (with cleanup on unmount).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
84
to
+88
| <div | ||
| class="flex min-h-screen flex-col space-y-16 px-6 pb-8 pt-28 lg:pl-8 lg:pr-24 lg:pt-36 xl:pr-32 2xl:pr-48" | ||
| class="flex min-h-screen flex-col space-y-8 px-6 pb-8 pt-28 lg:pl-8 lg:pr-24 lg:pt-32 xl:pr-32 2xl:pr-48" | ||
| > | ||
| <!-- Main menu --> | ||
| <ul class="flex flex-col space-y-4 lg:flex-grow lg:items-end lg:space-y-0"> | ||
| <ul class="flex flex-col space-y-4 lg:items-end lg:space-y-0"> |
Contributor
Author
There was a problem hiding this comment.
The PR description and title have been updated to also cover the tightened menu spacing, the desktop legal/social footer alignment, and the background scroll-lock changes.
Generated by Claude Code
Comment on lines
191
to
+206
|
|
||
| // Lock background scroll while the menu is open so the underlying | ||
| // page can't move behind the overlay, and release it when it closes | ||
| watch(menuIsOpen, () => { | ||
| if (bodyElement.value) { | ||
| bodyElement.value.style.overflow = menuIsOpen.value ? 'hidden' : '' | ||
| } | ||
| }) | ||
|
|
||
| // Make sure the scroll lock is released if the component is unmounted | ||
| // while the menu is still open, so the page can't stay locked | ||
| onBeforeUnmount(() => { | ||
| if (bodyElement.value) { | ||
| bodyElement.value.style.overflow = '' | ||
| } | ||
| }) |
Contributor
Author
There was a problem hiding this comment.
The scroll-lock now saves the body's current inline overflow value when the menu opens and restores that saved value on close and on unmount, so a pre-existing overflow is no longer clobbered.
Generated by Claude Code
claude
Bot
changed the title
Save the body's current inline overflow value when locking scroll on menu open and restore it on close and unmount, instead of hard-resetting to an empty string, so a pre-existing inline overflow is not clobbered.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
nuxt-app/components/Header.vue:210
- There are now two separate
watch(menuIsOpen, ...)subscriptions. This duplicates reactive work and makes it easier for the behaviors to drift (or subtly depend on execution order). Consider consolidating into a single watcher that both tracks analytics and manages scroll lock, keeping the open/close logic in one place.
// Track analytic menu events
watch(menuIsOpen, () => {
if (menuIsOpen.value) {
trackGoal(OPEN_MENU_EVENT_ID)
} else {
trackGoal(CLOSE_MENU_EVENT_ID)
}
})
// Lock background scroll while the menu is open so the underlying
// page can't move behind the overlay, and release it when it closes
watch(menuIsOpen, () => {
if (bodyElement.value) {
if (menuIsOpen.value) {
// Save the current inline overflow before locking so a value set
// elsewhere isn't lost, then lock the scroll
previousBodyOverflow.value = bodyElement.value.style.overflow
bodyElement.value.style.overflow = 'hidden'
} else {
// Restore the previously saved overflow value on close
bodyElement.value.style.overflow = previousBodyOverflow.value
}
}
})
nuxt-app/components/Header.vue:204
- Setting
body.style.overflow = 'hidden'can cause a visible layout shift on pages with scrollbars (scrollbar disappears, content width changes). If this is user-visible, consider compensating for scrollbar width (e.g., addpadding-rightwhile locked) or using a scroll-lock approach that avoids layout shift.
previousBodyOverflow.value = bodyElement.value.style.overflow
bodyElement.value.style.overflow = 'hidden'
Comment on lines
180
to
+218
| @@ -189,6 +193,30 @@ watch(menuIsOpen, () => { | |||
| } | |||
| }) | |||
|
|
|||
| // Lock background scroll while the menu is open so the underlying | |||
| // page can't move behind the overlay, and release it when it closes | |||
| watch(menuIsOpen, () => { | |||
| if (bodyElement.value) { | |||
| if (menuIsOpen.value) { | |||
| // Save the current inline overflow before locking so a value set | |||
| // elsewhere isn't lost, then lock the scroll | |||
| previousBodyOverflow.value = bodyElement.value.style.overflow | |||
| bodyElement.value.style.overflow = 'hidden' | |||
| } else { | |||
| // Restore the previously saved overflow value on close | |||
| bodyElement.value.style.overflow = previousBodyOverflow.value | |||
| } | |||
| } | |||
| }) | |||
|
|
|||
| // Make sure the scroll lock is released if the component is unmounted | |||
| // while the menu is still open, so the page can't stay locked | |||
| onBeforeUnmount(() => { | |||
| if (bodyElement.value) { | |||
| bodyElement.value.style.overflow = previousBodyOverflow.value | |||
| } | |||
| }) | |||
Contributor
Author
There was a problem hiding this comment.
Good catch. The onBeforeUnmount restore is now guarded with menuIsOpen.value, so it only runs when the scroll lock is actually held and won't touch a pre-existing overflow we never captured.
Generated by Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Suppressed comments (3)
nuxt-app/components/Header.vue:210
- Restoring
bodyElement.value.style.overflowto a single saved value can interfere with other scroll-lock mechanisms that may run while the menu is open (e.g., another modal opens and also locks scroll). When the menu closes, it will restore the pre-menu overflow and potentially unlock scroll that another component intended to keep locked. Consider using a shared scroll-lock utility with reference counting, or mark ownership (e.g., via a data attribute/class token) and only restore if the lock is still owned by this component.
watch(menuIsOpen, () => {
if (bodyElement.value) {
if (menuIsOpen.value) {
// Save the current inline overflow before locking so a value set
// elsewhere isn't lost, then lock the scroll
previousBodyOverflow.value = bodyElement.value.style.overflow
bodyElement.value.style.overflow = 'hidden'
} else {
// Restore the previously saved overflow value on close
bodyElement.value.style.overflow = previousBodyOverflow.value
}
}
})
nuxt-app/components/Header.vue:194
- There are now two separate
watch(menuIsOpen, ...)blocks. This works, but it spreads menu-open/close side effects across multiple watchers, making it easier to miss interactions (e.g., ordering/flush behavior). Consider consolidating into a single watcher that handles both analytics and scroll locking (or extracting scroll-lock into a dedicated composable likeuseScrollLockWhile(menuIsOpen)), so all menu open/close effects live together.
watch(menuIsOpen, () => {
if (menuIsOpen.value) {
trackGoal(OPEN_MENU_EVENT_ID)
} else {
trackGoal(CLOSE_MENU_EVENT_ID)
}
})
nuxt-app/components/Header.vue:210
- There are now two separate
watch(menuIsOpen, ...)blocks. This works, but it spreads menu-open/close side effects across multiple watchers, making it easier to miss interactions (e.g., ordering/flush behavior). Consider consolidating into a single watcher that handles both analytics and scroll locking (or extracting scroll-lock into a dedicated composable likeuseScrollLockWhile(menuIsOpen)), so all menu open/close effects live together.
watch(menuIsOpen, () => {
if (bodyElement.value) {
if (menuIsOpen.value) {
// Save the current inline overflow before locking so a value set
// elsewhere isn't lost, then lock the scroll
previousBodyOverflow.value = bodyElement.value.style.overflow
bodyElement.value.style.overflow = 'hidden'
} else {
// Restore the previously saved overflow value on close
bodyElement.value.style.overflow = previousBodyOverflow.value
}
}
})
Comment on lines
+198
to
+210
| watch(menuIsOpen, () => { | ||
| if (bodyElement.value) { | ||
| if (menuIsOpen.value) { | ||
| // Save the current inline overflow before locking so a value set | ||
| // elsewhere isn't lost, then lock the scroll | ||
| previousBodyOverflow.value = bodyElement.value.style.overflow | ||
| bodyElement.value.style.overflow = 'hidden' | ||
| } else { | ||
| // Restore the previously saved overflow value on close | ||
| bodyElement.value.style.overflow = previousBodyOverflow.value | ||
| } | ||
| } | ||
| }) |
Comment on lines
+216
to
+220
| onBeforeUnmount(() => { | ||
| if (bodyElement.value && menuIsOpen.value) { | ||
| bodyElement.value.style.overflow = previousBodyOverflow.value | ||
| } | ||
| }) |
Contributor
Author
There was a problem hiding this comment.
Fixed: the lock is now tracked with a dedicated scrollLocked flag that's set synchronously in the same callback that mutates overflow, and both the close-restore and the onBeforeUnmount restore are driven off that flag instead of menuIsOpen. So unmount always releases the lock regardless of the async watcher timing.
Generated by Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (2)
nuxt-app/components/Header.vue:216
- There are two separate
watch(menuIsOpen, ...)blocks. This duplicates reactive subscriptions and can make future edits error-prone (e.g., one watch updated but not the other). Consider consolidating into a singlewatch(menuIsOpen, (isOpen) => { ... })that both tracks analytics and applies/releases the scroll lock, using theisOpenparameter instead of re-readingmenuIsOpen.value.
// Track analytic menu events
watch(menuIsOpen, () => {
if (menuIsOpen.value) {
trackGoal(OPEN_MENU_EVENT_ID)
} else {
trackGoal(CLOSE_MENU_EVENT_ID)
}
})
// Lock background scroll while the menu is open so the underlying
// page can't move behind the overlay, and release it when it closes
watch(menuIsOpen, () => {
if (bodyElement.value) {
if (menuIsOpen.value) {
// Save the current inline overflow before locking so a value set
// elsewhere isn't lost, then lock the scroll
previousBodyOverflow.value = bodyElement.value.style.overflow
bodyElement.value.style.overflow = 'hidden'
scrollLocked.value = true
} else if (scrollLocked.value) {
// Restore the previously saved overflow value on close
bodyElement.value.style.overflow = previousBodyOverflow.value
scrollLocked.value = false
}
}
})
nuxt-app/components/Header.vue:215
- Restoring
body.style.overflowfrom a single saved value can conflict with other concurrent scroll locks (e.g., another modal/component also settingoverflow: hidden). If another lock is applied while the menu is open, closing the menu will restore the old value and may unintentionally re-enable scrolling. A more robust approach is to centralize scroll locking in a shared composable (e.g., reference-counted locks or a single source of truth that only removes the lock when all lockers have released).
if (bodyElement.value) {
if (menuIsOpen.value) {
// Save the current inline overflow before locking so a value set
// elsewhere isn't lost, then lock the scroll
previousBodyOverflow.value = bodyElement.value.style.overflow
bodyElement.value.style.overflow = 'hidden'
scrollLocked.value = true
} else if (scrollLocked.value) {
// Restore the previously saved overflow value on close
bodyElement.value.style.overflow = previousBodyOverflow.value
scrollLocked.value = false
}
}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (4)
nuxt-app/components/Header.vue:202
- There are now two separate
watch(menuIsOpen, ...)blocks. Since they respond to the same state change, consider combining them into a single watcher (analytics + scroll lock) to reduce duplicated reactive subscriptions and keep the open/close behavior easier to reason about in one place. If you prefer separation, using a named function for each handler would still help readability and future edits.
// Track analytic menu events
watch(menuIsOpen, () => {
if (menuIsOpen.value) {
trackGoal(OPEN_MENU_EVENT_ID)
} else {
trackGoal(CLOSE_MENU_EVENT_ID)
}
})
nuxt-app/components/Header.vue:231
- There are now two separate
watch(menuIsOpen, ...)blocks. Since they respond to the same state change, consider combining them into a single watcher (analytics + scroll lock) to reduce duplicated reactive subscriptions and keep the open/close behavior easier to reason about in one place. If you prefer separation, using a named function for each handler would still help readability and future edits.
watch(menuIsOpen, () => {
if (bodyElement.value) {
if (menuIsOpen.value) {
// Measure the scrollbar width before hiding overflow (afterwards
// the scrollbar is gone and the measurement would be 0)
const scrollbarWidth = window.innerWidth - document.documentElement.clientWidth
// Save the current inline overflow and padding-right before locking
// so values set elsewhere aren't lost, then lock the scroll
previousBodyOverflow.value = bodyElement.value.style.overflow
previousBodyPaddingRight.value = bodyElement.value.style.paddingRight
bodyElement.value.style.overflow = 'hidden'
// Compensate the hidden scrollbar with padding-right so the fixed
// header doesn't shift when the scrollbar disappears
if (scrollbarWidth > 0) {
const currentPaddingRight = parseFloat(getComputedStyle(bodyElement.value).paddingRight) || 0
bodyElement.value.style.paddingRight = `${currentPaddingRight + scrollbarWidth}px`
}
scrollLocked.value = true
} else if (scrollLocked.value) {
// Restore the previously saved overflow and padding-right on close
bodyElement.value.style.overflow = previousBodyOverflow.value
bodyElement.value.style.paddingRight = previousBodyPaddingRight.value
scrollLocked.value = false
}
}
})
nuxt-app/components/Header.vue:211
- This uses
window/documentdirectly inside a component watcher. In Nuxt (SSR), it’s safer to guard DOM access (e.g.,if (import.meta.client)/process.client) or consistently use the existing DOM composables you already import (likeuseDocument) to avoid runtime errors if this code ever executes in a non-browser context.
const scrollbarWidth = window.innerWidth - document.documentElement.clientWidth
nuxt-app/components/Header.vue:193
- The comment claims the lock state is set 'synchronously' and is consistent 'regardless of watcher timing', but Vue
watchcallbacks are scheduled (by default) and can be batched. Either adjust the wording to match the actual behavior, or make the behavior truly synchronous by setting the watcher to flush synchronously (and/or restructuring to use a cleanup-based pattern) so the comment remains accurate.
// Track whether the scroll lock is actually applied, set synchronously with
// the overflow change so it stays consistent regardless of watcher timing
const scrollLocked = ref(false)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Jan Gregor Emge-Triebel · Slack thread
What & why
The expandable navigation overlay grew too tall on desktop and had some layout rough edges. This branch removes the redundant "Home" entry, tightens the overlay's spacing, cleans up the footer alignment, and locks background scrolling while the menu is open.
Changes
1. Remove "Home" from the main navigation
/route is completely unchanged.2. Tighten the expandable-menu spacing
3. Fix footer alignment on desktop
4. Lock background scroll while the menu is open
overflowvalue on close and on unmount, so it never clobbers a value set elsewhere.How
All changes are contained in
nuxt-app/components/Header.vue, which renders the one responsive expandable navigation used for both desktop and mobile. The{ label: 'Home', href: '/' }entry was removed frommainMenuItems; overlay spacing and footer alignment were adjusted via template/class tweaks; and awatchonmenuIsOpen(plusonBeforeUnmount) togglesdocument.body.style.overflowthrough the SSR-safeuseBodyElement()ref, capturing the previous value on lock and restoring it on unlock. No route or page was touched —nuxt-app/pages/index.vue(the/homepage) and the logo link (to="/") are untouched.Verification
eslint components/Header.vue— 0 errors (one pre-existing, unrelated warning).vue-tsc --noEmit+node scripts/typecheck-ratchet.mjs— "Typecheck steady at 263 errors (baseline 263)", i.e. no new type errors introduced.prettier components/Header.vue --check— clean.Generated by Claude Code