[Uptime] Refactor header #58836
[Uptime] Refactor header #58836
Conversation
andrewvc
added
[zube]: In Progress
technical debt
|
Pinging @elastic/uptime (Team:uptime) |
andrewvc
changed the title
|
|
||
| const mapStateToProps: MapStateToPropsParam<StateProps, {}, AppState> = state => ({ | ||
| selectedMonitor: selectSelectedMonitor(state), | ||
| }); | ||
|
|
||
| const mapDispatchToProps: MapDispatchToPropsFunction<DispatchProps, {}> = (dispatch, own) => { | ||
| return { | ||
| dispatchGetMonitorStatus: (monitorId: string) => { | ||
| dispatch( | ||
| getSelectedMonitor({ | ||
| monitorId, | ||
| }) | ||
| ); | ||
| }, | ||
| }; | ||
| }; | ||
|
|
||
| export const MonitorPage = connect(mapStateToProps, mapDispatchToProps)(MonitorPageComponent); |
There was a problem hiding this comment.
Can you move this part of component to components/connected/pages? we are trying to keep all the data injecting in same folder. It helps us in testing presentational components.
There was a problem hiding this comment.
After thinking about it, since we've decided to reorganize this all soon, are you OK with it being left here for this PR?
| @@ -73,6 +74,7 @@ export const OverviewPageComponent = ({ autocomplete, indexPattern, setEsKueryFi | |||
|
|
|||
| return ( | |||
| <> | |||
| <PageHeader headingText={'Overview'} breadcrumbs={[]} datePicker={true} /> | |||
There was a problem hiding this comment.
We should use label
i18n.translate('xpack.uptime.overviewPage.headerText', {
defaultMessage: 'Overview',
description: `The text that will be displayed in the app's heading when the Overview page loads.`,
})
for "Overview".
| export const BaseBreadcrumb: ChromeBreadcrumb = { | ||
| text: i18n.translate('xpack.uptime.breadcrumbs.overviewBreadcrumbText', { | ||
| defaultMessage: 'Uptime', | ||
| }), | ||
| href: '#/', | ||
| }; |
There was a problem hiding this comment.
I just realized, when on monitor page, and if you click on uptime link to go back to overview page, it resets all filters, i think we can persist all those filters from overview page once we navigate to monitor page.
Especially date range filters, can append those here into href?
andrewvc
added
release_note:skip
|
@shahzad31 this is ready for another review, I believe I've addressed all comments |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
|
|
||
| const params = useUrlParams()[0](); |
There was a problem hiding this comment.
we should try to make usage of urlParams hooks easy, it's so awkward using it in current state.
Cleanup implementation of the header to let pages embed the header rather than the weird sort of control the header works with today. Also uses kibana context in a way that makes more sense, and provide a path forward for elastic#53550 since that will need to add a new header type (and some buttons next to the picker). Fixes elastic#58835
Cleanup implementation of the header to let pages embed the header rather than the weird sort of control the header works with today. Also uses kibana context in a way that makes more sense, and provide a path forward for #53550 since that will need to add a new header type (and some buttons next to the picker). Fixes #58835
Summary
Cleanup implementation of the header to let pages embed the header rather than the weird sort of control the header works with today. Also uses kibana context in a way that makes more sense, and provide a path forward for #53550 since that will need to add a new header type (and some buttons next to the picker).
Fixes #58835
Checklist
Delete any items that are not applicable to this PR.
For maintainers