-
Notifications
You must be signed in to change notification settings - Fork 2
Migrated Microsimulation page to app v2 using new components #244
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
SakshiKekre
left a comment
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.
Thanks for your work here @v-lerie. The page looks pretty good!
Since the time this PR was created, there have been several changes in the codebase where we have introduced a new layout for static pages. If you could rebase on top of
the main branch, you should be able to merge those changes and update the page to use the right layout.
31cedc1 to
26ac659
Compare
anth-volk
left a comment
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.
Thanks for this, Valerie! Had a couple changes requested, as well as (unfortunately) a merge conflict in Router we were hoping you could fix.
app/src/pages/Microsim.page.tsx
Outdated
| const { countryId } = useParams<{ countryId: string }>(); | ||
| const isUK = countryId?.toLowerCase() === 'uk'; |
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.
issue: Please use the useCurrentCountry hook for this instead.
app/src/pages/Microsim.page.tsx
Outdated
| </List> | ||
|
|
||
| <Title order={3} mt="xl"> | ||
| Machine Learning Enhancement |
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.
issue: Please use sentence case (i.e., "Machine learning enhancement") here and throughout.
…pp-v2 into feat/141-microsim-page
Fixes #141