-
Notifications
You must be signed in to change notification settings - Fork 23
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
PORTALS-3195 - Migrate AD Knowledge Portal to react-router v6 #1285
base: PORTALS-3195
Are you sure you want to change the base?
PORTALS-3195 - Migrate AD Knowledge Portal to react-router v6 #1285
Conversation
@@ -1,7 +1,7 @@ | |||
// Load variable_overrides first, so correct values for variables will be applied to the rest of the styles | |||
@use './config/style/variable_overrides'; | |||
|
|||
@use '@sage-bionetworks/synapse-portal-framework/src/style/App'; | |||
@use '@sage-bionetworks/synapse-portal-framework/style/App'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Vite config was updated to improve hot-module reloading & imports from the synapse-portal-framework package, so SCSS imports had to update to accomodate this.
@@ -0,0 +1,51 @@ | |||
import { NavbarConfig } from '@sage-bionetworks/synapse-portal-framework/components/navbar/Navbar' | |||
|
|||
export const navbarConfig: NavbarConfig = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each portal will have this new config file
<SurveyToast | ||
localStorageKey="org.sagebionetworks.security.cookies.portal.adkpsurvey.dismissed" | ||
title="What Metrics Matter to You? Help Us Improve the AD Knowledge Portal!" | ||
description="Take our quick survey and share your feedback to make the portal even better. Your input will directly impact the data and insights we provide." | ||
surveyURL="https://docs.google.com/forms/d/e/1FAIpQLScpWL2N3LGQlNcqKRXQ-qST-UPKngutNkvbhPVkozD7cQR8-g/viewform" | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Global' components can be added as children of App
matchColumnName: COMPUTATIONAL_TOOLS_COLUMN_NAMES.GRANT, | ||
URLColumnName: PROGRAM_TABLE_COLUMN_NAMES.GRANT_NUMBER, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't manage to do this for all portals & all components but it would save a lot of confusion and risk for error if we added constants for the column names used throughout the config, especially when cross-linking between relations. I started doing that here.
RssFeedCards, | ||
} from 'synapse-react-client' | ||
|
||
export default function HomePage() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complex home pages and details pages are created as components
|
||
export default function ProjectDetailsPage() { | ||
const searchParams = useGetPortalComponentSearchParams() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I can envision adding logic to define properties to hoist into the head
that would be meaningful for SEO.
This would require some thought on each DetailsPage. For example, ADKP projects here can be queried by either the project name, or the grant number. Only one of those would define the 'canonical URL'.
), | ||
children: [ | ||
...sharedRoutes, | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remainder is mostly just re-writing the old config into the new style.
{ | ||
path: 'Explore', | ||
element: <ExploreWrapper explorePaths={explorePageRoutes} />, | ||
children: [ | ||
...explorePageRoutes, | ||
{ | ||
// PORTALS-2001 - we renamed "Experimental Tools" to "Experimental Models" | ||
path: 'Experimental Tools', | ||
element: <RedirectWithQuery to={'/Explore/Experimental Models'} />, | ||
}, | ||
], | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Routes for Explore are mostly captured in the explorePageRoutes object
{ | ||
path: 'Explore/Projects/DetailsPage', | ||
element: <ProjectDetailsPage />, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each DetailsPage gets its own route
{ | ||
path: 'Explore/Studies/DetailsPage', | ||
element: <StudyDetailsPage />, | ||
children: studyDetailsPageChildRoutes, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DetailsPages with tabs also have child routes
|
||
export const studiesDetailsPageProps: DetailsPageProps = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DetailsPage configs were moved to the new page (in this case StudyDetailsPage.tsx)
No description provided.