Skip to content

Conversation

@maflaven
Copy link
Contributor

Hey yall, hope everyone's doing well!

I noticed nav links for sections on the main page will open another window instead of just scrolling to the matching section. Also clicking related resources menu link triggers a scroll to the top of the page (in addition to opening the menu). Added some changes to prevent both of these.

…d resources menu link from causing scroll to top of page
@maflaven maflaven requested review from Copilot and graft August 24, 2025 01:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes navigation link behavior issues where section links were opening in new windows instead of scrolling to sections, and the related resources menu link was causing unwanted scrolling to the top of the page.

  • Updated link click logic to check for valid scroll targets before opening external links
  • Added preventDefault() to menu click handler to stop default anchor behavior
  • Reorganized conditional flow for better readability and correct functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +47 to +51
router.push(href, { scroll: false })
window.scrollTo({
top: el.offsetTop - NavBarHeights[breakpoint].condensed,
behavior: 'smooth',
})
Copy link

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The router.push() call will always execute when an element is found, even for external URLs that contain hash fragments. This could cause navigation issues if href contains an external URL with a hash (e.g., 'https://example.com#section'). Consider checking if the href is an internal route before calling router.push().

Suggested change
router.push(href, { scroll: false })
window.scrollTo({
top: el.offsetTop - NavBarHeights[breakpoint].condensed,
behavior: 'smooth',
})
if (isInternalUrl(href)) {
router.push(href, { scroll: false })
window.scrollTo({
top: el.offsetTop - NavBarHeights[breakpoint].condensed,
behavior: 'smooth',
})
} else {
// For external URLs, just open in a new tab
window.open(href, '_blank');
return;
}

Copilot uses AI. Check for mistakes.
@graft
Copy link
Contributor

graft commented Aug 26, 2025

Thanks @maflaven! this appears to be a bug i introduced recently. I noticed this the other day as I have finally started working on the multi-page site & project explorer. Stay tuned for more exciting new bugs!

@maflaven
Copy link
Contributor Author

Oh sweet! Looking forward to it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants