jpmorganchase / salt-ds

React UI components built with a focus on accessibility, customisation and ease-of-use
https://www.saltdesignsystem.com
Apache License 2.0
109 stars 78 forks source link

Allow passing more props to NavigationItem's link element #3279

Open manelmadeira opened 2 months ago

manelmadeira commented 2 months ago

Area

UI Components

The problem

We are using @tanstack/react-router to manage our app's routing and we want to be able to use Salt's NavigationItem with their Link attributes.

They export a hook useLinkProps (docs) that allows us to send all the navigation related props to a custom anchor tag.

We tried using it with NavigationItem like this

const linkProps = useLinkProps({ to: '/apps/$appSlug', params: { appSlug: 'my-app' } });

return (
  <NavigationItem href={linkProps.href}>My Link</NavigationItem>
)

but that's not enough for @tanstack/react-router to navigate to the new route. After looking into their hook's source code, I realised we need to pass more than just href to the anchor tag for routing to work.

The solution

Is it possible to add an additional prop to NavigationItem to allow this (eg. linkProps)?

This would allow to do something like:

const linkProps = useLinkProps({ to: '/apps/$appSlug', params: { appSlug: 'my-app' } });

return (
  <NavigationItem linkProps={linkProps}>My Link</NavigationItem>
)

Alternatives and examples

Alternative we could allow passing a ReactElement to replace the anchor element rendered by NavigationItem and add styling to it.

Chakra UI allows to pass other elements to be rendered instead using an as property. https://v2.chakra-ui.com/community/recipes/as-prop

Are you a JPMorgan Chase & Co. employee?

mark-tate commented 2 months ago

We have similar issues with Next JS apps and next/link. We are going to provisionally pull this into our next sprint, at least for discussion and decide on the final approach.

manelmadeira commented 2 months ago

We have similar issues with Next JS apps and next/link. We are going to provisionally pull this into our next sprint, at least for discussion and decide on the final approach.

I'd imagine that the approach used for this would be the same for the Lab's BreadcrumbItem component, right?