-
Notifications
You must be signed in to change notification settings - Fork 375
refactor: [M3-9723] - Managed Tanstack routing #11994
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
Conversation
a5e3ca0
to
a9cf99d
Compare
}); | ||
|
||
return groups; | ||
}; |
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.
just moved that util but it is unchanged
enabled, | ||
refetchInterval: 20000, | ||
}); | ||
|
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 queries added here were apparently never needed cause we were fetching ALL then filtering to get a single record. As with other routing refactors and our new standards, we now fetch an individual record when performing an action in a drawer, modal etc
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.
Nice! Thank you @abailly-akamai!
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 the patience on this one and sorry it took me longer than I liked to get to. And as always, thanks for your work on this + your thoroughness in doing them.
Confirmed:
- Routes are routing as expected for create, edit, and delete actions
- Successful CRUD operations for SSH Access, Contacts, and Credentials
- Didn't test Monitor CRUD operations since we don't have support for that in the MSW, but did confirm the routes update as expected when interacting with the actions
- 🐛 Noted the
undefined
label in the header of the HistoryDrawer
- Sorting and filtering tables works for all tables
- Backwards and forwards browser navigation
- Visual parity with prod
<MaskableText isToggleable text={contact.name} /> | ||
</TableCell> | ||
<Hidden mdDown> | ||
<TableCell> | ||
<MaskableText text={contact.group ?? ''} isToggleable /> | ||
<MaskableText isToggleable text={contact.group ?? ''} /> | ||
</TableCell> | ||
</Hidden> | ||
<TableCell> | ||
<MaskableText text={contact.email} isToggleable /> | ||
<MaskableText isToggleable text={contact.email} /> | ||
</TableCell> | ||
<Hidden xsDown> | ||
<TableCell> | ||
<MaskableText text={contact.phone.primary ?? ''} isToggleable /> | ||
<MaskableText isToggleable text={contact.phone.primary ?? ''} /> | ||
</TableCell> | ||
<TableCell> | ||
<MaskableText text={contact.phone.secondary ?? ''} isToggleable /> | ||
<MaskableText isToggleable text={contact.phone.secondary ?? ''} /> |
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 the clean up. 🧹
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.
Minor: some linter warnings in this file
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.
Very nice work. It took me longer than anticipated to approve because I taking the time to understand the changes. Verified all actions within modals and drawers. No regressions with routing.
Cloud Manager UI test results🎉 559 passing tests on test run #16 ↗︎
|
* Initial commit: landing routing * monitors * remove barrel files * save progress * Wrap up monitors * SSH Access * Credentials * Wrapping up UI * fix units * e2e fixes * fix after rebase * fix remainin e2e failure * eslint fix * eslint fix * linting fixes * Added changeset: Managed Tanstack routing * feedback @mjac0bs
Description 📝
This PR migrates our routing tooling in the Managed feature to Tanstack router.
It also cleans up the code to adhere to our latest standards.
Changes 🔄
Preview 📷
There should not be any visual or functional regression with the feature. Only difference is the introduction of loading patterns in some edit/delete drawer/modals
How to test 🧪
Verification steps
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