-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Uptime] Fix document title according to page #65665
Conversation
Pinging @elastic/uptime (Team:uptime) |
@elasticmachine merge upstream |
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 finding this issue.
const overviewPage = useRouteMatch(OVERVIEW_ROUTE); | ||
const monitorPage = useRouteMatch(MONITOR_ROUTE); | ||
|
||
useEffect(() => { |
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.
This feels really fragile. If we add a new page we have to remember to update this page. We probably won't.
Make we should make this a part of the page header, and use a prop passed in there instead.
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 refactored it, use typescript now it will be enforced and hopefully next next page addition will be taken care of.
|
||
useEffect(() => { | ||
if (overviewPage) { | ||
document.title = 'Uptime - Kibana'; |
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.
Since these all use the safe suffix Uptime - Kibana
let's not repeat ourselves on each assignment.
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/dashboard/_async_dashboard·ts.dashboard sample data dashboard "after all" hook for "toggle from Discover to Dashboard attempt 2"Standard Out
Stack Trace
History
To update your PR or re-run it, just comment with: |
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.
LGTM
); | ||
const baseTitle = 'Uptime - Kibana'; | ||
|
||
const Routes: RouteProps[] = [ |
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.
++ on the move to plain objects here, easier to read
Summary
Fixes: #65664
Displays correct Uptime title according to page.