Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: ComponentsProvider for Link components #655

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

thomasdigby
Copy link
Member

@thomasdigby thomasdigby commented Apr 29, 2024

Open to comments/suggestions.

This new provider would allow the component library to render React Router or Next.js Link in place of a. We currently use NavigateHandler to mimic the SPA behaviour in atom-core, but there are a few weird edge-cases and bugs that can only really be resolved by using Link components directly. This context layer allows any @atom-learning/components component to access the "Link", and we can still combine that internally with logic for internal/external URLs and rendering a or Link.

I named it ComponentsProvider as I wanted it to be fairly generic and we don't really have a name for our component library. Very happy to rename, tweak the structure, props, etc.

Atom core

import { Link } from 'react-router-dom'
import { ComponentsProvider } from '@atom-learning/components'

const Providers = ({ children }) => (
  <ComponentsProvider Link={({ href, ...props }) => <Link to={href} {...props} />}>
    {/* Other Providers */}
    {children}
  </ComponentsProvider>
)

Marketing site

import RouterLink from 'next/link'
import { ComponentsProvider } from '@atom-learning/components'

const Providers = ({ children }) => (
  <ComponentsProvider Link={RouterLink}>
    {/* Other Providers */}
    {children}
  </ComponentsProvider>
)

@thomasdigby
Copy link
Member Author

@miaambt Tagging you if you have any q's about integration with Next.js router

@andoulla
Copy link
Contributor

Open to comments/suggestions.

This new provider would allow the component library to render React Router or Next.js Link in place of a. We currently use NavigateHandler to mimic the SPA behaviour in atom-core, but there are a few weird edge-cases and bugs that can only really be resolved by using Link components directly. This context layer allows any @atom-learning/components component to access the "Link", and we can still combine that internally with logic for internal/external URLs and rendering a or Link.

I named it ComponentsProvider as I wanted it to be fairly generic and we don't really have a name for our component library. Very happy to rename, tweak the structure, props, etc.

import { Link } from 'react-router-dom'
import { ComponentsProvider } from '@atom-learning/components'

const Providers = ({ children }) => (
  <ComponentsProvider value={{ Link: ({ href, ...props }) => <Link to={href} {...props} /> }}>
    {/* Other Providers */}
    {children}
  </ComponentsProvider>
)

I have a few questions:

  • Will we need to add this component at the app-base level, or next to each link, do we need to use the react-router link? What is the advice there?
  • I'm wondering if this will affect any of the unit-tests in core as it might make mocking difficult (however we could do it in the new customRender that Marty created)

@thomasdigby
Copy link
Member Author

Open to comments/suggestions.
This new provider would allow the component library to render React Router or Next.js Link in place of a. We currently use NavigateHandler to mimic the SPA behaviour in atom-core, but there are a few weird edge-cases and bugs that can only really be resolved by using Link components directly. This context layer allows any @atom-learning/components component to access the "Link", and we can still combine that internally with logic for internal/external URLs and rendering a or Link.
I named it ComponentsProvider as I wanted it to be fairly generic and we don't really have a name for our component library. Very happy to rename, tweak the structure, props, etc.

import { Link } from 'react-router-dom'
import { ComponentsProvider } from '@atom-learning/components'

const Providers = ({ children }) => (
  <ComponentsProvider value={{ Link: ({ href, ...props }) => <Link to={href} {...props} /> }}>
    {/* Other Providers */}
    {children}
  </ComponentsProvider>
)

I have a few questions:

* Will we need to add this component at the app-base level, or next to each link, do we need to use the react-router link? What is the advice there?

* I'm wondering if this will affect any of the unit-tests in core as it might make mocking difficult (however we could do it in the new customRender that Marty created)

It will just be a single provider, which we can just replace in the existing slot that the NavigateHandler currently lives in. Everything else stays the same. You keep using <Link /> and <Button /> from @atom-learning/components.

The unit tests in core should be unaffected as we'll be rendering this in the test-providers.

@thomasdigby thomasdigby marked this pull request as ready for review May 13, 2024 15:22
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.

None yet

2 participants