[Ingest Manager] Consolidate routing and add breadcrumbs to all pages #66475

jen-huang · 2020-05-13T20:43:42Z

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.ts was added which exports PAGE_ROUTING_PATHS and pagePathGetters, as well as types for page names
- PAGE_ROUTING_PATHS are strings used for declaring React routes i.e. <Routh path={...}>
- pagePathGetters are methods to retrieve/build a path to a specific page
- The two are declared separately rather than using 'magic' to build pagePathGetters from PAGE_ROUTING_PATHS as it's trivial to update paths in both places
useLink() hook was updated to return two methods: getHref() and getPath()
- Exporting methods rather than returning an href string directly lets us use useLink more easily in components
- getHref is used for links that appear in the UI, it builds a full path that includes the Kibana base path
- getPath does not include the Kibana base path and is used for React router's history.push
All existing links and redirects were updated to use useLink()
- Some components/sections had their own routing hooks, those were removed and usages were replaced
useBreadcrumbs() hook was created and implemented across all pages
- In addition to setting breadcrumbs, calling useBreadcrumbs() also sets the page title by reversing breadcrumbs
- The file also includes definitions for breadcrumbs for all page names

Updating routes becomes trivial with these changes. For example in 7a09c91 I updated /epm routes to /integrations to resolve #66030. This updated all links including breadcrumbs.

Screenshots

elasticmachine · 2020-05-13T20:43:45Z

Pinging @elastic/ingest-management (Team:Ingest Management)

nchaulet · 2020-05-13T22:43:47Z

x-pack/plugins/ingest_manager/public/applications/ingest_manager/hooks/use_link.ts

- return core.http.basePath.prepend(`${BASE_PATH}#${path}`);
-}
+ return {
+ getPath: (page: StaticPage | DynamicPage, values?: DynamicPagePathValues) => {


why not returning getPath directly? we can avoid to create a new function

good catch, leftover logic from when I DRY'd getPath 😅

jfsiii

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!

jfsiii · 2020-05-14T00:28:11Z

x-pack/plugins/ingest_manager/public/applications/ingest_manager/constants/page_paths.ts

+ edit_datasource: ({ configId, datasourceId }) =>
+ `/configs/${configId}/edit-datasource/${datasourceId}`,
+ fleet: () => '/fleet',
+ fleet_agent_list: () => '/fleet/agents',


I saw the kuery param used somewhere. Can we change to something like

Suggested change

fleet_agent_list: () => '/fleet/agents',

fleet_agent_list: ({ kuery }) => `/fleet/agents?kuery=${kuery}`',

jfsiii · 2020-05-14T00:28:40Z

...r/public/applications/ingest_manager/sections/agent_config/components/linked_agent_count.tsx

+ <EuiLink
+ href={`${getHref(
+ 'fleet_agent_list'
+ )}?kuery=${AGENT_SAVED_OBJECT_TYPE}.config_id : ${agentConfigId}`}


Can this go into an options object?

jfsiii · 2020-05-14T00:30:15Z

...ingest_manager/public/applications/ingest_manager/sections/epm/hooks/use_package_install.tsx

 panel: 'settings',
 });
- window.location.href = settingsUrl;
+ history.push(settingsPath);


paul-tavares · 2020-05-14T13:16:42Z

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

jen-huang · 2020-05-14T17:43:40Z

Thanks for the reviews everyone. I pushed up a change that addresses the concerns 🙂

jen-huang · 2020-05-14T20:04:44Z

@elasticmachine merge upstream

kibanamachine · 2020-05-14T22:16:00Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9625105

History

💔 Build #48037 failed 7b61116
💚 Build #47797 succeeded 7a09c91

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

…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

jen-huang added 8 commits May 12, 2020 16:20

Initial pass at normalizing links and redirects

eb1c2ee

Remove unused nav components

00b2a52

Normalize & centralize routing paths

a3f136b

Add breadcrumbs to all pages

2206740

Remove unused var

7a3d0b0

Merge remote-tracking branch 'upstream/master' into ingest/breadcrumb

24ab8ec

Merge remote-tracking branch 'upstream/master' into ingest/breadcrumb

bda8326

Change /epm to /integrations (elastic#66030)

7a09c91

jen-huang added chore v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.9.0 Team:Fleet Team label for Observability Data Collection Fleet team labels May 13, 2020

jen-huang requested a review from a team May 13, 2020 20:43

jen-huang self-assigned this May 13, 2020

nchaulet reviewed May 13, 2020

View reviewed changes

jfsiii approved these changes May 14, 2020

View reviewed changes

jfsiii reviewed May 14, 2020

View reviewed changes

jen-huang added 2 commits May 14, 2020 10:41

Fixes from review: kuery param, getPath cleanup, use chrome.docTitle,…

2950370

… and teardown breadcrumb and doc title

Merge remote-tracking branch 'upstream/master' into ingest/breadcrumb

7b61116

Merge branch 'master' into ingest/breadcrumb

9625105

jen-huang merged commit 7c22204 into elastic:master May 14, 2020

jen-huang deleted the ingest/breadcrumb branch May 14, 2020 22:28

jen-huang mentioned this pull request May 14, 2020

[7.x] [Ingest Manager] Consolidate routing and add breadcrumbs to all pages (#66475) #66668

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ingest Manager] Consolidate routing and add breadcrumbs to all pages #66475

[Ingest Manager] Consolidate routing and add breadcrumbs to all pages #66475

jen-huang commented May 13, 2020 •

edited

Loading

elasticmachine commented May 13, 2020

nchaulet May 13, 2020

jen-huang May 14, 2020

jfsiii left a comment

jfsiii May 14, 2020

jfsiii May 14, 2020

jfsiii May 14, 2020

paul-tavares commented May 14, 2020

jen-huang commented May 14, 2020

jen-huang commented May 14, 2020

kibanamachine commented May 14, 2020

	fleet_agent_list: () => '/fleet/agents',
	fleet_agent_list: ({ kuery }) => `/fleet/agents?kuery=${kuery}`',

[Ingest Manager] Consolidate routing and add breadcrumbs to all pages #66475

[Ingest Manager] Consolidate routing and add breadcrumbs to all pages #66475

Conversation

jen-huang commented May 13, 2020 • edited Loading

Summary

Screenshots

elasticmachine commented May 13, 2020

nchaulet May 13, 2020

Choose a reason for hiding this comment

jen-huang May 14, 2020

Choose a reason for hiding this comment

jfsiii left a comment

Choose a reason for hiding this comment

jfsiii May 14, 2020

Choose a reason for hiding this comment

jfsiii May 14, 2020

Choose a reason for hiding this comment

jfsiii May 14, 2020

Choose a reason for hiding this comment

paul-tavares commented May 14, 2020

jen-huang commented May 14, 2020

jen-huang commented May 14, 2020

kibanamachine commented May 14, 2020

💚 Build Succeeded

History

jen-huang commented May 13, 2020 •

edited

Loading