[Uptime] Remove tabs from details page #85955
Conversation
shahzad31
added
release_note:skip
x-pack/plugins/uptime/public/components/common/header/page_header.tsx
Outdated
Show resolved
Hide resolved
| @@ -52,19 +53,20 @@ export const PageHeader = () => { | |||
|
|
|||
| const isMonRoute = useRouteMatch(MONITOR_ROUTE); | |||
|
|
|||
| if (isStepDetailRoute) { | |||
There was a problem hiding this comment.
Do we need unit tests for these? I'd use react testing library to check if these pieces, either the tabs, monitor heading, or nothing, were in the DOM depending on the route found with useRouteMatch
There was a problem hiding this comment.
i totally agree, those are missing
| @@ -0,0 +1,55 @@ | |||
| /* | |||
There was a problem hiding this comment.
Does this new component need unit tests?
| // generated id like `auto-http-0X8D6082B94BBE3B8A`. | ||
| // We may deprecate this behavior in the next major release, because | ||
| // the heartbeat config will require an explicit ID. | ||
| const getPageTitle = (monId: string, selectedMonitor: Ping | null) => { |
There was a problem hiding this comment.
I'd prefer monitorId to monId. It seems we are using the full word elsewhere, so I think it'd be helpful to keep it consistent and explicit.
| @@ -4,7 +4,7 @@ | |||
| * you may not use this file except in compliance with the Elastic License. | |||
| */ | |||
|
|
|||
| import { EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiTitle } from '@elastic/eui'; | |||
| import { EuiSpacer } from '@elastic/eui'; | |||
There was a problem hiding this comment.
I'd expect a test to fail from removing this much markup. Do we have unit tests for this component?
| */ | ||
|
|
||
| import { EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiTitle } from '@elastic/eui'; | ||
| import React from 'react'; |
There was a problem hiding this comment.
Just curious, do you have any standards for how you order imports? I've worked at companies where there were strong opinions about how to order imports. I noticed that in this component third party libraries are listed after internal libraries, whereas in another its reversed, so just curious.
There was a problem hiding this comment.
My $0.02 cents is that so long as the linter doesn't complain it's fine, esp. given that imports tend to be mostly managed by editors these days.
There was a problem hiding this comment.
we don't sort alphabetically, but we have lint rule for relative/absolute import orders
botelastic
bot
added
the
Team:Uptime - DEPRECATED
|
Pinging @elastic/uptime (Team:uptime) |
| return autoGeneratedId.test(id); | ||
| }; | ||
|
|
||
| // For monitors with no explicit ID, we display the URL instead of the |
There was a problem hiding this comment.
I don't think this case is working right, here's what I see:
| return autoGeneratedId.test(id); | ||
| }; | ||
|
|
||
| // For monitors with no explicit ID, we display the URL instead of the |
There was a problem hiding this comment.
I don't think this case is working right, here's what I see:
|
@andrewvc fixed the issue |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Distributable file count
History
To update your PR or re-run it, just comment with: |
Summary
Fixes: #85826
Removed tabs from detail monitor pages.