[Dashboard Usability] Unified panel options pane #148301

nickpeihl · 2023-01-03T14:33:55Z

Summary

Adds a panel settings flyover to customize the title, description, and time range for panels in a dashboard. Replaces the "Customize panel title" and "Customize time range" options in the panel context menu with a "Customize panel setting" option.

It is not currently possible to provide description for by-value panels in a dashboard. By-reference panels can have a description, but it must be defined in the saved object. To enable support for custom descriptions on both by-value and by-reference panels this PR adds description and defaultDescription keys to the EmbeddableOutput interface and a getDescription method to the IEmbeddable interface.

CustomizePanelTitleAction and CustomTimeRangeAction have been replaced by CustomizePanelAction. CustomTimeRangeAction was moved from the ui_actions_enhanced plugin to the embeddable plugin. CustomTimeRangeAction was originally Basic licensed, but because of the license change from OSS to SSPL, we no longer need maintain these in separate codebases.

Flaky test runner

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
Documentation was added for features that require explanation or tutorials
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

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

For maintainers

This was checked for breaking API changes and was labeled appropriately

…-fix'

…e' into unified-options-pane

…-fix'

…ibana into unified-options-pane

Unlike EuiTextField, EuiTextArea cannot receive undefined for the value prop. So we must coerce to a blank string.

nickpeihl · 2023-01-30T15:14:20Z

Looks like I broke panel time ranges for embeddables in Canvas. Working on a fix.

ThomThomson

Everything LGTM! Ran this locally and tested on a number of dashboards editing or not editing the panel titles and descriptions of a variety of embeddable types both by reference and by value.

Also looked through the code, and the changes are looking great, left a few very small nits.

Appreciate the FTR results - looks like it failed once due to an unrelated flaky test?

One thing - I'm not sure if this is just my computer, but I was having a bug where the unified options pane would animate open twice each time it was clicked:

ThomThomson · 2023-02-01T23:53:35Z

src/plugins/embeddable/public/lib/embeddables/embeddable.tsx

@@ -21,6 +21,13 @@ import { genericEmbeddableInputIsEqual, omitGenericEmbeddableInput } from './dif
 function getPanelTitle(input: EmbeddableInput, output: EmbeddableOutput) {
  return input.hidePanelTitles ? '' : input.title === undefined ? output.defaultTitle : input.title;
 }
+function getPanelDescription(input: EmbeddableInput, output: EmbeddableOutput) {
+  return input.hidePanelTitles


Is there a way to make this slightly more understandable? Double ternaries can sometimes be head-scratching.

ThomThomson · 2023-02-01T23:55:01Z

src/plugins/embeddable/public/lib/embeddables/embeddable.tsx

@@ -187,6 +195,10 @@ export abstract class Embeddable<
    return this.output.title || '';
  }

+  public getDescription(): string {
+    return this.output.description || '';


super tiny nit: Would using ?? be more correct here? With this code, if the output.description was already a blank string it would fail the check and return the fallback. The behaviour is completely the same - which is why this is a super tiny nit!

ThomThomson · 2023-02-01T23:56:13Z

src/plugins/embeddable/public/lib/panel/embeddable_panel.tsx

-        return new Promise<{ title: string | undefined; hideTitle?: boolean }>((resolve) => {
-          const session = overlays.openModal(
-            toMountPoint(
-              <CustomizePanelModal


Wow, I didn't realise this code lived right inside the embeddable panel before, nice clean up!

ThomThomson · 2023-02-01T23:59:24Z

...dable/public/lib/panel/panel_header/panel_actions/customize_panel/customize_panel_editor.tsx

+    onClose();
+  };
+
+  const renderCustomTitleComponent = () => {


Eventually we could split these up into separate files, might be a good follow-up PR, especially as this window is only expected to grow!

Yup. Happy to work on a follow-up PR for this. 👍

KOTungseth · 2023-02-02T15:17:41Z

THIS is amazing!

I have a few suggestions for the UI copy.

For Controls, the settings page header is Control settings. Can we simply the panel settings header to Panel settings instead of Edit panel settings?

Since the header includes the word panel, can we remove panel from the subsequent fields? (This is common in other save modals)

Show panel title -> Show title or Display title

Panel title -> Title

Panel description -> Description

Apply custom time range to panel -> Apply custom time range

Panel time range -> Custom time range

cc: @gchaps @florent-leborgne

elasticmachine · 2023-02-02T20:05:56Z

Pinging @elastic/kibana-presentation (Team:Presentation)

kibana-ci · 2023-02-02T20:54:45Z

💚 Build Succeeded

Buildkite Build
Commit: de19d09

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`embeddable`	75	78	+3
`uiActionsEnhanced`	155	151	-4
total			-1

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`	427	430	+3
`lens`	599	598	-1
`maps`	267	266	-1
`visualizations`	771	770	-1
total			-0

Async chunks

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

id	before	after	diff
`dashboard`	364.6KB	364.6KB	+20.0B
`discover`	382.9KB	382.9KB	-20.0B
`lens`	1.3MB	1.3MB	+84.0B
`maps`	2.7MB	2.7MB	-122.0B
`ml`	3.4MB	3.4MB	+132.0B
`uiActionsEnhanced`	135.1KB	131.9KB	-3.1KB
`visualizations`	258.4KB	258.4KB	-12.0B
total			-3.1KB

Page load bundle

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

id	before	after	diff
`embeddable`	69.2KB	73.4KB	+4.3KB
`uiActionsEnhanced`	20.3KB	16.8KB	-3.4KB
total			+876.0B

Unknown metric groups

API count

id	before	after	diff
`embeddable`	528	532	+4
`lens`	694	693	-1
`maps`	268	267	-1
`visualizations`	801	800	-1
total			+1

async chunk count

id	before	after	diff
`uiActionsEnhanced`	5	4	-1

History

💚 Build #105223 succeeded 6aa6987
💚 Build #104829 succeeded 491361e
💛 Build #104278 was flaky 3c6d0fe
💔 Build #104229 failed 49ba0a4
💔 Build #103878 failed e736261
💔 Build #103542 failed ae7d063

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

## Summary Closes elastic#148563 After this [PR]( elastic#148301) being merged, it will be easier to add the description info icon and the custom data range to the new metric. For this reason we decided to also allow the panel title for now. On the next minors we are going to hide it from the panel and display all the aforementioned info on the viz title. <img width="988" alt="image" src="https://user-images.githubusercontent.com/17003240/215037834-0f556673-8628-484e-aa32-c34188fc7064.png"> ### Checklist - [x] [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

nickpeihl and others added 23 commits December 27, 2022 14:53

Replace panel title modal with flyout

b98a4ab

Add custom time to panel editor

8270120

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

5fe7c32

…-fix'

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

a118d9a

Merge remote-tracking branch 'upstream/main' into unified-options-pane

ecc6c9c

Add time range to edit panel settings

3873430

Merge remote-tracking branch 'upstream/main' into unified-options-pane

f1b82c0

Merge remote-tracking branch 'refs/remotes/origin/unified-options-pan…

c6f3b65

…e' into unified-options-pane

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

fabc23f

…-fix'

Dedupe exported method

bb76266

Fix data-test-subj name

676c8cd

Create DashboardCustomizePanelProvider for functional tests

5792aaa

Merge remote-tracking branch 'upstream/main' into unified-options-pane

22ffadd

Merge branch 'unified-options-pane' of https://github.com/nickpeihl/k…

6ce97fc

…ibana into unified-options-pane

Add missing module

4b3fc93

Fix i18n namespace

a100c62

Remove unneeded DashboardPanelTimeRangeProvider in functional tests

5125c8d

Merge remote-tracking branch 'upstream/main' into unified-options-pane

e895508

Fix reset description UX

8d6929f

Unlike EuiTextField, EuiTextArea cannot receive undefined for the value prop. So we must coerce to a blank string.

Title should be blank string if undefined

69c562a

Use time range embeddable in customize panel test

b48aef4

Merge remote-tracking branch 'upstream/main' into unified-options-pane

980aaa9

Unskip test

a051bdb

nickpeihl marked this pull request as ready for review January 25, 2023 18:29

nickpeihl requested review from a team as code owners January 25, 2023 18:29

nickpeihl added 7 commits January 30, 2023 12:04

Fix time range badges in Canvas

10bda3c

Documentation updates

cbd4ccc

Merge remote-tracking branch 'upstream/main' into unified-options-pane

e736261

Allow panel time range to be set in View mode

49ba0a4

Fix i18n translations

3731bc7

Merge remote-tracking branch 'upstream/main' into unified-options-pane

284078d

Fix malformed import

3c6d0fe

nickpeihl added release_note:enhancement v8.7.0 labels Jan 31, 2023

Merge branch 'main' into unified-options-pane

491361e

ThomThomson approved these changes Feb 2, 2023

View reviewed changes

ThomThomson and others added 2 commits February 2, 2023 12:30

close customize panel overlay when dashboard is destroyed

6aa6987

Review feedback

de19d09

nickpeihl added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Feb 2, 2023

rshen91 approved these changes Feb 2, 2023

View reviewed changes

nickpeihl merged commit ace2c30 into elastic:main Feb 2, 2023

kibanamachine added the backport:skip This commit does not require backporting label Feb 2, 2023

ThomThomson mentioned this pull request Feb 6, 2023

unlinked visualizations from the Library still include pop-up info #131296

Closed

nickpeihl mentioned this pull request Feb 10, 2023

[Dashboard Usability] Unified Dashboard Options #144532

Closed

ThomThomson mentioned this pull request Mar 27, 2023

Allow adding notes from the dashboard while changing panel title #83743

Closed

asmith-elastic mentioned this pull request Jun 2, 2023

Toggling Show title from the Panel Settings toggles both the Title and Description #158977

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dashboard Usability] Unified panel options pane #148301

[Dashboard Usability] Unified panel options pane #148301

nickpeihl commented Jan 3, 2023 •

edited

Loading

nickpeihl commented Jan 30, 2023

ThomThomson left a comment •

edited

Loading

ThomThomson Feb 1, 2023

ThomThomson Feb 1, 2023

ThomThomson Feb 1, 2023

ThomThomson Feb 1, 2023

nickpeihl Feb 2, 2023

KOTungseth commented Feb 2, 2023

elasticmachine commented Feb 2, 2023

kibana-ci commented Feb 2, 2023

API count

async chunk count

[Dashboard Usability] Unified panel options pane #148301

[Dashboard Usability] Unified panel options pane #148301

Conversation

nickpeihl commented Jan 3, 2023 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

nickpeihl commented Jan 30, 2023

ThomThomson left a comment • edited Loading

Choose a reason for hiding this comment

ThomThomson Feb 1, 2023

Choose a reason for hiding this comment

ThomThomson Feb 1, 2023

Choose a reason for hiding this comment

ThomThomson Feb 1, 2023

Choose a reason for hiding this comment

ThomThomson Feb 1, 2023

Choose a reason for hiding this comment

nickpeihl Feb 2, 2023

Choose a reason for hiding this comment

KOTungseth commented Feb 2, 2023

elasticmachine commented Feb 2, 2023

kibana-ci commented Feb 2, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

async chunk count

History

nickpeihl commented Jan 3, 2023 •

edited

Loading

ThomThomson left a comment •

edited

Loading