Skip to content

Commit

Permalink
[Infra UI] Change breadcrumbs and add return button (elastic#164726)
Browse files Browse the repository at this point in the history
closes: elastic#148956

## Summary

This PR adds a return button to the Node Details page, both old and new
versions. When returning to the previous page, the page has to load with
its previous state. **This doesn't work for the Inventory and Metrics
UI.**

The Return button will not show when: the URL is copied from the address
bar and opened in a new tab/browser. Clicking on the a link to open the
page in a new tab will show the return button.


**Return to Host View**


https://github.com/elastic/kibana/assets/2767137/06e218fc-5f35-4e67-a4f9-3f04b3f79dee


**Return to Inventory UI**


https://github.com/elastic/kibana/assets/2767137/906012ce-e5ca-49dd-a19b-2b3d16e9e4f6

_Returning state in Inventory UI doesn't work. There are 2 main
problems: The hierarchy of the WaffleOptionContext in the page and the
Saved View load, which overrides the URL state._

**Return to Metrics UI**


https://github.com/elastic/kibana/assets/2767137/de2b19f4-c5e2-4368-8e00-b50ae9ba357b

_Returning state in Metrics UI doesn't work because the Saved View
overrides the URL state._

**Return to APM**


https://github.com/elastic/kibana/assets/2767137/704ce2f0-17eb-4fa9-a791-f0cf9b29c43a

**Return button remains after page refresh**


https://github.com/elastic/kibana/assets/2767137/58600504-1863-4254-83ea-a2d488e2b38a

**Here it's possible to see that the Inventory UI doesn't return to its
previous state**


###  How to test this PR.

- Setup a new Kibana instance
- Follow the steps from the video above

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
2 people authored and bryce-b committed Sep 19, 2023
1 parent 35ca3bd commit 014abd4
Show file tree
Hide file tree
Showing 23 changed files with 347 additions and 121 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,88 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { useEuiTheme, EuiIcon, type EuiPageHeaderProps } from '@elastic/eui';
import {
useEuiTheme,
EuiIcon,
type EuiPageHeaderProps,
type EuiBreadcrumbsProps,
EuiFlexGroup,
EuiFlexItem,
} from '@elastic/eui';
import { css } from '@emotion/react';
import { useLinkProps } from '@kbn/observability-shared-plugin/public';
import React, { useCallback, useMemo } from 'react';
import { capitalize } from 'lodash';
import { useHistory, useLocation } from 'react-router-dom';
import { FormattedMessage } from '@kbn/i18n-react';
import { useKibanaContextForPlugin } from '../../../hooks/use_kibana';
import { APM_HOST_FILTER_FIELD } from '../constants';
import { LinkToAlertsRule, LinkToApmServices, LinkToNodeDetails } from '../links';
import { FlyoutTabIds, type LinkOptions, type Tab, type TabIds } from '../types';
import { FlyoutTabIds, type RouteState, type LinkOptions, type Tab, type TabIds } from '../types';
import { useAssetDetailsRenderPropsContext } from './use_asset_details_render_props';
import { useDateRangeProviderContext } from './use_date_range';
import { useTabSwitcherContext } from './use_tab_switcher';

type TabItem = NonNullable<Pick<EuiPageHeaderProps, 'tabs'>['tabs']>[number];

export const usePageHeader = (tabs: Tab[], links?: LinkOptions[]) => {
export const usePageHeader = (tabs: Tab[] = [], links: LinkOptions[] = []) => {
const { rightSideItems } = useRightSideItems(links);
const { tabEntries } = useTabs(tabs);
const { breadcrumbs } = useTemplateHeaderBreadcrumbs();

return { rightSideItems, tabEntries, breadcrumbs };
};

export const useTemplateHeaderBreadcrumbs = () => {
const history = useHistory();
const location = useLocation<RouteState>();
const {
services: {
application: { navigateToApp },
},
} = useKibanaContextForPlugin();

const onClick = (e: React.MouseEvent) => {
if (location.state) {
navigateToApp(location.state.originAppId, {
replace: true,
path: `${location.state.originPathname}${location.state.originSearch}`,
});
} else {
history.goBack();
}
e.preventDefault();
};

const breadcrumbs: EuiBreadcrumbsProps['breadcrumbs'] =
// If there is a state object in location, it's persisted in case the page is opened in a new tab or after page refresh
// With that, we can show the return button. Otherwise, it will be hidden (ex: the user opened a shared URL or opened the page from their bookmarks)
location.state || history.length > 1
? [
{
text: (
<EuiFlexGroup gutterSize="xs" alignItems="center">
<EuiFlexItem>
<EuiIcon size="s" type="arrowLeft" />
</EuiFlexItem>
<EuiFlexItem>
<FormattedMessage
id="xpack.infra.assetDetails.header.return"
defaultMessage="Return"
/>
</EuiFlexItem>
</EuiFlexGroup>
),
color: 'primary',
'aria-current': false,
'data-test-subj': 'infraAssetDetailsReturnButton',
href: '#',
onClick,
},
]
: [];

return { rightSideItems, tabEntries };
return { breadcrumbs };
};

const useRightSideItems = (links?: LinkOptions[]) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import React from 'react';
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiButtonEmpty } from '@elastic/eui';
import { useLinkProps } from '@kbn/observability-shared-plugin/public';
import { getNodeDetailUrl } from '../../../pages/link_to';
import { useNodeDetailsRedirect } from '../../../pages/link_to';
import type { InventoryItemType } from '../../../../common/inventory_models/types';

export interface LinkToNodeDetailsProps {
Expand All @@ -22,13 +22,16 @@ export const LinkToNodeDetails = ({
assetType,
dateRangeTimestamp,
}: LinkToNodeDetailsProps) => {
const { getNodeDetailUrl } = useNodeDetailsRedirect();
const nodeDetailMenuItemLinkProps = useLinkProps({
...getNodeDetailUrl({
nodeType: assetType,
nodeId: asset.id,
from: dateRangeTimestamp.from,
to: dateRangeTimestamp.to,
assetName: asset.name,
search: {
from: dateRangeTimestamp.from,
to: dateRangeTimestamp.to,
assetName: asset.name,
},
}),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type { ContentTemplateProps } from '../types';

export const Page = ({ header: { tabs = [], links = [] } }: ContentTemplateProps) => {
const { asset, loading } = useAssetDetailsRenderPropsContext();
const { rightSideItems, tabEntries } = usePageHeader(tabs, links);
const { rightSideItems, tabEntries, breadcrumbs } = usePageHeader(tabs, links);
const { headerHeight } = useKibanaHeader();

return loading ? (
Expand Down Expand Up @@ -49,6 +49,7 @@ export const Page = ({ header: { tabs = [], links = [] } }: ContentTemplateProps
pageTitle={asset.name}
tabs={tabEntries}
rightSideItems={rightSideItems}
breadcrumbs={breadcrumbs}
/>
<EuiPageTemplate.Section grow>
<Content />
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/infra/public/components/asset_details/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { TimeRange } from '@kbn/es-query';
import { Search } from 'history';
import type { InventoryItemType } from '../../../common/inventory_models/types';

export interface Asset {
Expand Down Expand Up @@ -79,4 +80,10 @@ export interface ContentTemplateProps {
header: Pick<AssetDetailsProps, 'tabs' | 'links'>;
}

export interface RouteState {
originAppId: string;
originPathname?: string;
originSearch?: Search;
}

export type DataViewOrigin = 'logs' | 'metrics';
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ import { createLazyContainerMetricsTable } from './create_lazy_container_metrics
import IntegratedContainerMetricsTable from './integrated_container_metrics_table';
import { metricByField } from './use_container_metrics_table';

jest.mock('../../../pages/link_to', () => ({
useNodeDetailsRedirect: jest.fn(() => ({
getNodeDetailUrl: jest.fn(() => ({
app: 'metrics',
pathname: 'link-to/container-detail/example-01',
search: { from: '1546340400000', to: '1546344000000' },
})),
})),
}));

describe('ContainerMetricsTable', () => {
const timerange = {
from: 'now-15m',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ import { HostMetricsTable } from './host_metrics_table';
import IntegratedHostMetricsTable from './integrated_host_metrics_table';
import { metricByField } from './use_host_metrics_table';

jest.mock('../../../pages/link_to', () => ({
useNodeDetailsRedirect: jest.fn(() => ({
getNodeDetailUrl: jest.fn(() => ({
app: 'metrics',
pathname: 'link-to/host-detail/example-01',
search: { from: '1546340400000', to: '1546344000000' },
})),
})),
}));

describe('HostMetricsTable', () => {
const timerange = {
from: 'now-15m',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ import IntegratedPodMetricsTable from './integrated_pod_metrics_table';
import { PodMetricsTable } from './pod_metrics_table';
import { metricByField } from './use_pod_metrics_table';

jest.mock('../../../pages/link_to', () => ({
useNodeDetailsRedirect: jest.fn(() => ({
getNodeDetailUrl: jest.fn(() => ({
app: 'metrics',
pathname: 'link-to/pod-detail/example-01',
search: { from: '1546340400000', to: '1546344000000' },
})),
})),
}));

describe('PodMetricsTable', () => {
const timerange = {
from: 'now-15m',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { EuiLink } from '@elastic/eui';
import React from 'react';
import { useLinkProps } from '@kbn/observability-shared-plugin/public';
import type { InventoryItemType } from '../../../../../common/inventory_models/types';
import { getNodeDetailUrl } from '../../../../pages/link_to';
import { useNodeDetailsRedirect } from '../../../../pages/link_to';
import type { MetricsExplorerTimeOptions } from '../../../../pages/metrics/metrics_explorer/hooks/use_metrics_explorer_options';

type ExtractStrict<T, U extends T> = Extract<T, U>;
Expand All @@ -28,12 +28,15 @@ export const MetricsNodeDetailsLink = ({
nodeType,
timerange,
}: MetricsNodeDetailsLinkProps) => {
const { getNodeDetailUrl } = useNodeDetailsRedirect();
const linkProps = useLinkProps(
getNodeDetailUrl({
nodeType,
nodeId: id,
from: parse(timerange.from)?.valueOf(),
to: parse(timerange.to)?.valueOf(),
search: {
from: parse(timerange.from)?.valueOf(),
to: parse(timerange.to)?.valueOf(),
},
})
);

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/infra/public/pages/link_to/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@
export { LinkToLogsPage } from './link_to_logs';
export { LinkToMetricsPage } from './link_to_metrics';
export { RedirectToNodeLogs } from './redirect_to_node_logs';
export { getNodeDetailUrl, RedirectToNodeDetail } from './redirect_to_node_detail';
export { RedirectToNodeDetail } from './redirect_to_node_detail';
export { useNodeDetailsRedirect } from './use_node_details_redirect';
5 changes: 5 additions & 0 deletions x-pack/plugins/infra/public/pages/link_to/query_params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@ export const getNodeNameFromLocation = (location: Location) => {
const nameParam = getParamFromQueryString(getQueryStringFromLocation(location), 'assetName');
return nameParam;
};

export const getStateFromLocation = (location: Location) => {
const nameParam = getParamFromQueryString(getQueryStringFromLocation(location), 'state');
return nameParam;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,24 @@
*/

import React from 'react';
import { Redirect, RouteComponentProps } from 'react-router-dom';

import { LinkDescriptor } from '@kbn/observability-shared-plugin/public';
import { Redirect, useLocation, useRouteMatch } from 'react-router-dom';
import { replaceMetricTimeInQueryString } from '../metrics/metric_detail/hooks/use_metrics_time';
import { getFromFromLocation, getToFromLocation, getNodeNameFromLocation } from './query_params';
import {
getFromFromLocation,
getToFromLocation,
getNodeNameFromLocation,
getStateFromLocation,
} from './query_params';
import { InventoryItemType } from '../../../common/inventory_models/types';
import { RouteState } from '../../components/asset_details/types';

export const RedirectToNodeDetail = () => {
const {
params: { nodeType, nodeId },
} = useRouteMatch<{ nodeType: InventoryItemType; nodeId: string }>();

type RedirectToNodeDetailProps = RouteComponentProps<{
nodeId: string;
nodeType: InventoryItemType;
}>;
const location = useLocation();

export const RedirectToNodeDetail = ({
match: {
params: { nodeId, nodeType },
},
location,
}: RedirectToNodeDetailProps) => {
const searchString = replaceMetricTimeInQueryString(
getFromFromLocation(location),
getToFromLocation(location)
Expand All @@ -38,33 +38,21 @@ export const RedirectToNodeDetail = ({
}
}

return <Redirect to={`/detail/${nodeType}/${nodeId}?${queryParams.toString()}`} />;
};
let state: RouteState | undefined;
try {
const stateFromLocation = getStateFromLocation(location);
state = stateFromLocation ? JSON.parse(stateFromLocation) : undefined;
} catch (err) {
state = undefined;
}

export const getNodeDetailUrl = ({
nodeType,
nodeId,
to,
from,
assetName,
}: {
nodeType: InventoryItemType;
nodeId: string;
to?: number;
from?: number;
assetName?: string;
}): LinkDescriptor => {
return {
app: 'metrics',
pathname: `link-to/${nodeType}-detail/${nodeId}`,
search: {
...(assetName ? { assetName } : undefined),
...(to && from
? {
to: `${to}`,
from: `${from}`,
}
: undefined),
},
};
return (
<Redirect
to={{
pathname: `/detail/${nodeType}/${nodeId}`,
search: queryParams.toString(),
state,
}}
/>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import { renderHook } from '@testing-library/react-hooks';
import { useNodeDetailsRedirect } from './use_node_details_redirect';
import { coreMock } from '@kbn/core/public/mocks';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';

const coreStartMock = coreMock.createStart();

jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useLocation: jest.fn(() => ({
pathname: '',
search: '',
})),
}));

const wrapper = ({ children }: { children: React.ReactNode }): JSX.Element => (
<KibanaContextProvider services={{ ...coreStartMock }}>{children}</KibanaContextProvider>
);

describe('useNodeDetailsRedirect', () => {
it('should return the LinkProperties', () => {
const { result } = renderHook(() => useNodeDetailsRedirect(), { wrapper });

const fromDateStrig = '2019-01-01T11:00:00Z';
const toDateStrig = '2019-01-01T12:00:00Z';

expect(
result.current.getNodeDetailUrl({
nodeType: 'host',
nodeId: 'example-01',
search: {
from: new Date(fromDateStrig).getTime(),
to: new Date(toDateStrig).getTime(),
},
})
).toStrictEqual({
app: 'metrics',
pathname: 'link-to/host-detail/example-01',
search: { from: '1546340400000', to: '1546344000000' },
});
});
});
Loading

0 comments on commit 014abd4

Please sign in to comment.