[Infra UI] Change breadcrumbs and add return button #164726

crespocarlos · 2023-08-24T14:42:13Z

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

hosts_view_node_details.mov

Return to Inventory UI

inventory_node_details.mov

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

metrics_node_details.mov

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

Return to APM

APM_node_details.mov

Return button remains after page refresh

refresh_return.mov

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

apmmachine · 2023-08-24T14:42:29Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

…ails-page-return-button

elasticmachine · 2023-08-28T15:13:07Z

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

crespocarlos · 2023-08-28T15:13:31Z

@elasticmachine merge upstream

…ails-page-return-button

mykolaharmash · 2023-08-29T11:35:33Z

x-pack/plugins/infra/public/components/asset_details/hooks/use_page_header.tsx

+  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


One small caveat about history.length here. If a user has a browser tab with existing history and then pastes a Kibana URL into that tab, history will still have all of the previous records and the Return button will go back to an arbitrary page from the history. Kind of an edge case, but worth mentioning.

I tried to simulate this, but the history is reset when I paste a URL in a tab with existing history.
Returning using history.goBack() should be an edge case. I'm even thinking about removing it.

Seems like react router does some additional transformations with the history. Just checked, after pasting a URL window.history has all the previous records and history from useHistory() has only the current record. All good then 👍

mykolaharmash

Just tested, everything works as on the videos ✨ Left a few small comments.

mykolaharmash · 2023-08-29T12:17:36Z

x-pack/plugins/infra/public/pages/link_to/redirect_to_node_detail.tsx

+      to={{
+        pathname: `/detail/${nodeType}/${nodeId}`,
+        search: queryParams.toString(),
+        state: state ? JSON.parse(state) : undefined,


Might be worth wrapping JSON.parse with try/catch considering the unstructured nature of the state property.

True! Good catch.

mykolaharmash · 2023-08-29T12:24:41Z

x-pack/plugins/infra/public/pages/link_to/use_node_details_redirect.ts

+            ? {
+                state: JSON.stringify({
+                  originAppId: appId,
+                  originData: location.search,


nit: originData sounds to me like it holds some data object, might be worth sticking to originSearch to better describe the content.

Agree! renaming it.

x-pack/plugins/infra/public/pages/link_to/use_node_details_redirect.ts

…ails-page-return-button

kibana-ci · 2023-08-29T14:21:44Z

💚 Build Succeeded

Buildkite Build
Commit: 4501caa

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`infra`	1423	1424	+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`	2.0MB	2.0MB	+1.8KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`infra`	51	50	-1

Total ESLint disabled count

id	before	after	diff
`infra`	60	59	-1

History

💚 Build #154380 succeeded 0efcde3
💚 Build #154150 succeeded 5e6295f
💛 Build #154114 was flaky 07dae1d
💔 Build #154032 failed e9aaaae
💔 Build #153525 failed 369f141
💛 Build #153447 was flaky 8edffc4

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

mykolaharmash

Looks good! ✨🚀

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>

Change breadcrumbs and add return button

fc60d07

crespocarlos force-pushed the 148956-node-details-page-return-button branch from 527228d to fc60d07 Compare August 24, 2023 14:46

crespocarlos added 4 commits August 25, 2023 11:57

Merge branch 'main' of github.com:elastic/kibana into 148956-node-det…

8edffc4

…ails-page-return-button

Implement route state persistance

391825c

Replace instead of push route

369f141

Centralize route state in hook

93734a8

crespocarlos force-pushed the 148956-node-details-page-return-button branch from 9c6c0f2 to 93734a8 Compare August 28, 2023 09:54

crespocarlos added 2 commits August 28, 2023 12:32

Fix return to APM

e9aaaae

Fix tests

07dae1d

crespocarlos marked this pull request as ready for review August 28, 2023 15:13

crespocarlos requested a review from a team as a code owner August 28, 2023 15:13

Merge branch 'main' into 148956-node-details-page-return-button

5e6295f

crespocarlos changed the title ~~Change breadcrumbs and add return button~~ [Infra UI] Change breadcrumbs and add return button Aug 28, 2023

crespocarlos added 2 commits August 29, 2023 09:28

Merge branch 'main' of github.com:elastic/kibana into 148956-node-det…

5e4ba9b

…ails-page-return-button

Small adjustment

0efcde3

mykolaharmash reviewed Aug 29, 2023

View reviewed changes

crespocarlos added 2 commits August 29, 2023 15:05

Merge branch 'main' of github.com:elastic/kibana into 148956-node-det…

eeb84c2

…ails-page-return-button

CR fixes

4501caa

crespocarlos requested a review from mykolaharmash August 29, 2023 15:53

mykolaharmash approved these changes Aug 30, 2023

View reviewed changes

crespocarlos merged commit 9ca30e8 into elastic:main Aug 30, 2023
23 checks passed

crespocarlos deleted the 148956-node-details-page-return-button branch August 30, 2023 07:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Infra UI] Change breadcrumbs and add return button #164726

[Infra UI] Change breadcrumbs and add return button #164726

crespocarlos commented Aug 24, 2023 •

edited

Loading

apmmachine commented Aug 24, 2023

elasticmachine commented Aug 28, 2023

crespocarlos commented Aug 28, 2023

mykolaharmash Aug 29, 2023

crespocarlos Aug 29, 2023

mykolaharmash Aug 29, 2023

mykolaharmash left a comment

mykolaharmash Aug 29, 2023

crespocarlos Aug 29, 2023

mykolaharmash Aug 29, 2023

crespocarlos Aug 29, 2023

kibana-ci commented Aug 29, 2023

ESLint disabled line counts

Total ESLint disabled count

mykolaharmash left a comment

[Infra UI] Change breadcrumbs and add return button #164726

[Infra UI] Change breadcrumbs and add return button #164726

Conversation

crespocarlos commented Aug 24, 2023 • edited Loading

Summary

How to test this PR.

apmmachine commented Aug 24, 2023

🤖 GitHub comments

elasticmachine commented Aug 28, 2023

crespocarlos commented Aug 28, 2023

mykolaharmash Aug 29, 2023

Choose a reason for hiding this comment

crespocarlos Aug 29, 2023

Choose a reason for hiding this comment

mykolaharmash Aug 29, 2023

Choose a reason for hiding this comment

mykolaharmash left a comment

Choose a reason for hiding this comment

mykolaharmash Aug 29, 2023

Choose a reason for hiding this comment

crespocarlos Aug 29, 2023

Choose a reason for hiding this comment

mykolaharmash Aug 29, 2023

Choose a reason for hiding this comment

crespocarlos Aug 29, 2023

Choose a reason for hiding this comment

kibana-ci commented Aug 29, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

mykolaharmash left a comment

Choose a reason for hiding this comment

crespocarlos commented Aug 24, 2023 •

edited

Loading