-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(crons/uptime): Move to insights top-level nav #92459
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
feat(crons/uptime): Move to insights top-level nav #92459
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #92459 +/- ##
===========================================
+ Coverage 46.25% 87.88% +41.63%
===========================================
Files 10216 10232 +16
Lines 585498 586360 +862
Branches 22785 22783 -2
===========================================
+ Hits 270818 515350 +244532
+ Misses 314250 70580 -243670
Partials 430 430 |
@@ -1728,11 +1728,6 @@ function buildRoutes() { | |||
)} | |||
/> | |||
</Route> | |||
<Route path={`${MODULE_BASE_URLS[ModuleName.UPTIME]}/`}> |
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 don't think you'll be able to remove these routes completely until the new nav is fully GA'd. You'll need to make sure it still works in the old nav (you can test by toggling it on/off in the help menu).
You can use usePrefersStackedNav()
to check which whether or not the new nav is active
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.
ah right 😢
12f0930
to
51e0038
Compare
51e0038
to
3dc4c88
Compare
3dc4c88
to
c4d135a
Compare
c4d135a
to
1918b01
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! But I think there are some missing links that need to be converted going to /insights/backend/crons/
, like this one in alerts:
<LinkButton size="xs" icon={<IconOpen />} to="/insights/backend/crons/"> |
1918b01
to
b3b6020
Compare
Thanks for catching those links. I think this should be everything now 👍 I had to remove a guide anchor that was hiding a button that an acceptance test was clicking because of the fact that the tabs were removed |
b3b6020
to
a23b407
Compare
Before this change both Crons and Uptime were hidden under Backend and Frontend inisghts. These are now directly available in the insights secondary nav <img alt="clipboard.png" width="301" src="https://i.imgur.com/0OcoryB.png" />
Before this change both Crons and Uptime were hidden under Backend and
Frontend inisghts.
These are now directly available in the insights secondary nav