[Infra UI] Change breadcrumbs and add return button #164726
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
crespocarlos
added
Feature:Metrics UI
crespocarlos
force-pushed
the
148956-node-details-page-return-button
branch
from
527228d to
fc60d07
Compare
crespocarlos
force-pushed
the
148956-node-details-page-return-button
branch
from
9c6c0f2 to
93734a8
Compare
|
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
|
@elasticmachine merge upstream |
crespocarlos
changed the title
…ails-page-return-button
| 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 👍
| to={{ | ||
| pathname: `/detail/${nodeType}/${nodeId}`, | ||
| search: queryParams.toString(), | ||
| state: state ? JSON.parse(state) : undefined, |
There was a problem hiding this comment.
Might be worth wrapping JSON.parse with try/catch considering the unstructured nature of the state property.
There was a problem hiding this comment.
True! Good catch.
| ? { | ||
| state: JSON.stringify({ | ||
| originAppId: appId, | ||
| originData: location.search, |
There was a problem hiding this comment.
nit: originData sounds to me like it holds some data object, might be worth sticking to originSearch to better describe the content.
There was a problem hiding this comment.
Agree! renaming it.
…ails-page-return-button
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
closes: #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
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.