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
Rename Overview pages to Details pages #3934
Rename Overview pages to Details pages #3934
Conversation
@@ -115,7 +115,7 @@ const AppContents_: React.FC<AppContentsProps> = ({ activePerspective, flags }) | |||
|
|||
<Route path={['/all-namespaces', '/ns/:ns']} component={RedirectComponent} /> | |||
<LazyRoute | |||
path="/dashboards" | |||
path="/overview" |
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'm undecided on this because it will break old links to old route /overview/:namespace
, which used to be the project overview. We might leave the URL as-is and just update the labels in the UI.
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.
FYI @rawagner after some more discussion I think we've agreed to not break the URL right now to avoid bookmarks going to a weird place.
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.
ok, the change to url was removed
9114e78
to
d8d310b
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.
/approve
/lgtm
Thanks, feel free to remove WIP when ready
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.
Got approval from stakeholders on the Admin & Dev sides - I think this is good to go. 👍
d8d310b
to
0e9d1b0
Compare
@spadgett I had to rebase, can you lgtm again ? I didnt find any issues so far so removing WIP. |
faeaa99
to
759a45c
Compare
/retest |
1 similar comment
/retest |
/lgtm |
/retest Please review the full test history for this PR and help us cut down flakes. |
/lgtm cancel @rawagner Looks like you need to update the Monitoring tests to expect |
759a45c
to
6cb2e74
Compare
/retest |
1 similar comment
/retest |
@kyoto fixed the monitoring tests |
6cb2e74
to
421e535
Compare
/lgtm |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
/hold needs rebase again |
Rename Dashboards page to Overview Rename Overview Dashboard page to Cluster
421e535
to
fe2c461
Compare
/hold cancel |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andybraren, benjaminapetersen, kyoto, rawagner, spadgett, suomiy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Rename Dashboards page to Overview
Rename Overview Dashboard page to Cluster
@andybraren @spadgett