Redesign the "Add Panel" Experience #183764

eokoneyo · 2024-05-17T16:35:47Z

Summary

This PR introduces changes to the dashboard add panel selection functionality, so that panel selection would now happen from within a flyout, and as such panels are now grouped together logically.

With this implementation any panel that is intended to show up within this new flyout is required to have either been registered leveraging the ui action trigger ADD_PANEL_TRIGGER and have it's grouping value defined or belong to a subset of visualization types (PROMOTED, TOOLS, and LEGACY) that would automatically get grouped.

It's worth pointing out that because we can't control the order at which UI actions gets registered, we won't always get the the panel groups in the same order, for this specific reason ~~a new optional property (placementPriority) has been added in~~ the property order is now leveraged such that it allows a user registering a UI action define a relative weight for where they'd like their group to show up. All registered actions would be rendered in descending order considering all order defined, in the case where no order is defined 0 is assumed for the group. In addition an action which is registered without a group, would automatically get assigned into a default group titled "Other".

The search implemented within the add panel is rudimentary, checking if the group titles and group item titles contain the input character; when a group title is matched the entire group is remains highlighted, in the case that the group isn't matched and it's just the group item, only said item is highlighted within it's group.

Visuals

Default view

Search match view

P.S.

This changes also includes changes to the display of certain panels;

ML group has a new title i.e. Machine Learning and Analytics
In serverless, the observability panels (SLO*) only shows as a selection choice in the observability project type.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support

Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

eokoneyo · 2024-05-24T09:46:16Z

src/plugins/dashboard/public/dashboard_actions/add_agg_vis_action.ts

+  }
+
+  public async isCompatible() {
+    return true;


Need further clarification about this, I'm assuming it would be compatible since this action only calls a function to display the selection modal.

I would check that at least apiCanAddNewPanel(embeddable) is true to make sure the embeddable is capable of adding new panels.

FYI you would be checking to ensure the parent API is compatible - so something like apiHasParentApi(embeddable) && apiCanAddNewPanel(embeddable.parentApi) :)

eokoneyo · 2024-05-28T10:55:34Z

/ci

eokoneyo · 2024-05-28T11:26:12Z

/ci

eokoneyo · 2024-05-28T12:53:54Z

/ci

eokoneyo · 2024-05-30T10:31:49Z

/ci

eokoneyo · 2024-05-30T14:13:41Z

/ci

src/plugins/ui_actions/public/context_menu/build_eui_context_menu_panels.tsx

eokoneyo · 2024-05-30T14:51:13Z

x-pack/plugins/observability_solution/slo/public/ui_actions/index.ts

+
+  if (!isServerless) {
+    // only register these actions in stateful kibana
+    uiActions.addTriggerAction(ADD_PANEL_TRIGGER, addOverviewPanelAction);
+    uiActions.addTriggerAction(ADD_PANEL_TRIGGER, addErrorBudgetPanelAction);
+    uiActions.addTriggerAction(ADD_PANEL_TRIGGER, addAlertsPanelAction);
+  }


exclude the registering of observability SLO actions in serverless.

@eokoneyo Can you elaborate more why you had to exclude these actions on serverless?

@eokoneyo I just noticed that this file is outdated and you indeed register these actions on stateful and observability serverless. In the beginning I thought you were disabling on serverless just the SLO edit actions, but that was my misunderstanding.

eokoneyo · 2024-05-30T17:56:06Z

/ci

eokoneyo · 2024-05-31T14:00:17Z

/ci

eokoneyo · 2024-05-31T14:35:44Z

/ci

cqliu1 · 2024-06-21T21:52:03Z

UX change: I changed the add panel flyout search behavior to hiding the sections that don't match the search string. This resolves an issue with a failing maps test where the flyout was scrolling to the bottom of the flyout which prevented the maps panel option from being clicked.

Tiny UI change: I added an empty state for when no panel types match your search string.

ThomThomson

A lot of this is looking good! Tested it locally and it also works great - especially the new searching.

I thought that we had removed all of the placementPriority code, but I still see a lot of references to it. I'd really like to have this whole system removed, as the actions already contain an order param.

Is this possible?

ThomThomson · 2024-06-21T22:11:52Z

src/plugins/dashboard/public/dashboard_actions/add_agg_vis_action.ts

+    return isApiCompatible(embeddable);
+  }
+
+  public execute({ embeddable }: EmbeddableApiContext): Promise<void> {


nit: why not use an async function here?

Addressed in 48bef80

ThomThomson · 2024-06-21T22:12:28Z

src/plugins/dashboard/public/dashboard_actions/add_agg_vis_action.ts

+  return apiHasType(api) && api.type === 'dashboard';
+};
+
+export class AddAggVisualizationPanelAction implements Action<EmbeddableApiContext> {


Any reason this action is in the Dashboard plugin and not in the Visualize plugin?

I moved this to the visualizations plugin in 48bef80

ThomThomson · 2024-06-21T22:13:53Z

src/plugins/dashboard/public/dashboard_app/top_nav/add_panel_action_menu_items.ts

+  items: PanelSelectionMenuItem[];
+};
+
+export interface PlacementPriority {


I thought we removed this placementPriority concept in favour of the order key? If so, can we clean these up? If they are still used, I'd like to move away from them because this info should be captured in the normal action types.

edit it looks like these are still used in quite a few places. See top level review comment.

I removed all references to placementPriority in 910065f.

…fix'

ThomThomson

Code looks much better! I tested locally again, and found a very small regression - if you open an agg based editor now and hit cancel you'll end up at /app/dashboard which is a 404 page. I wasn't able to reproduce this in main.

Another small detail we could consider - perhaps in a followup, is that oftentimes the editors need async-loaded code, or take some time to open. This means that the add panel flyout closes immediately, and then the editor for the chart type you selected takes a moment to appear. We could handle this load state more gracefully - this is especially noticeable with the ES|QL editor.

ThomThomson · 2024-06-24T14:26:25Z

src/plugins/embeddable/public/lib/embeddables/common/constants.ts

+
+import { i18n } from '@kbn/i18n';
+
+export const COMMON_EMBEDDABLE_GROUPING = {


nit: can we type these? Maybe UiActionsPresentableGrouping<unknown>?

Updated the type in 5337820

cqliu1 · 2024-06-25T15:58:19Z

I tested locally again, and found a very small regression - if you open an agg based editor now and hit cancel you'll end up at /app/dashboard which is a 404 page. I wasn't able to reproduce this in main.

I was passing the wrong originatingApp. This is fixed with 56a4697.

ThomThomson

Changes LGTM!

Ran it locally on chrome, ensuring that all add panel actions worked as expected. I think it might be worth creating an issue for the ability to wait for an editor to be ready before closing the add panel menu - because it was jarring a few times when the flyout closed and there was a visible delay in opening the next step.

I also looked through the code, and it all seems functional. IMO, there are a lot of ways it could be made more concise or organized, but likely most of this cleanup will happen after the embeddable refactor, once we've removed the legacy factories.

x-pack/plugins/aiops/public/ui_actions/create_change_point_chart.tsx

x-pack/plugins/ml/public/ui_actions/create_swim_lane.tsx

peteharverson · 2024-06-26T17:01:25Z

Noted when testing this for ML that the ML panel types are still shown even when the ML feature is None for the current space. However this is not related to the changes in this PR, and I have raised a separate issue to fix on the ML side - #187007.

kibana-ci · 2024-06-26T18:23:42Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 882b7dd
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-183764-882b7dd0fb0c
Observability Deployment

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`aiops`	631	632	+1
`apm`	1886	1887	+1
`cloudSecurityPosture`	680	681	+1
`dashboard`	539	563	+24
`discover`	938	939	+1
`embeddable`	139	140	+1
`esqlDataGrid`	397	398	+1
`eventAnnotationListing`	617	618	+1
`lens`	1478	1479	+1
`logsExplorer`	598	599	+1
`securitySolution`	5535	5536	+1
`slo`	852	853	+1
`uiActions`	39	40	+1
`visualizations`	437	447	+10
total			+46

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`embeddable`	447	461	+14
`links`	57	58	+1
`uiActions`	103	110	+7
`visualizations`	834	838	+4
total			+26

Async chunks

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

id	before	after	diff
`aiops`	558.7KB	558.8KB	+60.0B
`canvas`	1.1MB	1.1MB	+36.0B
`dashboard`	495.2KB	503.5KB	+8.3KB
`discover`	808.8KB	809.1KB	+254.0B
`imageEmbeddable`	66.9KB	66.9KB	+19.0B
`lens`	1.5MB	1.5MB	+254.0B
`ml`	4.6MB	4.6MB	+939.0B
`securitySolution`	13.7MB	13.7MB	+254.0B
`slo`	868.5KB	868.9KB	+411.0B
`visualizations`	291.1KB	291.3KB	+171.0B
total			+10.6KB

Page load bundle

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

id	before	after	diff
`dashboard`	43.5KB	43.1KB	-425.0B
`embeddable`	70.7KB	71.2KB	+554.0B
`imageEmbeddable`	5.7KB	5.9KB	+207.0B
`infra`	101.8KB	101.9KB	+66.0B
`lens`	49.3KB	49.4KB	+62.0B
`links`	33.8KB	33.9KB	+106.0B
`maps`	54.5KB	54.5KB	+9.0B
`ml`	80.2KB	80.2KB	+14.0B
`slo`	23.1KB	23.1KB	-8.0B
`uiActions`	22.9KB	23.4KB	+441.0B
`visTypeMarkdown`	9.6KB	9.6KB	+9.0B
`visTypeTimeseries`	20.6KB	20.6KB	+7.0B
`visualizations`	61.9KB	63.1KB	+1.2KB
total			+2.2KB

Unknown metric groups

API count

id	before	after	diff
`embeddable`	557	571	+14
`links`	57	58	+1
`uiActions`	149	156	+7
`visualizations`	865	869	+4
total			+26

ESLint disabled line counts

id	before	after	diff
`dashboard`	9	10	+1
`visualizations`	17	18	+1
total			+2

Total ESLint disabled count

id	before	after	diff
`dashboard`	9	10	+1
`visualizations`	19	20	+1
total			+2

History

💛 Build #217971 was flaky 947b2c9
💛 Build #217771 was flaky 79ea2e8
💔 Build #217708 failed 56a4697
💔 Build #217697 failed f78c1be

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

cc @eokoneyo

peteharverson

Change for ML LGTM. Thanks for switching the order of the items @cqliu1

## Summary Follow up to #183764. The labels for the Visualizations and Observability groupings shown in the Add Panel flyout in Dashboard were not being translated. ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

eokoneyo added Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:large Large Level of Effort Project:Dashboard Usability Related to the Dashboard Usability initiative labels May 17, 2024

eokoneyo self-assigned this May 17, 2024

eokoneyo force-pushed the feat/resolve-144418 branch 2 times, most recently from bd2aeab to c564ea6 Compare May 24, 2024 09:44

eokoneyo commented May 24, 2024

View reviewed changes

eokoneyo force-pushed the feat/resolve-144418 branch 4 times, most recently from e3a5e7c to b5180a7 Compare May 28, 2024 08:33

eokoneyo force-pushed the feat/resolve-144418 branch 2 times, most recently from 6d80484 to 50422f5 Compare May 28, 2024 11:25

eokoneyo force-pushed the feat/resolve-144418 branch from 0727cd2 to a4fd21c Compare May 30, 2024 09:57

eokoneyo force-pushed the feat/resolve-144418 branch from a4fd21c to dfe8c0d Compare May 30, 2024 14:12

eokoneyo force-pushed the feat/resolve-144418 branch from dfe8c0d to 8a41342 Compare May 30, 2024 14:35

eokoneyo commented May 30, 2024

View reviewed changes

src/plugins/ui_actions/public/context_menu/build_eui_context_menu_panels.tsx Outdated Show resolved Hide resolved

eokoneyo commented May 30, 2024

View reviewed changes

eokoneyo force-pushed the feat/resolve-144418 branch from 8d75eed to 2c75629 Compare May 30, 2024 17:43

eokoneyo force-pushed the feat/resolve-144418 branch from 2c75629 to 7bf242c Compare May 31, 2024 13:59

eokoneyo force-pushed the feat/resolve-144418 branch from 7bf242c to 5906d1d Compare May 31, 2024 14:35

ThomThomson reviewed Jun 21, 2024

View reviewed changes

cqliu1 and others added 5 commits June 21, 2024 15:49

Remove all references to placementPriority

910065f

Move addAggVisAction to visualizations plugin

48bef80

Remove unused import

be9021e

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

677377e

…fix'

Fix i18n id

3a0c698

ThomThomson reviewed Jun 24, 2024

View reviewed changes

cqliu1 added 6 commits June 24, 2024 10:16

Merge remote-tracking branch 'upstream' into feat/resolve-144418

4ee460f

Fix add agg based vis action originatingApp

de6d165

Add type for COMMON_EMBEDDABLE_GROUPING

5337820

Update compatibility logic for add_agg_vis_action

68c1afa

Merge remote-tracking branch 'upstream' into feat/resolve-144418

f78c1be

Check for appContext in add_agg_vis_action

56a4697

Fix type errors

79ea2e8

ThomThomson approved these changes Jun 26, 2024

View reviewed changes

Merge branch 'main' into feat/resolve-144418

947b2c9

peteharverson reviewed Jun 26, 2024

View reviewed changes

x-pack/plugins/aiops/public/ui_actions/create_change_point_chart.tsx Outdated Show resolved Hide resolved

x-pack/plugins/ml/public/ui_actions/create_swim_lane.tsx Outdated Show resolved Hide resolved

cqliu1 added 2 commits June 26, 2024 10:28

Change ml options order

ca83f0c

Merge branch 'main' into feat/resolve-144418

882b7dd

peteharverson approved these changes Jun 26, 2024

View reviewed changes

cqliu1 merged commit 22e0545 into elastic:main Jun 26, 2024
21 checks passed

kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels Jun 26, 2024

cqliu1 mentioned this pull request Jun 27, 2024

[Dashboard] Add missing translated strings to add panel flyout #187122

Merged

10 tasks

eokoneyo deleted the feat/resolve-144418 branch July 1, 2024 11:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redesign the "Add Panel" Experience #183764

Redesign the "Add Panel" Experience #183764

eokoneyo commented May 17, 2024 •

edited

Loading

eokoneyo May 24, 2024

cqliu1 Jun 4, 2024

Heenawter Jun 4, 2024

eokoneyo commented May 28, 2024

eokoneyo commented May 28, 2024

eokoneyo commented May 28, 2024

eokoneyo commented May 30, 2024

eokoneyo commented May 30, 2024

eokoneyo May 30, 2024

mgiota Jun 19, 2024 •

edited

Loading

mgiota Jun 20, 2024 •

edited

Loading

eokoneyo commented May 30, 2024

eokoneyo commented May 31, 2024

eokoneyo commented May 31, 2024

cqliu1 commented Jun 21, 2024 •

edited

Loading

ThomThomson left a comment

ThomThomson Jun 21, 2024

cqliu1 Jun 21, 2024

ThomThomson Jun 21, 2024

cqliu1 Jun 21, 2024

ThomThomson Jun 21, 2024 •

edited

Loading

cqliu1 Jun 21, 2024

ThomThomson left a comment

ThomThomson Jun 24, 2024

cqliu1 Jun 25, 2024

cqliu1 commented Jun 25, 2024

ThomThomson left a comment

peteharverson commented Jun 26, 2024

kibana-ci commented Jun 26, 2024 •

edited

Loading

API count

ESLint disabled line counts

Total ESLint disabled count

peteharverson left a comment


		import { i18n } from '@kbn/i18n';

		export const COMMON_EMBEDDABLE_GROUPING = {

Redesign the "Add Panel" Experience #183764

Redesign the "Add Panel" Experience #183764

Conversation

eokoneyo commented May 17, 2024 • edited Loading

Summary

Visuals

Default view

Search match view

P.S.

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eokoneyo commented May 28, 2024

eokoneyo commented May 28, 2024

eokoneyo commented May 28, 2024

eokoneyo commented May 30, 2024

eokoneyo commented May 30, 2024

Choose a reason for hiding this comment

mgiota Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

mgiota Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

eokoneyo commented May 30, 2024

eokoneyo commented May 31, 2024

eokoneyo commented May 31, 2024

cqliu1 commented Jun 21, 2024 • edited Loading

ThomThomson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomThomson Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomThomson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cqliu1 commented Jun 25, 2024

ThomThomson left a comment

Choose a reason for hiding this comment

peteharverson commented Jun 26, 2024

kibana-ci commented Jun 26, 2024 • edited Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

peteharverson left a comment

Choose a reason for hiding this comment

eokoneyo commented May 17, 2024 •

edited

Loading

mgiota Jun 19, 2024 •

edited

Loading

mgiota Jun 20, 2024 •

edited

Loading

cqliu1 commented Jun 21, 2024 •

edited

Loading

ThomThomson Jun 21, 2024 •

edited

Loading

kibana-ci commented Jun 26, 2024 •

edited

Loading