Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dashboard Usability] Unified panel options pane #148301

Merged
merged 39 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
b98a4ab
Replace panel title modal with flyout
nickpeihl Dec 27, 2022
8270120
Add custom time to panel editor
nickpeihl Dec 29, 2022
5fe7c32
[CI] Auto-commit changed files from 'node scripts/ts_project_linter -…
kibanamachine Jan 3, 2023
a118d9a
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine Jan 3, 2023
ecc6c9c
Merge remote-tracking branch 'upstream/main' into unified-options-pane
nickpeihl Jan 3, 2023
3873430
Add time range to edit panel settings
nickpeihl Jan 9, 2023
f1b82c0
Merge remote-tracking branch 'upstream/main' into unified-options-pane
nickpeihl Jan 9, 2023
c6f3b65
Merge remote-tracking branch 'refs/remotes/origin/unified-options-pan…
nickpeihl Jan 9, 2023
fabc23f
[CI] Auto-commit changed files from 'node scripts/ts_project_linter -…
kibanamachine Jan 9, 2023
bb76266
Dedupe exported method
nickpeihl Jan 11, 2023
676c8cd
Fix data-test-subj name
nickpeihl Jan 19, 2023
5792aaa
Create DashboardCustomizePanelProvider for functional tests
nickpeihl Jan 19, 2023
22ffadd
Merge remote-tracking branch 'upstream/main' into unified-options-pane
nickpeihl Jan 19, 2023
6ce97fc
Merge branch 'unified-options-pane' of https://github.com/nickpeihl/k…
nickpeihl Jan 19, 2023
4b3fc93
Add missing module
nickpeihl Jan 19, 2023
a100c62
Fix i18n namespace
nickpeihl Jan 19, 2023
5125c8d
Remove unneeded DashboardPanelTimeRangeProvider in functional tests
nickpeihl Jan 19, 2023
e895508
Merge remote-tracking branch 'upstream/main' into unified-options-pane
nickpeihl Jan 23, 2023
8d6929f
Fix reset description UX
nickpeihl Jan 23, 2023
69c562a
Title should be blank string if undefined
nickpeihl Jan 23, 2023
b48aef4
Use time range embeddable in customize panel test
nickpeihl Jan 24, 2023
980aaa9
Merge remote-tracking branch 'upstream/main' into unified-options-pane
nickpeihl Jan 25, 2023
a051bdb
Unskip test
nickpeihl Jan 25, 2023
b9c1a8d
Merge branch 'main' into unified-options-pane
stratoula Jan 26, 2023
761b558
Add functional tests for panel custom time ranges
nickpeihl Jan 26, 2023
3b67edb
Fix type
nickpeihl Jan 27, 2023
286478d
Remove old unit test
nickpeihl Jan 27, 2023
6b8d436
Merge remote-tracking branch 'refs/remotes/origin/unified-options-pan…
nickpeihl Jan 27, 2023
ae7d063
Review feedback
nickpeihl Jan 27, 2023
10bda3c
Fix time range badges in Canvas
nickpeihl Jan 30, 2023
cbd4ccc
Documentation updates
nickpeihl Jan 30, 2023
e736261
Merge remote-tracking branch 'upstream/main' into unified-options-pane
nickpeihl Jan 30, 2023
49ba0a4
Allow panel time range to be set in View mode
nickpeihl Jan 31, 2023
3731bc7
Fix i18n translations
nickpeihl Jan 31, 2023
284078d
Merge remote-tracking branch 'upstream/main' into unified-options-pane
nickpeihl Jan 31, 2023
3c6d0fe
Fix malformed import
nickpeihl Jan 31, 2023
491361e
Merge branch 'main' into unified-options-pane
nickpeihl Feb 1, 2023
6aa6987
close customize panel overlay when dashboard is destroyed
ThomThomson Feb 2, 2023
de19d09
Review feedback
nickpeihl Feb 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions docs/user/canvas.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,7 @@ Add a panel that you saved in *Visualize Library* to your workpad.

* *Edit Visualization* — Opens the visualization editor so that you can edit the panel.

* *Edit panel title* — Allows you to change the panel title.

