-
Notifications
You must be signed in to change notification settings - Fork 1
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
Moving component NavReportsList to shared folder and adding new Preferences types #817
Conversation
This pull request is automatically being deployed by Amplify Hosting (learn more). |
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.
Good idea on re-using the navigation menu MultiPageMenu for both Reports and Settings. Looks good from what I can tell.
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 splitting out these changes from the main UI! Can you advise which places to test in the UI to make sure that everything is still working?
src/components/Shared/MultiPageLayout/MultiPageMenu/MultiPageMenu.tsx
Outdated
Show resolved
Hide resolved
src/components/Shared/MultiPageLayout/MultiPageMenu/MultiPageMenuItems.ts
Outdated
Show resolved
Hide resolved
src/components/Shared/MultiPageLayout/MultiPageMenu/Item/Item.tsx
Outdated
Show resolved
Hide resolved
@canac These changes affect the side menu on all the Report pages. Most have a side menu, however, some like |
c6c88ea
to
f823b1f
Compare
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.
Looks good and seems to work locally!
I have merged this into staging locally and fixed the merge conflicts |
Description
We have done a lot of work on Preferences. To prevent a 100-plus page PR I'm pushing certain changes into
main
which doesn't affect the user.These changes are to the
NavReportsList
Component. I've changed how Reports use this repo by moving it's location to the Shared folder and prepping it with Preferences TS types.Changes
NavReportsList
components to the Shared folderNavReportsList
.Checklist: