Refactor observability plugin breadcrumbs by smith · Pull Request #102290 · elastic/kibana

smith · 2021-06-16T04:55:42Z

Previously the observability plugin set the page title and breadcrumbs in the main app rendering component based on the breadcrumb property of the current route.

In addition, there's a useBreadcrumb hook used by the UX app, exploratory view, and cases.

The conflict between these was creating situations where neither would work and the breadcrumbs would just show "Kibana".

Remove the breadcrumb properties from the routes and the main app breadcrumb handling and just use useBreadcrumb on all pages.

Fixes #102131.

Previously the observability plugin set the page title and breadcrumbs in the main app rendering component based on the `breadcrumb` property of the current route. In addition, there's a `useBreadcrumb` hook used by the UX app, exploratory view, and cases. The conflict between these was creating situations where neither would work and the breadcrumbs would just show "Kibana". Remove the breadcrumb properties from the routes and the main app breadcrumb handling and just use `useBreadcrumb` on all pages. Fixes elastic#102131.

smith · 2021-06-16T05:01:59Z

x-pack/plugins/observability/public/application/index.tsx

 */

 import { i18n } from '@kbn/i18n';
-import React, { MouseEvent, useEffect } from 'react';


Sorting imports here.

smith · 2021-06-16T05:03:50Z

x-pack/plugins/translations/translations/ja-JP.json

    "xpack.monitoring.updateLicenseButtonLabel": "ライセンスを更新",
    "xpack.monitoring.updateLicenseTitle": "ライセンスの更新",
    "xpack.monitoring.useAvailableLicenseDescription": "すでに新しいライセンスがある場合は、今すぐアップロードしてください。",
-    "xpack.observability.alerts.breadcrumb": "アラート",


@Bamieh most of these still exist but have been replaced by keys that more closely match Kibana i18n guidelines.

smith · 2021-06-16T05:09:08Z

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

-    href,
-  };
-};
-export const casesBreadcrumbs = {


These were moved into pages/cases/links.ts.

sorenlouv · 2021-06-16T08:25:06Z

x-pack/plugins/observability/public/hooks/use_breadcrumbs.test.tsx

+      expect(setTitle).toHaveBeenCalledWith(['Two', 'One', 'Observability']);
+    });
+  });
+});


Thanks for adding tests!

sorenlouv · 2021-06-16T08:26:08Z

x-pack/plugins/observability/public/pages/alerts/index.tsx

+        defaultMessage: 'Alerts',
+      }),
+    },
+  ]);


This looks like a good way to set breadcrumbs. Should we do something similar for APM?

Btw. I'm wondering if Kibana should provide something like this. Seems odd that every plugin reinvents the wheel.

Agreed, if this can be moved up the hierarchy, we had this hook originally in uptime, and we moved it here, to be used in UX app and other places.

I think we can use this mostly as-is in APM or other places (it's currently used in UX as Shahzad mentioned.)

There's ongoing discussion in elastic/observability-design#53.

sorenlouv

lgtm

shahzad31

Looks Good !!

kibanamachine · 2021-06-16T14:07:20Z

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id	before	after	diff
`observability`	451.3KB	450.9KB	-463.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`observability`	52.7KB	52.5KB	-280.0B

History

💔 Build #131568 failed 59196b6

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

stephmilovic · 2021-06-16T16:39:37Z

I'm still seeing the bug where the breadcrumbs go to "Kibana" on the first navigation click within Cases. The second time behaves as expected:

smith · 2021-06-16T17:01:50Z

@stephmilovic you're right wtf? I'll check it out.

stephmilovic

Sure enough, the bug was my fault 😆 Got it fixed here: #102429

Thanks for your help with this @smith, LGTM!

Previously the observability plugin set the page title and breadcrumbs in the main app rendering component based on the `breadcrumb` property of the current route. In addition, there's a `useBreadcrumb` hook used by the UX app, exploratory view, and cases. The conflict between these was creating situations where neither would work and the breadcrumbs would just show "Kibana". Remove the breadcrumb properties from the routes and the main app breadcrumb handling and just use `useBreadcrumb` on all pages. Fixes elastic#102131.

kibanamachine · 2021-06-17T05:30:57Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

Previously the observability plugin set the page title and breadcrumbs in the main app rendering component based on the `breadcrumb` property of the current route. In addition, there's a `useBreadcrumb` hook used by the UX app, exploratory view, and cases. The conflict between these was creating situations where neither would work and the breadcrumbs would just show "Kibana". Remove the breadcrumb properties from the routes and the main app breadcrumb handling and just use `useBreadcrumb` on all pages. Fixes #102131. Co-authored-by: Nathan L Smith <nathan.smith@elastic.co>

…egrations-to-global-search * 'master' of github.com:elastic/kibana: (46 commits) [Lens] Add some more documentation for dynamic coloring (elastic#101369) hide not searchable results when no term (elastic#102401) [Lens] Fix Formula functional test with multiple suggestions (elastic#102378) Fix trusted apps modified by field displayed as a date field (elastic#102377) [Lens] Docs for time shift (elastic#102048) update readme of logs-metrics-ui (elastic#101968) Refactor observability plugin breadcrumbs (elastic#102290) [Index Patterns] Move rollup config to index pattern management v2 (elastic#102285) [Security Solution][Endpoint] Isolate Action should only be available to Platinum+ licenses (elastic#102374) [build] Updates Ironbank templates (elastic#102407) Update security best practices document (elastic#100814) [Enterprise Search] Set up initial KibanaPageTemplate (elastic#102170) [Reporting/Docs] Add section to troubleshooting guide to explain the StatusCodeError logs (elastic#102278) [DOCS] Updating Elastic Security Overview topic (elastic#101922) [Uptime] refactor Synthetics Integration package UI (elastic#102080) [Task Manager] Log at different levels based on the state (elastic#101751) [APM] Fixing time comparison types (elastic#101423) [RAC] Update alert documents in lifecycle rule type helper (elastic#101598) [ML] Functional tests - fix and re-activate alerting flyout test (elastic#102368) [Reporting] remove unused reference to path.data config (elastic#102267) ... # Conflicts: # x-pack/plugins/fleet/kibana.json

smith requested review from a team, shahzad31 and stephmilovic June 16, 2021 04:55

i18n fix

630b52d

smith commented Jun 16, 2021

View reviewed changes

Make i18n keys a bit shorter

db2561d

smith commented Jun 16, 2021

View reviewed changes

Only call addClickHandlers once

59196b6

sorenlouv reviewed Jun 16, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master' into nls/obs-bread

eba7482

sorenlouv approved these changes Jun 16, 2021

View reviewed changes

shahzad31 approved these changes Jun 16, 2021

View reviewed changes

stephmilovic approved these changes Jun 16, 2021

View reviewed changes

smith merged commit 6113520 into elastic:master Jun 17, 2021

smith deleted the nls/obs-bread branch June 17, 2021 05:27

kibanamachine mentioned this pull request Jun 17, 2021

[7.x] Refactor observability plugin breadcrumbs (#102290) #102447

Merged

Conversation

smith commented Jun 16, 2021

Uh oh!

smith Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

smith Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

smith Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

sorenlouv Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

sorenlouv Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

shahzad31 Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

smith Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

sorenlouv left a comment

Choose a reason for hiding this comment

Uh oh!

shahzad31 left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jun 16, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

stephmilovic commented Jun 16, 2021

Uh oh!

smith commented Jun 16, 2021

Uh oh!

stephmilovic left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jun 17, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants