[Infra Monitoring UI] Change infrastructure pages titles to use breadcrumbs #135874 #140824

jennypavlova · 2022-09-15T14:19:57Z

Summary

Related #135874
This PR changes the titles in Logs and Infrastructure apps. Similar to other apps now the titles are constructed from the breadcrumbs in reverse order. This helps us to keep the titles consistent. Also, it makes it easier for users to navigate using screen readers.

Based on the comment regarding how we should set the document title here I also changed the error page title to use chrome.docTitle.change in (ebe26be). The error page can also use the general breadcrumbs title but it has a different title now. So I kept the current title and added the Observability title part to it so it will be closer to the other page titles.

Example

Old title: Metrics | Inventory - Kibana
New title: Inventory - Infrastructure - Observability - Elastic

Testing

I checked locally. Steps:

Navigate to the Infrastructure page
Check all document titles (all pages: Hosts/Metrics Explorer/Inventory and sub-pages)
Same with Logs pages (Stream/Categories/Anomalies)
✅ check in the cloud as well.

…-use-breadcrumbs

elasticmachine · 2022-09-23T09:24:30Z

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

…-use-breadcrumbs

crespocarlos

Thanks for this change and for adding the tests 💪 ! I left just a couple of comments, but nothing major.

crespocarlos · 2022-09-26T13:57:55Z

x-pack/plugins/infra/public/hooks/use_breadcrumbs.ts

-    ]);
+    ];
+
+    const docTitle: string[] = [...breadcrumbs]


We're already casting breadcrumb.text as string so the returned array will be a string[]. Do we still need to declare docTitle as string[]?

You are right, there is no need to declare it. Removed.

crespocarlos · 2022-09-26T14:33:05Z

x-pack/plugins/infra/public/pages/metrics/metric_detail/components/page_error.tsx

 import { ErrorPageBody } from '../../../error';
 import { InfraHttpError } from '../../../../types';
+import { errorTitle } from '../../../../translations';

 interface Props {
  name: string;
  error: InfraHttpError;
 }

 export const PageError = ({ error, name }: Props) => {


It would be nice to add a test for this one!

I added one unit test for the error page component and a functional test 4e2fd39

crespocarlos · 2022-09-26T15:11:51Z

The title changes according to what's expected, but the error page apparently has a problem. Perhaps the way I brute-forced an error, by blocking requests to 0.0.0.0:5601/ftw/api/metrics/node_details, isn't the right way. But maybe it's worth to look into.

Here the title should contain Uh oh, but metricbeat overview page is always the last to change the title. There is a race condition between useBreadcrumbs and useDocumentTitle, I'm trying to figure out how to solve it.

crespocarlos · 2022-09-26T15:17:48Z

@elasticmachine merge upstream

…-use-breadcrumbs

matschaffer · 2022-09-27T07:11:42Z

@elasticmachine merge upstream

…-use-breadcrumbs

jennypavlova · 2022-09-27T09:18:09Z

The title changes according to what's expected, but the error page apparently has a problem. Perhaps the way I brute-forced an error, by blocking requests to 0.0.0.0:5601/ftw/api/metrics/node_details, isn't the right way. But maybe it's worth to look into.

Here the title should contain Uh oh, but metricbeat overview page is always the last to change the title. There is a race condition between useBreadcrumbs and useDocumentTitle, I'm trying to figure out how to solve it.

@crespocarlos Thank you for finding that! Really interesting when I was testing it I put a not existing name in the url to force the error and it looks good:

I will take a look and try to reproduce your case and find a solution.

…-use-breadcrumbs

crespocarlos

Thanks for adding the tests for the page_error component! I just left a couple of comments/questions.

crespocarlos · 2022-09-28T08:49:54Z

x-pack/plugins/infra/public/pages/metrics/metric_detail/components/page_error.test.tsx

+ */
+
+import React from 'react';
+import { mount } from 'enzyme';


Enzyme works just fine, especially if we need to play with component internals and state. In this case we don't need to do that so I believe we could use react-testing-library. What do you think?

That's a good point, thank you. I agree, I realize that we don't actually need to change the internal state there - just to test if the hook is called and the text error message is correct. I updated it 👍

crespocarlos · 2022-09-28T08:58:01Z

x-pack/plugins/infra/public/pages/metrics/metric_detail/components/page_error.test.tsx

+    } as InfraHttpError);
+
+    const callOut = mounted.find('EuiCallOut');
+    expect(callOut.render()).toMatchSnapshot();


Instead of matching a snapshot, how about if we just assert the existence of the "Please click the back button and try again." on the page?

I removed the snapshot and I added a check for the error name and message

kibana-ci · 2022-09-28T10:21:29Z

💚 Build Succeeded

Buildkite Build
Commit: 95a5de0

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`infra`	1007	1006	-1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`infra`	1.0MB	1022.9KB	-1.6KB

History