* *Customize time range* — Allows you to change the time filter dedicated to the panel.
* *Edit panel settings* — Allows you to change the panel title, panel description, and panel time range.

* *Inspect* — Allows you to drill down into the panel data.

Expand Down
8 changes: 5 additions & 3 deletions docs/user/dashboard/dashboard.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ To make changes to the panel, use the panel menu options.
+
To make changes without changing the original version, open the panel menu, then click *More > Unlink from library*.

* *Edit panel title* — Opens the *Customize panel* window to change the *Panel title*.
* *Edit panel settings* — Opens the *Edit panel settings* window to change the *Panel title*, *Panel description*, and *Panel time range*.

* *More > Replace panel* — Opens the *Visualize Library* so you can select a new panel to replace the existing panel.

Expand Down Expand Up @@ -341,9 +341,11 @@ For more information about {kib} and {es} filters, refer to <<kibana-concepts-an

To apply a panel-level time filter:

. Open the panel menu, then select *More > Customize time range*.
. Open the panel menu, then select *More > Edit panel settings*.

. Enter the time range you want to view, then click *Add to panel*.
. Toggle the switch labelled *Apply a custom time range*.

. Enter the time range you want to view, then click *Save*.

[float]
[[apply-design-options]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ image::images/lens_lineChartMetricOverTimeBottomAxis_8.3.png[Bottom axis menu]

Since you removed the axis labels, add a panel title:

. Open the panel menu, then select *Edit panel title*.
. Open the panel menu, then select *Edit panel settings*.

. In the *Panel title* field, enter `Median of bytes`, then click *Save*.
+
Expand Down Expand Up @@ -245,7 +245,7 @@ image::images/lens_pieChartCompareSubsetOfDocs_7.16.png[Pie chart that compares

Add a panel title:

. Open the panel menu, then select *Edit panel title*.
. Open the panel menu, then select *Edit panel settings*.

. In the *Panel title* field, enter `Sum of bytes from large requests`, then click *Save*.

Expand Down Expand Up @@ -278,7 +278,7 @@ image::images/lens_barChartDistributionOfNumberField_7.16.png[Bar chart that dis

Add a panel title:

. Open the panel menu, then select *Edit panel title*.
. Open the panel menu, then select *Edit panel settings*.

. In the *Panel title* field, enter `Website traffic`, then click *Save*.

Expand Down Expand Up @@ -342,7 +342,7 @@ image::images/lens_treemapMultiLevelChart_7.16.png[Treemap visualization]

Add a panel title:

. Open the panel menu, then select *Edit panel title*.
. Open the panel menu, then select *Edit panel settings*.

. In the *Panel title* field, enter `Page views by location and referrer`, then click *Save*.

Expand Down Expand Up @@ -376,4 +376,4 @@ Now that you have a complete overview of your web server data, save the dashboar

. Select *Store time with dashboard*.

. Click *Save*.
. Click *Save*.
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ export class SavedSearchEmbeddable
initialInput,
{
defaultTitle: savedSearch.title,
defaultDescription: savedSearch.description,
editUrl,
editPath,
editApp: 'discover',
Expand Down Expand Up @@ -595,10 +596,6 @@ export class SavedSearchEmbeddable
return this.inspectorAdapters;
}

public getDescription() {
return this.savedSearch.description;
}

/**
* @returns Local/panel-level array of filters for Saved Search embeddable
*/
Expand Down
1 change: 1 addition & 0 deletions src/plugins/embeddable/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export enum ViewMode {
export type EmbeddableInput = {
viewMode?: ViewMode;
title?: string;
description?: string;
/**
* Note this is not a saved object id. It is used to uniquely identify this
* Embeddable instance from others (e.g. inside a container). It's possible to
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/embeddable/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"githubTeam": "kibana-app-services"
},
"description": "Adds embeddables service to Kibana",
"requiredPlugins": ["inspector", "uiActions"],
"requiredPlugins": ["data", "inspector", "uiActions"],
"extraPublicDirs": ["common"],
"requiredBundles": ["savedObjects", "kibanaReact", "kibanaUtils"]
}
13 changes: 13 additions & 0 deletions src/plugins/embeddable/public/lib/embeddables/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

? ''
: input.description === undefined
? output.defaultDescription
: input.description;
}
export abstract class Embeddable<
TEmbeddableInput extends EmbeddableInput = EmbeddableInput,
TEmbeddableOutput extends EmbeddableOutput = EmbeddableOutput,
Expand Down Expand Up @@ -61,6 +68,7 @@ export abstract class Embeddable<

this.output = {
title: getPanelTitle(input, output),
description: getPanelDescription(input, output),
...(this.reportsEmbeddableLoad()
? {}
: {
Expand Down Expand Up @@ -187,6 +195,10 @@ export abstract class Embeddable<
return this.output.title || '';
}

public getDescription(): string {
return this.output.description || '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

}

/**
* Returns the top most parent embeddable, or itself if this embeddable
* is not within a parent.
Expand Down Expand Up @@ -283,6 +295,7 @@ export abstract class Embeddable<
this.inputSubject.next(newInput);
this.updateOutput({
title: getPanelTitle(this.input, this.output),
description: getPanelDescription(this.input, this.output),
} as Partial<TEmbeddableOutput>);
if (oldLastReloadRequestTime !== newInput.lastReloadRequestTime) {
this.reload();
Expand Down
7 changes: 7 additions & 0 deletions src/plugins/embeddable/public/lib/embeddables/i_embeddable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ export interface EmbeddableOutput {
editApp?: string;
editPath?: string;
defaultTitle?: string;
defaultDescription?: string;
title?: string;
description?: string;
editable?: boolean;
// Whether the embeddable can be edited inline by re-requesting the explicit input from the user
editableWithExplicitInput?: boolean;
Expand Down Expand Up @@ -166,6 +168,11 @@ export interface IEmbeddable<
*/
getTitle(): string | undefined;

/**
* Returns the description of this embeddable.
*/
getDescription(): string | undefined;

/**
* Returns the top most parent embeddable, or itself if this embeddable
* is not within a parent.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ test('Runs customize panel action on title click when in edit mode', async () =>
...s,
universalActions: {
...s.universalActions,
customizePanelTitle: { execute: titleExecute, isCompatible: jest.fn() },
customizePanel: { execute: titleExecute, isCompatible: jest.fn() },
},
}));

Expand Down
48 changes: 16 additions & 32 deletions src/plugins/embeddable/public/lib/panel/embeddable_panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ import classNames from 'classnames';
import React, { ReactNode } from 'react';
import { Subscription } from 'rxjs';
import deepEqual from 'fast-deep-equal';
import { CoreStart, OverlayStart, ThemeServiceStart } from '@kbn/core/public';
import { toMountPoint } from '@kbn/kibana-react-plugin/public';
import { CoreStart, ThemeServiceStart } from '@kbn/core/public';
import { isPromise } from '@kbn/std';
import { UsageCollectionStart } from '@kbn/usage-collection-plugin/public';
import { MaybePromise } from '@kbn/utility-types';
Expand All @@ -43,13 +42,12 @@ import { ViewMode } from '../types';
import { EmbeddablePanelError } from './embeddable_panel_error';
import { RemovePanelAction } from './panel_header/panel_actions';
import { AddPanelAction } from './panel_header/panel_actions/add_panel/add_panel_action';
import { CustomizePanelTitleAction } from './panel_header/panel_actions/customize_title/customize_panel_action';
import { CustomizePanelAction } from './panel_header/panel_actions/customize_panel/customize_panel_action';
import { PanelHeader } from './panel_header/panel_header';
import { InspectPanelAction } from './panel_header/panel_actions/inspect_panel_action';
import { EditPanelAction } from '../actions';
import { CustomizePanelModal } from './panel_header/panel_actions/customize_title/customize_panel_modal';
import { EmbeddableStart } from '../../plugin';
import { EmbeddableStateTransfer, isSelfStyledEmbeddable } from '..';
import { EmbeddableStateTransfer, isSelfStyledEmbeddable, CommonlyUsedRange } from '..';

const sortByOrderField = (
{ order: orderA }: { order?: number },
Expand Down Expand Up @@ -85,6 +83,8 @@ interface Props {
getActions?: UiActionsService['getTriggerCompatibleActions'];
getEmbeddableFactory?: EmbeddableStart['getEmbeddableFactory'];
getAllEmbeddableFactories?: EmbeddableStart['getEmbeddableFactories'];
dateFormat?: string;
commonlyUsedRanges?: CommonlyUsedRange[];
overlays?: CoreStart['overlays'];
notifications?: CoreStart['notifications'];
application?: CoreStart['application'];
Expand Down Expand Up @@ -121,7 +121,7 @@ interface InspectorPanelAction {
}

interface BasePanelActions {
customizePanelTitle: CustomizePanelTitleAction;
customizePanel: CustomizePanelAction;
addPanel: AddPanelAction;
inspectPanel: InspectPanelAction;
removePanel: RemovePanelAction;
Expand Down Expand Up @@ -281,6 +281,7 @@ export class EmbeddablePanel extends React.Component<Props, State> {
if (this.state.error) contentAttrs['data-error'] = true;

const title = this.props.embeddable.getTitle();
const description = this.props.embeddable.getDescription();
const headerId = this.generateId();

const selfStyledOptions = isSelfStyledEmbeddable(this.props.embeddable)
Expand All @@ -302,13 +303,14 @@ export class EmbeddablePanel extends React.Component<Props, State> {
getActionContextMenuPanel={this.getActionContextMenuPanel}
hidePanelTitle={this.state.hidePanelTitle || !!selfStyledOptions?.hideTitle}
isViewMode={viewOnlyMode}
customizeTitle={
'customizePanelTitle' in this.state.universalActions
? this.state.universalActions.customizePanelTitle
customizePanel={
'customizePanel' in this.state.universalActions
? this.state.universalActions.customizePanel
: undefined
}
closeContextMenu={this.state.closeContextMenu}
title={title}
description={description}
index={this.props.index}
badges={this.state.badges}
notifications={this.state.notifications}
Expand Down Expand Up @@ -397,34 +399,16 @@ export class EmbeddablePanel extends React.Component<Props, State> {
) {
return actions;
}
const createGetUserData = (overlays: OverlayStart, theme: ThemeServiceStart) =>
async function getUserData(context: { embeddable: IEmbeddable }) {
return new Promise<{ title: string | undefined; hideTitle?: boolean }>((resolve) => {
const session = overlays.openModal(
toMountPoint(
<CustomizePanelModal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

embeddable={context.embeddable}
updateTitle={(title, hideTitle) => {
session.close();
resolve({ title, hideTitle });
}}
cancel={() => session.close()}
/>,
{ theme$: theme.theme$ }
),
{
'data-test-subj': 'customizePanel',
}
);
});
};

// Universal actions are exposed on the context menu for every embeddable, they bypass the trigger
// registry.
return {
...actions,
customizePanelTitle: new CustomizePanelTitleAction(
createGetUserData(this.props.overlays, this.props.theme)
customizePanel: new CustomizePanelAction(
this.props.overlays,
this.props.theme,
this.props.commonlyUsedRanges,
this.props.dateFormat
),
addPanel: new AddPanelAction(
this.props.getEmbeddableFactory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@
*/

import { canInheritTimeRange } from './can_inherit_time_range';
import { HelloWorldContainer } from '@kbn/embeddable-plugin/public/lib/test_samples';
import { HelloWorldEmbeddable } from '@kbn/embeddable-plugin/public/tests/fixtures';
import { TimeRangeEmbeddable, TimeRangeContainer } from './test_helpers';
import {
HelloWorldContainer,
TimeRangeContainer,
TimeRangeEmbeddable,
} from '../../../../test_samples';
import { HelloWorldEmbeddable } from '../../../../../tests/fixtures';

test('canInheritTimeRange returns false if embeddable is inside container without a time range', () => {
const embeddable = new TimeRangeEmbeddable(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* Side Public License, v 1.
*/

import { Embeddable, IContainer, ContainerInput } from '@kbn/embeddable-plugin/public';
import type { TimeRange } from '@kbn/es-query';
import { TimeRangeInput } from './custom_time_range_action';
import { Embeddable, IContainer, ContainerInput } from '../../../../..';
import { TimeRangeInput } from './customize_panel_action';

interface ContainerTimeRangeInput extends ContainerInput<TimeRangeInput> {
timeRange: TimeRange;
Expand Down
Loading