[Refactor] Establish Pattern and Remove Routing Dependencies

[sandrahoang686]

We need to decouple react-router and navigation dependencies within our veda-ui components. This is because routing is not controlled at the instance level and react-router does not work for nextJs.

Components shouldn't manage any of the routing and should just return the selected filters in which the passed in callback from the instance side should handle the routing.

• should we do it by core feature..?

[hanbyul-here]

From Data Catalog implementation try, we could find the routing problems that need to be handled.

Base finding: react-router-dom component doesn't work with Next instance, and Next has its own Link component.

1. <NavLink> of react-router-dom embedded in source code
   One of the possible solutions is to Pass Link Components from Consumer level

2. Components using react-router for their behaviors (ex. sticky header using useLocation of react-router-dom to detect page transition )
   We should first identify these components and approach them holistically. ex. Do we need this behavior when it is not used in SPA environment?

3. Query parameter manager needing react-router-dom (useQSState hook requires navigate from react-router-dom's useNavigate as a parameter)
   One of the possible solutions is to make this as a prop

4. Make sure the link works with routings defined on the instance level. (ex. does dataset catalog card work with different DATACATALOG_PATH ?)

[sandrahoang686]

Update: This has been kindof been worked on when during breakout components by having the Link Component pass down as a prop. But there are some hacks in there that need to be clean up and we should relook at how we are doing this to see if it is the most optimal and clean...

Going to think about what we should do with this ticket...

[sandrahoang686]

@vgeorge some ideas we discussed beforehand were possibly creating a context provider similar to what was created for environment variables. Gjore's PR here. This would reduce the copious amounts of prop drilling we currently do with LinkProperties. But I think to tackle this more holistically and i'm unsure how much of a huge lift this is or if it should be done iteratively - I was thinking it probably makes more sense to remove all routing and navigation logic out from our components. It would be ideal to extract all this logic out into a callback fn which is then provided to the component. For current architecture, the callback would be passed from our high level container components (example E&A container) and then for refactor, we'd pass these callbacks from the instance which should manage routing.

My two cents on what I've seen best patterns wise but again we probably want to take time and effort into consideration. cc @dzole0311

[vgeorge]

I will have limited availability in the next few weeks and wanted to provide a status update in case someone else has time to tackle the remaining work.

In #1270 and #1276, we reduced the dependency on React Router. I believe the next steps are:

1. Remove usage of the useLocation hook:
   This should be straightforward since most occurrences involve reading location.pathname. These can be replaced with the new custom usePathname hook.

2. Refactor usage of linkProperties:
   I suggest focusing on making the components independent of routing and navigation logic, as proposed by @sandrahoang686. Adding a context provider might complicate usage in Next.js, so I'd avoid it.

   I'd recommend addressing this interactively at the leaf component level to prevent unintended breaks in different parts of the app. For example, the Catalog Card is a good starting point. We could introduce an href prop that, when provided, makes the card act as a link and ignores onClick events. We would still need a Link element to support both Next.js and SPAs. Below is a sample implementation of a CustomLink component:

   import React from 'react';

  // try loading dependencies during build time
   let NextLink: any;
   try {
     NextLink = require('next/link').default;
   } catch (e) {
     NextLink = null;
   }

   let ReactRouterLink: any;
   try {
     ReactRouterLink = require('react-router-dom').Link;
   } catch (e) {
     ReactRouterLink = null;
   }

   interface CustomLinkProps extends React.AnchorHTMLAttributes<HTMLAnchorElement> {
     to?: string;
     href?: string;
     children: React.ReactNode;
   }

   export const CustomLink = ({ to, href, children, ...props }: CustomLinkProps) => {
     const path = to || href;

     if (NextLink) {
       return (
         <NextLink href={path} {...props}>
           {children}
         </NextLink>
       );
     }

     if (ReactRouterLink) {
       return (
         <ReactRouterLink to={path} {...props}>
           {children}
         </ReactRouterLink>
       );
     }

     // Fallback to a regular anchor tag if no routing library is available
     return (
       <a href={path} {...props}>
         {children}
       </a>
     );
   };

[sandrahoang686]

@vgeorge wrong "Sandra" tagged 😄

[sandrahoang686]

Thanks for the input & thoughts @vgeorge 🙇‍♀ . I'm thinking if we are going to tackle the issue with routing coupled up with the components we should probably fully tackle it since this ticket was meant for that. I'm thinking the proposed solution above might still be some sort of bandage as it still worries about "how to navigate" in our veda-ui components when it'd be ideal to totally move away from that. But I agree with starting with the card component to iterate on removing routing entirely from our library. Another thing is, we are using Next in our template instance but ideally we dont want the veda-ui library to care or know - if someone wanted to use our library with Gatsby, they should be able too (I think gatsby works with react-router-dom though but has its own preferred routing mechanism). Thinking that creating some type custom routing controller component that handles based on framework isn't too scalable and is hence more bandagey?? I'll experiment with a first pass at removing routing in one of our components to see how that goes and we could bounce off ideas from that PR :)

PR experimentation here: https://github.com/NASA-IMPACT/veda-ui/pull/1310/files

[sandrahoang686]

This will be done in iterations as we work to remove components in veda-ui from being tightly integrated to routing/nav.
This can close after PR #1310 is merged.

Follow up tickets have been created here...

• Remove LinkProperties from StoriesHub Component Remove LinkProperties from StoriesHub Component #1325
• Remove LinkProperties from DatasetSelectorModal Component Remove LinkProperties from DatasetSelectorModal Component #1326
• Remove LinkProperties from Card Component [PLACEHOLDER] Remove LinkProperties from Card Component [PLACEHOLDER]  #1327

[vgeorge]

@sandrahoang686 while I agree we shouldn't be handling every framework use case, I don't think relying only on event handlers like onDatasetClick is the ideal approach. This will make the components stop working as semantic links, affecting accessibility. One possible approach is to allow the parent component to pass an <a>, a Next.js <Link> or whatever element in a as property, a common approach on many component libraries. Let's follow up on this in #1327.