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.

Sign up for GitHub

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

Jump to bottom

[Link] add react-router NavLink integration #293

Open
billhimmelsbach opened this issue Jan 30, 2024 · 0 comments
Open

[Link] add react-router NavLink integration #293

billhimmelsbach opened this issue Jan 30, 2024 · 0 comments
Assignees
@billhimmelsbach
Labels
refactor

Comments

@billhimmelsbach
Copy link
Contributor

After # is merged, we'll have the ability to use react-router links using our Link component, but there are probably some good use cases coming up to use NavLinks too.

Current behavior

  • Link only supports react-router Links

Expected behavior

  • Link supports react-router Links and NavLinks

Notes
@billhimmelsbach billhimmelsbach self-assigned this Jan 30, 2024
@billhimmelsbach billhimmelsbach mentioned this issue Jan 30, 2024
Merged
billhimmelsbach added a commit that referenced this issue Jan 30, 2024
@billhimmelsbach
feat(Link): add react-router link integration (#294)
d3888b7 
Well this took some experimentation, but we got there. Allows `Link` to
use a `isRouterLink` prop to convert them to `react-router` links with
minimal hassle.

## Changes

- add `react-router-dom` as an external dependency in the vite config,
since `react-router` needs its links to share the same instance of the
library to work. Works in concert [with this
change](https://github.com/cfpb/sbl-frontend/compare/198-integrate-react-router-links?expand=1#diff-6a3b01ba97829c9566ef2d8dc466ffcffb4bdac08706d3d6319e42e0aa6890ddR26-R28)
in `sbl-frontend`. What a pain this was. (Also, I added a maybe missing
`react-dom` from the `optimizeDeps.exclude` array for good measure)
- add a `isRouterLink` prop that switches `Link` to using `react-router`
Links
- fixes some Typescript errors
- adds a story for `isRouterLink` prop called "Link using React Router
Link"

## How to test this PR

1. Does the `isRouterLink` story "Link using React Router Link" render
without errors?

## Screenshots
![Screenshot 2024-01-29 at 11 15 16
PM](https://github.com/cfpb/design-system-react/assets/19983248/a3c9e483-945c-423c-9efa-1bf08bd8b5cd)

## Notes

- I could have also added a `NavLink` option, but I don't think we have
a use case for it since we have some similar logic built out already. I
made [a ticket for
it](#293), in case we
want to add it in.
- Enables this PR on sbl-frontend:
cfpb/sbl-frontend#201
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@billhimmelsbach billhimmelsbach

Labels
refactor
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@billhimmelsbach