Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

Create simpler Link component and use it throughout the codebase #30

Closed
bretthayes opened this issue Mar 10, 2022 · 2 comments
Closed
Assignees
Labels
team/content-platform Content Platform Team related tickets.

Comments

@bretthayes
Copy link
Contributor

bretthayes commented Mar 10, 2022

  1. Create our own a Link/A component that simplifies and inherits props for Next.JS' Link component.
  2. Replace all instances of Next.JS's Link element where it uses internal navigation

See these discussions for context on why: this, this, and this

@bretthayes bretthayes moved this to Backlog in Content Platform Work Mar 10, 2022
@bretthayes bretthayes moved this from Backlog to Ready for Development in Content Platform Work Mar 10, 2022
@bretthayes bretthayes added the team/content-platform Content Platform Team related tickets. label Mar 22, 2022
@bretthayes bretthayes self-assigned this Mar 22, 2022
@bretthayes bretthayes added this to the Phase 1 milestone Mar 22, 2022
@bretthayes bretthayes moved this from Ready for Development to In Progress in Content Platform Work Mar 23, 2022
@bretthayes bretthayes moved this from In Progress to Ready for Development in Content Platform Work Mar 23, 2022
@bretthayes bretthayes moved this from Ready for Development to In Progress in Content Platform Work Mar 24, 2022
@bretthayes
Copy link
Contributor Author

Noting this here in case this code below is helpful for anyone, but I'm going to close this ticket in lieu of us using the default Next Link element. I've thought about this a bit, and if we introduce our own API and Next.js decides to improve theirs, I'd like to keep our codebase flexible in case that happens. Also, instead of writing out our own API interface for props for a custom Link element, it seems more natural to include HTML properties on the child <a> for Next's Link element if we need to use those properties for edge cases.

import { FunctionComponent } from 'react'

import {default as NextLink, LinkProps} from 'next/link'

interface Props extends Omit<LinkProps, 'passHref'> {
    className?: string
}

export const Link: FunctionComponent<Props> = ({ href, className, children, ...props }) => {

    let defaultChildren = children

    if (className) {
        defaultChildren = (
            <a href="#ignored" className={className}>
                {children}
            </a>
        )
    }

    return (
        <NextLink href={href} passHref={!!className} {...props}>
            {defaultChildren}
        </NextLink>
    )
}

Repository owner moved this from In Progress to Done in Content Platform Work Mar 25, 2022
@bretthayes bretthayes moved this from Done to Not Doing/Cancelled in Content Platform Work Mar 25, 2022
@bretthayes bretthayes reopened this Apr 8, 2022
Repository owner moved this from Not Doing/Cancelled to Ready for Development in Content Platform Work Apr 8, 2022
@bretthayes bretthayes removed this from the AR - Sprint 0 milestone Apr 11, 2022
@bretthayes
Copy link
Contributor Author

Closing this since there is now a PR in vercel/next.js#36436 that changes the behaviour of Next JS's Link component. We'll be able to upgrade in the future to adopt this implementation and update our Link components across the codebase.

cc: @katjuell @st0nebraker @zlonko

Repository owner moved this from Ready for Development 🚦 to Done ✅ in Content Platform Work Apr 25, 2022
@bretthayes bretthayes moved this from Done ✅ to Not Doing/Cancelled 🗑 in Content Platform Work Apr 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
team/content-platform Content Platform Team related tickets.
Projects
No open projects
Status: Not Doing/Cancelled 🗑
Development

No branches or pull requests

1 participant