[Ingest Manager] Consolidate routing and add breadcrumbs to all pages#66475
[Ingest Manager] Consolidate routing and add breadcrumbs to all pages#66475jen-huang merged 11 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/ingest-management (Team:Ingest Management) |
| return core.http.basePath.prepend(`${BASE_PATH}#${path}`); | ||
| } | ||
| return { | ||
| getPath: (page: StaticPage | DynamicPage, values?: DynamicPagePathValues) => { |
There was a problem hiding this comment.
why not returning getPath directly? we can avoid to create a new function
There was a problem hiding this comment.
good catch, leftover logic from when I DRY'd getPath 😅
jfsiii
left a comment
There was a problem hiding this comment.
I checked this out locally and clicked around. The breadcrumb changes LGTM and are reason enough for me to ship.
I'd like to revisit the links later but anything I'd want to change is also much easier thanks to this PR. It's a big improvement over what we have now. Thanks!
| edit_datasource: ({ configId, datasourceId }) => | ||
| `/configs/${configId}/edit-datasource/${datasourceId}`, | ||
| fleet: () => '/fleet', | ||
| fleet_agent_list: () => '/fleet/agents', |
There was a problem hiding this comment.
I saw the kuery param used somewhere. Can we change to something like
| fleet_agent_list: () => '/fleet/agents', | |
| fleet_agent_list: ({ kuery }) => `/fleet/agents?kuery=${kuery}`', |
| <EuiLink | ||
| href={`${getHref( | ||
| 'fleet_agent_list' | ||
| )}?kuery=${AGENT_SAVED_OBJECT_TYPE}.config_id : ${agentConfigId}`} |
There was a problem hiding this comment.
Can this go into an options object?
| panel: 'settings', | ||
| }); | ||
| window.location.href = settingsUrl; | ||
| history.push(settingsPath); |
|
Hi @jen-huang my only suggestion would be for the Plugin unmount callback to reset the breadcrumb. I recently noticed that the breadcrumb seems to persist across Plugins - noticed it in Endpoint where we are not currently setting it and were seeing other plugin's crumbs :) |
… and teardown breadcrumb and doc title
|
Thanks for the reviews everyone. I pushed up a change that addresses the concerns 🙂 |
|
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…elastic#66475) * Initial pass at normalizing links and redirects * Remove unused nav components * Normalize & centralize routing paths * Add breadcrumbs to all pages * Remove unused var * Change /epm to /integrations (elastic#66030) * Fixes from review: kuery param, getPath cleanup, use chrome.docTitle, and teardown breadcrumb and doc title Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
…#66475) (#66668) * Initial pass at normalizing links and redirects * Remove unused nav components * Normalize & centralize routing paths * Add breadcrumbs to all pages * Remove unused var * Change /epm to /integrations (#66030) * Fixes from review: kuery param, getPath cleanup, use chrome.docTitle, and teardown breadcrumb and doc title Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
…ent/add-support-in-url-for-hidden-toggle * 'master' of github.com:elastic/kibana: (34 commits) [SIEM][CASE] Fix bug when connector is deleted. (elastic#65876) [SIEM][CASE] Improve layout (elastic#66232) [Index Management] Support Hidden Indices (elastic#66422) Add Login Selector functional tests. (elastic#65705) Lens drilldowns (elastic#65675) [ML] Custom template for apiDoc markdown (elastic#66567) Don't bootstrap core type emits (elastic#66377) [Dashboard] Improve loading error handling (elastic#66372) [APM] Minor style fixes for the node strokes (elastic#66574) [Ingest Manager] Fix create data source from integration (elastic#66626) [Metrics UI] Fix default metric alert interval for new conditions (elastic#66610) [Metrics UI] Fix alignment and allow clearing metric value (elastic#66589) Don't return package name for non-package data streams (elastic#66606) [Ingest Manager] Consolidate routing and add breadcrumbs to all pages (elastic#66475) [Docs/Reporting] Have the docs about granular timeout match Cloud docs (elastic#66267) Don't automatically add license header to code inside plugins dir. (elastic#66601) [APM] Don't trigger map layout if no elements (elastic#66625) [Logs UI] Validate ML job setup time ranges (elastic#66426) Fix pagination bugs in CCR and Remote Clusters (elastic#65931) Add cloud icon for supported settings and embed single-sourced getting started (elastic#65610) ... # Conflicts: # x-pack/plugins/index_management/public/application/sections/home/index_list/index_table/index_table.js # x-pack/plugins/index_management/server/lib/fetch_indices.ts
Resolves #63981
Resolves #66030
Summary
This PR adds breadcrumbs to all pages. As part of this work, I also consolidated our routing and linking constants and methods (the breadcrumbs work would have been messy otherwise 🙂). More detailed summary of changes:
constants/page_paths.tswas added which exportsPAGE_ROUTING_PATHSandpagePathGetters, as well as types for page namesPAGE_ROUTING_PATHSare strings used for declaring React routes i.e.<Routh path={...}>pagePathGettersare methods to retrieve/build a path to a specific pagepagePathGettersfromPAGE_ROUTING_PATHSas it's trivial to update paths in both placesuseLink()hook was updated to return two methods:getHref()andgetPath()useLinkmore easily in componentsgetHrefis used for links that appear in the UI, it builds a full path that includes the Kibana base pathgetPathdoes not include the Kibana base path and is used for React router'shistory.pushuseLink()useBreadcrumbs()hook was created and implemented across all pagesuseBreadcrumbs()also sets the page title by reversing breadcrumbsUpdating routes becomes trivial with these changes. For example in 7a09c91 I updated
/epmroutes to/integrationsto resolve #66030. This updated all links including breadcrumbs.Screenshots