Skip to content

Conversation

@jonkafton
Copy link
Contributor

@jonkafton jonkafton commented Oct 24, 2024

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/5853.

Description (What does it do?)

Replaces any remaining plain links to internal routes with Next.js <Link> components.

How can this be tested?

  • Check the nav drawer links. These should navigate without reloading the page.

    • Search links (navigating between items in “Learn” and “Discover Learning Resources”) should update the search filters without reload.
    • The nav drawer should close on selection.
  • The header search icon should navigate without reload.

  • Footer links should navigate without reload ("Accessibility" and "Contact Us" are external, but still navigate ok).

Additional Context

The Channel detail links to external sites and login links in the header, user menu and Personalize section are the only remaining plain <a> tags. The login links use <ButtonLink> with rawAnchor={true}, which we perhaps don't need as Next.js <Link> works fine for external URLs and target="_blank" (e.g. the MIT logo link in the header).

@jonkafton jonkafton added the Needs Review An open Pull Request that is ready for review label Oct 24, 2024
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki left a comment

Choose a reason for hiding this comment

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

👍 with the one comment

icon?: string | ReactElement
description?: string
href: string
onClick?: () => void
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be used? Let's remove it to avoid expanding the interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's newly used (L191) to pass the drawer close when the link is clicked.

@jonkafton jonkafton merged commit dad6b2f into main Oct 25, 2024
11 checks passed
@jonkafton jonkafton removed the Needs Review An open Pull Request that is ready for review label Oct 31, 2024
@rhysyngsun rhysyngsun deleted the jk/5853-nextjs-links branch February 7, 2025 20:39
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