💔 Build #76328 failed e548f25
💔 Build #76220 failed 525c577
💔 Build #76175 failed 5c9b6d5
💔 Build #76084 failed b084fb7
💚 Build #76052 succeeded 98b9124

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jennypavlova

crespocarlos

Looks good! Thanks for all the improvements

jennypavlova self-assigned this Sep 19, 2022

jennypavlova added ci:cloud-deploy Create or update a Cloud deployment Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Sep 19, 2022

jennypavlova added 7 commits September 22, 2022 18:14

Use breadcrumbs to set page title intead of DocumentTitle

e53c650

Remove document title component and use breadcrumbs - Logs Page

b3dd5d3

Remove no longer needed document title translations

4ea8b09

Use docTitle.change in error page and remove document title component

4ca752d

Add document title check to the functional tests

50cc230

Move test to check title after load

12c7820

test: Title change

a4166c0

jennypavlova force-pushed the 135874-change-infrastructure-pages-titles-to-use-breadcrumbs branch from 5f3d989 to a4166c0 Compare September 22, 2022 16:14

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

66c356f

…-use-breadcrumbs

jennypavlova marked this pull request as ready for review September 23, 2022 09:24

jennypavlova requested a review from a team as a code owner September 23, 2022 09:24

jennypavlova added release_note:skip Skip the PR/issue when compiling release notes v8.6.0 labels Sep 23, 2022

smith added the technical debt Improvement of the software architecture and operational architecture label Sep 23, 2022

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

80e772a

…-use-breadcrumbs

crespocarlos reviewed Sep 26, 2022

View reviewed changes

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

21fa65d

…-use-breadcrumbs

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

9c6893c

…-use-breadcrumbs

This comment was marked as resolved.

Sign in to view

Remove unnecessary docTitle type

247702b

jennypavlova added 2 commits September 27, 2022 14:42

Add error page tests

4e2fd39

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

a801588

…-use-breadcrumbs

jennypavlova added 7 commits September 27, 2022 15:41

Update snapshot

88968d8

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

98b9124

…-use-breadcrumbs

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

b084fb7

…-use-breadcrumbs

Functional tests: Wait for loading the header before checking the title

2245f97

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

5c9b6d5

…-use-breadcrumbs

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

525c577

…-use-breadcrumbs

Merge branch 'main' into 135874-change-infrastructure-pages-titles-to…

e548f25

…-use-breadcrumbs

jennypavlova requested a review from crespocarlos September 28, 2022 08:13

jennypavlova removed the ci:cloud-deploy Create or update a Cloud deployment label Sep 28, 2022

crespocarlos reviewed Sep 28, 2022

View reviewed changes

Change test to use react testing library

95a5de0

jennypavlova requested a review from crespocarlos September 28, 2022 11:19

crespocarlos approved these changes Sep 28, 2022

View reviewed changes

jennypavlova merged commit 82492c7 into elastic:main Sep 28, 2022

kibanamachine added the backport:skip This commit does not require backporting label Sep 28, 2022

jennypavlova deleted the 135874-change-infrastructure-pages-titles-to-use-breadcrumbs branch September 28, 2022 11:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Infra Monitoring UI] Change infrastructure pages titles to use breadcrumbs #135874 #140824

[Infra Monitoring UI] Change infrastructure pages titles to use breadcrumbs #135874 #140824

jennypavlova commented Sep 15, 2022 •

edited

Loading

elasticmachine commented Sep 23, 2022

crespocarlos left a comment

crespocarlos Sep 26, 2022

jennypavlova Sep 27, 2022

crespocarlos Sep 26, 2022

jennypavlova Sep 27, 2022

crespocarlos commented Sep 26, 2022 •

edited

Loading

crespocarlos commented Sep 26, 2022

matschaffer commented Sep 27, 2022

This comment was marked as resolved.

jennypavlova commented Sep 27, 2022

crespocarlos left a comment

crespocarlos Sep 28, 2022

jennypavlova Sep 28, 2022

crespocarlos Sep 28, 2022

jennypavlova Sep 28, 2022

kibana-ci commented Sep 28, 2022

crespocarlos left a comment

[Infra Monitoring UI] Change infrastructure pages titles to use breadcrumbs #135874 #140824

[Infra Monitoring UI] Change infrastructure pages titles to use breadcrumbs #135874 #140824

Conversation

jennypavlova commented Sep 15, 2022 • edited Loading

Summary

Example

Testing

elasticmachine commented Sep 23, 2022

crespocarlos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crespocarlos commented Sep 26, 2022 • edited Loading

crespocarlos commented Sep 26, 2022

matschaffer commented Sep 27, 2022

This comment was marked as resolved.

jennypavlova commented Sep 27, 2022

crespocarlos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Sep 28, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

crespocarlos left a comment

Choose a reason for hiding this comment

jennypavlova commented Sep 15, 2022 •

edited

Loading

crespocarlos commented Sep 26, 2022 •

edited

Loading