From 133016850fabf44f3f527496a67eab5d102443b4 Mon Sep 17 00:00:00 2001 From: Hannah Mudge Date: Thu, 4 Jan 2024 14:39:44 -0700 Subject: [PATCH] [Dashboard Usability] Conditionally auto focus on title input in panel settings flyout (#173777) Closes https://github.com/elastic/kibana/issues/170786 ## Summary Currently, in order to change a panel title, the most efficient way to do this (assuming the dashboard is already in edit mode) is to click on the panel title -> click on the title input in the flyout -> click save at the bottom of the flyout. Notice that this process currently takes **three** clicks, which can start to add up if you need to change multiple titles in one session - so, in order to remove one click from this process, I've made it so that the title input is **auto focused** on when opening the settings flyout through the panel title (and not when it is opened from the context menu). > [!NOTE] > I chose to make this auto-focus behaviour conditional on how the flyout was opened because, from an a11y perspective, it can be jarring to have your focus taken out of the natural element order (as noted in the [EUI docs](https://eui.elastic.co/#/layout/popover#setting-an-initial-focus)). It feels natural that, in order to change the panel title, you click on it - so, auto focusing on the title input makes sense, even when using keyboard controls. However, if you are opening the settings flyout via the context menu, it is **less likely** that the goal is to change the title - so, forcing the focus could feel unnatural in this case. I added tests for this new auto focus behaviour and, since I was interested in learning a bit more about RTL, I also added a few other tests for the dashboard panel components. As part of this, I migrated a few functional tests to unit tests, since this is a faster and more reliable way to test certain rendering conditionals. ### Video https://github.com/elastic/kibana/assets/8698078/229c1303-c81d-46b8-a567-76885361d9fa ### 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 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] 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)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### 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) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../embeddable_panel/embeddable_panel.tsx | 29 +-- .../can_inherit_time_range.ts | 2 +- .../custom_time_range_badge.test.ts | 31 +-- .../custom_time_range_badge.tsx | 12 +- .../customize_panel_action.test.ts | 15 +- .../customize_panel_action.tsx | 114 +--------- .../customize_panel_editor.test.tsx | 200 ++++++++++++++++++ .../customize_panel_editor.tsx | 55 +++-- .../does_inherit_time_range.ts | 4 +- .../filters_details.tsx | 3 + .../open_customize_panel.tsx | 75 +++++++ .../time_range_helpers.ts | 53 +++++ .../embeddable_panel_header.test.tsx | 171 +++++++++++++++ .../panel_header/embeddable_panel_header.tsx | 4 +- .../panel_header/embeddable_panel_title.tsx | 29 ++- src/plugins/embeddable/public/plugin.tsx | 11 +- src/plugins/embeddable/tsconfig.json | 3 +- .../apps/dashboard/group2/panel_time_range.ts | 15 -- .../apps/dashboard/group2/panel_titles.ts | 38 ---- 19 files changed, 601 insertions(+), 263 deletions(-) create mode 100644 src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_editor.test.tsx create mode 100644 src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/open_customize_panel.tsx create mode 100644 src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/time_range_helpers.ts create mode 100644 src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_header.test.tsx diff --git a/src/plugins/embeddable/public/embeddable_panel/embeddable_panel.tsx b/src/plugins/embeddable/public/embeddable_panel/embeddable_panel.tsx index c24383ff9fb18..134b65efc9d86 100644 --- a/src/plugins/embeddable/public/embeddable_panel/embeddable_panel.tsx +++ b/src/plugins/embeddable/public/embeddable_panel/embeddable_panel.tsx @@ -6,20 +6,23 @@ * Side Public License, v 1. */ -import { isNil } from 'lodash'; +import { EuiFlexGroup, EuiFlexItem, EuiPanel, htmlIdGenerator } from '@elastic/eui'; import classNames from 'classnames'; -import { distinct, map } from 'rxjs'; +import { isNil } from 'lodash'; import React, { ReactNode, useEffect, useMemo, useState } from 'react'; -import { EuiFlexGroup, EuiFlexItem, EuiPanel, htmlIdGenerator } from '@elastic/eui'; +import { distinct, map } from 'rxjs'; -import { UI_SETTINGS } from '@kbn/data-plugin/common'; import { PanelLoader } from '@kbn/panel-loader'; +import { core, embeddableStart, inspector } from '../kibana_services'; +import { EmbeddableErrorHandler, EmbeddableOutput, ViewMode } from '../lib'; +import { EmbeddablePanelError } from './embeddable_panel_error'; import { + CustomizePanelAction, EditPanelAction, - RemovePanelAction, InspectPanelAction, - CustomizePanelAction, + RemovePanelAction, } from './panel_actions'; +import { EmbeddablePanelHeader } from './panel_header/embeddable_panel_header'; import { EmbeddablePhase, EmbeddablePhaseEvent, @@ -30,10 +33,6 @@ import { useSelectFromEmbeddableInput, useSelectFromEmbeddableOutput, } from './use_select_from_embeddable'; -import { EmbeddablePanelError } from './embeddable_panel_error'; -import { core, embeddableStart, inspector } from '../kibana_services'; -import { ViewMode, EmbeddableErrorHandler, EmbeddableOutput } from '../lib'; -import { EmbeddablePanelHeader } from './panel_header/embeddable_panel_header'; const getEventStatus = (output: EmbeddableOutput): EmbeddablePhase => { if (!isNil(output.error)) { @@ -61,8 +60,6 @@ export const EmbeddablePanel = (panelProps: UnwrappedEmbeddablePanelProps) => { * bypass the trigger registry. */ const universalActions = useMemo(() => { - const commonlyUsedRanges = core.uiSettings.get(UI_SETTINGS.TIMEPICKER_QUICK_RANGES); - const dateFormat = core.uiSettings.get(UI_SETTINGS.DATE_FORMAT); const stateTransfer = embeddableStart.getStateTransfer(); const editPanel = new EditPanelAction( embeddableStart.getEmbeddableFactory, @@ -71,13 +68,7 @@ export const EmbeddablePanel = (panelProps: UnwrappedEmbeddablePanelProps) => { ); const actions: PanelUniversalActions = { - customizePanel: new CustomizePanelAction( - core.overlays, - core.theme, - editPanel, - commonlyUsedRanges, - dateFormat - ), + customizePanel: new CustomizePanelAction(editPanel), removePanel: new RemovePanelAction(), editPanel, }; diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/can_inherit_time_range.ts b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/can_inherit_time_range.ts index 139933c8d9390..f2c1a3b7a9aac 100644 --- a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/can_inherit_time_range.ts +++ b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/can_inherit_time_range.ts @@ -8,8 +8,8 @@ import type { TimeRange } from '@kbn/es-query'; -import { TimeRangeInput } from './customize_panel_action'; import { Embeddable, IContainer, ContainerInput } from '../../..'; +import { TimeRangeInput } from './time_range_helpers'; interface ContainerTimeRangeInput extends ContainerInput { timeRange: TimeRange; diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/custom_time_range_badge.test.ts b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/custom_time_range_badge.test.ts index 454c92a602691..1c1b7226b7f30 100644 --- a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/custom_time_range_badge.test.ts +++ b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/custom_time_range_badge.test.ts @@ -6,16 +6,13 @@ * Side Public License, v 1. */ -import { themeServiceMock } from '@kbn/core-theme-browser-mocks'; -import { overlayServiceMock } from '@kbn/core-overlays-browser-mocks'; - import { - TimeRangeEmbeddable, TimeRangeContainer, + TimeRangeEmbeddable, TIME_RANGE_EMBEDDABLE, } from '../../../lib/test_samples/embeddables'; -import { CustomTimeRangeBadge } from './custom_time_range_badge'; import { EditPanelAction } from '../edit_panel_action/edit_panel_action'; +import { CustomTimeRangeBadge } from './custom_time_range_badge'; const editPanelAction = { execute: jest.fn(), @@ -42,13 +39,7 @@ test(`badge is not compatible with embeddable that inherits from parent`, async const child = container.getChild('1'); - const compatible = await new CustomTimeRangeBadge( - overlayServiceMock.createStartContract(), - themeServiceMock.createStartContract(), - editPanelAction, - [], - 'MM YYYY' - ).isCompatible({ + const compatible = await new CustomTimeRangeBadge(editPanelAction, 'MM YYYY').isCompatible({ embeddable: child, }); expect(compatible).toBe(false); @@ -76,13 +67,7 @@ test(`badge is compatible with embeddable that has custom time range`, async () const child = container.getChild('1'); - const compatible = await new CustomTimeRangeBadge( - overlayServiceMock.createStartContract(), - themeServiceMock.createStartContract(), - editPanelAction, - [], - 'MM YYYY' - ).isCompatible({ + const compatible = await new CustomTimeRangeBadge(editPanelAction, 'MM YYYY').isCompatible({ embeddable: child, }); expect(compatible).toBe(true); @@ -109,13 +94,7 @@ test('Attempting to execute on incompatible embeddable throws an error', async ( const child = container.getChild('1'); - const badge = await new CustomTimeRangeBadge( - overlayServiceMock.createStartContract(), - themeServiceMock.createStartContract(), - editPanelAction, - [], - 'MM YYYY' - ); + const badge = await new CustomTimeRangeBadge(editPanelAction, 'MM YYYY'); async function check() { await badge.execute({ embeddable: child }); diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/custom_time_range_badge.tsx b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/custom_time_range_badge.tsx index 08e864b76b1ab..5866c3b5ec195 100644 --- a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/custom_time_range_badge.tsx +++ b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/custom_time_range_badge.tsx @@ -11,9 +11,10 @@ import { PrettyDuration } from '@elastic/eui'; import { renderToString } from 'react-dom/server'; import { Action } from '@kbn/ui-actions-plugin/public'; -import { Embeddable } from '../../..'; +import { EditPanelAction, Embeddable } from '../../..'; import { doesInheritTimeRange } from './does_inherit_time_range'; -import { TimeRangeInput, hasTimeRange, CustomizePanelAction } from './customize_panel_action'; +import { CustomizePanelAction } from './customize_panel_action'; +import { hasTimeRange, TimeRangeInput } from './time_range_helpers'; export const CUSTOM_TIME_RANGE_BADGE = 'CUSTOM_TIME_RANGE_BADGE'; @@ -29,6 +30,13 @@ export class CustomTimeRangeBadge public readonly id = CUSTOM_TIME_RANGE_BADGE; public order = 7; + constructor( + protected readonly editPanel: EditPanelAction, + protected readonly dateFormat?: string + ) { + super(editPanel); + } + public getDisplayName({ embeddable }: TimeBadgeActionContext) { return renderToString( { }); test('execute should open flyout', async () => { - const customizePanelAction = new CustomizePanelAction(overlays, theme, editPanelActionMock); - const spy = jest.spyOn(overlays, 'openFlyout'); - await customizePanelAction.execute({ embeddable }); + const customizePanelAction = new CustomizePanelAction(editPanelActionMock); + const spy = jest.spyOn(openCustomizePanel, 'openCustomizePanelFlyout'); + await customizePanelAction.execute({ embeddable }); expect(spy).toHaveBeenCalled(); }); diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_action.tsx b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_action.tsx index 63fc1902102b6..7f70ecf51acac 100644 --- a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_action.tsx +++ b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_action.tsx @@ -6,48 +6,16 @@ * Side Public License, v 1. */ -import React from 'react'; import { i18n } from '@kbn/i18n'; -import { TimeRange } from '@kbn/es-query'; -import { createKibanaReactContext } from '@kbn/kibana-react-plugin/public'; -import { OverlayStart, ThemeServiceStart } from '@kbn/core/public'; -import { toMountPoint } from '@kbn/react-kibana-mount'; import { Action, IncompatibleActionError } from '@kbn/ui-actions-plugin/public'; -import { core } from '../../../kibana_services'; -import { - IEmbeddable, - Embeddable, - EmbeddableInput, - EmbeddableOutput, - EditPanelAction, -} from '../../..'; -import { ViewMode, CommonlyUsedRange } from '../../../lib/types'; -import { tracksOverlays } from '../track_overlays'; -import { CustomizePanelEditor } from './customize_panel_editor'; +import { EditPanelAction, Embeddable, IEmbeddable } from '../../..'; +import { ViewMode } from '../../../lib/types'; +import { openCustomizePanelFlyout } from './open_customize_panel'; +import { isTimeRangeCompatible, TimeRangeInput } from './time_range_helpers'; export const ACTION_CUSTOMIZE_PANEL = 'ACTION_CUSTOMIZE_PANEL'; -const VISUALIZE_EMBEDDABLE_TYPE = 'visualization'; - -type VisualizeEmbeddable = IEmbeddable<{ id: string }, EmbeddableOutput & { visTypeName: string }>; - -function isVisualizeEmbeddable( - embeddable: IEmbeddable | VisualizeEmbeddable -): embeddable is VisualizeEmbeddable { - return embeddable.type === VISUALIZE_EMBEDDABLE_TYPE; -} - -export interface TimeRangeInput extends EmbeddableInput { - timeRange: TimeRange; -} - -export function hasTimeRange( - embeddable: IEmbeddable | Embeddable -): embeddable is Embeddable { - return (embeddable as Embeddable).getInput().timeRange !== undefined; -} - export interface CustomizePanelActionContext { embeddable: IEmbeddable | Embeddable; } @@ -57,35 +25,7 @@ export class CustomizePanelAction implements Action public id = ACTION_CUSTOMIZE_PANEL; public order = 40; - constructor( - protected readonly overlays: OverlayStart, - protected readonly theme: ThemeServiceStart, - protected readonly editPanel: EditPanelAction, - protected readonly commonlyUsedRanges?: CommonlyUsedRange[], - protected readonly dateFormat?: string - ) {} - - protected isTimeRangeCompatible({ embeddable }: CustomizePanelActionContext): boolean { - const isInputControl = - isVisualizeEmbeddable(embeddable) && - (embeddable as VisualizeEmbeddable).getOutput().visTypeName === 'input_control_vis'; - - const isMarkdown = - isVisualizeEmbeddable(embeddable) && - (embeddable as VisualizeEmbeddable).getOutput().visTypeName === 'markdown'; - - const isImage = embeddable.type === 'image'; - const isNavigation = embeddable.type === 'navigation'; - - return Boolean( - embeddable && - hasTimeRange(embeddable) && - !isInputControl && - !isMarkdown && - !isImage && - !isNavigation - ); - } + constructor(protected readonly editPanel: EditPanelAction) {} public getDisplayName({ embeddable }: CustomizePanelActionContext): string { return i18n.translate('embeddableApi.customizePanel.action.displayName', { @@ -100,7 +40,7 @@ export class CustomizePanelAction implements Action public async isCompatible({ embeddable }: CustomizePanelActionContext) { // It should be possible to customize just the time range in View mode return ( - embeddable.getInput().viewMode === ViewMode.EDIT || this.isTimeRangeCompatible({ embeddable }) + embeddable.getInput().viewMode === ViewMode.EDIT || isTimeRangeCompatible({ embeddable }) ); } @@ -109,46 +49,6 @@ export class CustomizePanelAction implements Action if (!isCompatible) { throw new IncompatibleActionError(); } - - // send the overlay ref to the root embeddable if it is capable of tracking overlays - const rootEmbeddable = embeddable.getRoot(); - const overlayTracker = tracksOverlays(rootEmbeddable) ? rootEmbeddable : undefined; - - const { Provider: KibanaReactContextProvider } = createKibanaReactContext({ - uiSettings: core.uiSettings, - }); - - const onEdit = () => { - this.editPanel.execute({ embeddable }); - }; - - const handle = this.overlays.openFlyout( - toMountPoint( - - { - if (overlayTracker) overlayTracker.clearOverlays(); - handle.close(); - }} - onEdit={onEdit} - /> - , - { theme: this.theme, i18n: core.i18n } - ), - { - size: 's', - 'data-test-subj': 'customizePanel', - onClose: (overlayRef) => { - if (overlayTracker) overlayTracker.clearOverlays(); - overlayRef.close(); - }, - maxWidth: true, - } - ); - overlayTracker?.openOverlay(handle); + openCustomizePanelFlyout({ editPanel: this.editPanel, embeddable }); } } diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_editor.test.tsx b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_editor.test.tsx new file mode 100644 index 0000000000000..a2ccf9782deba --- /dev/null +++ b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_editor.test.tsx @@ -0,0 +1,200 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; + +import { __IntlProvider as IntlProvider } from '@kbn/i18n-react'; +import '@testing-library/jest-dom'; +import { render, screen, waitFor } from '@testing-library/react'; + +import { Filter } from '@kbn/es-query'; +import { ViewMode } from '../../../../common'; +import { FilterableEmbeddable, IEmbeddable } from '../../../lib'; +import { + ContactCardEmbeddable, + ContactCardEmbeddableFactory, + ContactCardEmbeddableInput, +} from '../../../lib/test_samples'; +import { EditPanelAction } from '../edit_panel_action/edit_panel_action'; +import { CustomizePanelAction } from './customize_panel_action'; +import { CustomizePanelEditor } from './customize_panel_editor'; + +const editPanelActionMock = { execute: jest.fn() } as unknown as EditPanelAction; + +const mockEmbeddableFactory = new ContactCardEmbeddableFactory((() => null) as any, {} as any); +const customizePanelAction = new CustomizePanelAction(editPanelActionMock); +customizePanelAction.execute = jest.fn(); + +const DEFAULT_PANEL_TITLE = 'Panel title'; + +const createEmbeddable = async ( + initialInput?: Partial +): Promise => { + return await mockEmbeddableFactory.create({ + id: '20', + firstName: 'Bilbo', + lastName: 'Baggins', + title: DEFAULT_PANEL_TITLE, + ...initialInput, + }); +}; + +const DEFAULT_PROPS = { + timeRangeCompatible: true, + onClose: jest.fn(), + onEdit: jest.fn(), +}; + +describe('panel title / description', () => { + test('does not render if in view mode', async () => { + const mockEmbeddable = await createEmbeddable({ viewMode: ViewMode.VIEW }); + render( + + + + ); + + const customizePanelForm = await screen.findByTestId('customizePanelForm'); + const titleDescriptionComponent = screen.queryByTestId('customEmbeddableTitleComponent'); + expect(customizePanelForm).not.toContainElement(titleDescriptionComponent); + }); + + test('title input receives focus when `focusOnTitle` is `true`', async () => { + const mockEmbeddable = await createEmbeddable({ viewMode: ViewMode.EDIT }); + render( + + + + ); + + const customTitleComponent = await screen.findByTestId('customEmbeddablePanelTitleInput'); + expect(customTitleComponent).toHaveFocus(); + }); + + test('title input does not receive focus when `focusOnTitle` is `false`', async () => { + const mockEmbeddable = await createEmbeddable({ viewMode: ViewMode.EDIT }); + render( + + + + ); + + const customTitleComponent = await screen.findByTestId('customEmbeddablePanelTitleInput'); + expect(customTitleComponent).not.toHaveFocus(); + }); +}); + +describe('custom time picker', () => { + test('renders custom time picker if embeddable supports it', async () => { + const mockEmbeddable = await createEmbeddable({ viewMode: ViewMode.EDIT }); + render( + + + + ); + + const customTimeRangeComponent = await screen.findByTestId('customizePanelTimeRangeDatePicker'); + expect(customTimeRangeComponent).toBeDefined(); + }); + + test('does not render custom time picker if embeddable does not support it', async () => { + const mockEmbeddable = await createEmbeddable({ viewMode: ViewMode.EDIT }); + render( + + + + ); + + const customizePanelForm = await screen.findByTestId('customizePanelForm'); + const customTimeRangeComponent = screen.queryByTestId('customizePanelTimeRangeDatePicker'); + expect(customizePanelForm).not.toContainElement(customTimeRangeComponent); + }); + + test('does not render filters and/or query info if embeddable does not support it', async () => { + const mockEmbeddable = await createEmbeddable({ + viewMode: ViewMode.EDIT, + }); + + render( + + + + ); + + const customizePanelForm = await screen.findByTestId('customizePanelForm'); + const customPanelQuery = screen.queryByTestId('panelCustomQueryRow'); + expect(customizePanelForm).not.toContainElement(customPanelQuery); + const customPanelFilters = screen.queryByTestId('panelCustomFiltersRow'); + expect(customizePanelForm).not.toContainElement(customPanelFilters); + }); + + describe('filterable embeddable', () => { + test('renders custom filters, if provided', async () => { + const mockEmbeddable: FilterableEmbeddable = (await createEmbeddable({ + viewMode: ViewMode.EDIT, + })) as unknown as FilterableEmbeddable; + + mockEmbeddable.getFilters = jest.fn().mockResolvedValue([ + { + meta: {}, + query: {}, + $state: {}, + }, + ] as Filter[]); + mockEmbeddable.getQuery = jest.fn().mockResolvedValue({}); + render( + + + + ); + await waitFor(() => { + expect(screen.getByTestId('euiSkeletonLoadingAriaWrapper')).toBeInTheDocument(); + }); + const customPanelQuery = await screen.findByTestId('panelCustomFiltersRow'); + expect(customPanelQuery).toBeInTheDocument(); + }); + + test('renders a custom query, if provided', async () => { + const mockEmbeddable: FilterableEmbeddable = (await createEmbeddable({ + viewMode: ViewMode.EDIT, + })) as unknown as FilterableEmbeddable; + mockEmbeddable.getFilters = jest.fn().mockResolvedValue([]); + mockEmbeddable.getQuery = jest.fn().mockResolvedValue({ query: 'field : value' }); + render( + + + + ); + await waitFor(() => { + expect(screen.getByTestId('euiSkeletonLoadingAriaWrapper')).toBeInTheDocument(); + }); + const customPanelQuery = await screen.findByTestId('customPanelQuery'); + expect(customPanelQuery).toHaveTextContent('field : value'); + }); + }); +}); diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_editor.tsx b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_editor.tsx index be1e7df0c1057..9893e016af08b 100644 --- a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_editor.tsx +++ b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/customize_panel_editor.tsx @@ -6,40 +6,40 @@ * Side Public License, v 1. */ -import React, { useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import { - EuiFormRow, - EuiFieldText, - EuiSwitch, - EuiFlyoutHeader, - EuiTitle, - EuiFlyoutBody, - EuiForm, - EuiTextArea, - EuiFlyoutFooter, - EuiButtonEmpty, EuiButton, + EuiButtonEmpty, + EuiFieldText, EuiFlexGroup, EuiFlexItem, - EuiSuperDatePicker, + EuiFlyoutBody, + EuiFlyoutFooter, + EuiFlyoutHeader, + EuiForm, + EuiFormRow, EuiSpacer, + EuiSuperDatePicker, + EuiSwitch, + EuiTextArea, + EuiTitle, } from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; import { TimeRange } from '@kbn/es-query'; +import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; -import { TimeRangeInput } from './customize_panel_action'; -import { canInheritTimeRange } from './can_inherit_time_range'; -import { doesInheritTimeRange } from './does_inherit_time_range'; import { - IEmbeddable, - Embeddable, CommonlyUsedRange, - ViewMode, + Embeddable, + IEmbeddable, isFilterableEmbeddable, + ViewMode, } from '../../../lib'; +import { canInheritTimeRange } from './can_inherit_time_range'; +import { doesInheritTimeRange } from './does_inherit_time_range'; import { FiltersDetails } from './filters_details'; +import { TimeRangeInput } from './time_range_helpers'; type PanelSettings = { title?: string; @@ -55,10 +55,11 @@ interface CustomizePanelProps { commonlyUsedRanges?: CommonlyUsedRange[]; onClose: () => void; onEdit: () => void; + focusOnTitle?: boolean; } export const CustomizePanelEditor = (props: CustomizePanelProps) => { - const { onClose, embeddable, dateFormat, timeRangeCompatible, onEdit } = props; + const { onClose, embeddable, dateFormat, timeRangeCompatible, onEdit, focusOnTitle } = props; const editMode = embeddable.getInput().viewMode === ViewMode.EDIT; const [hideTitle, setHideTitle] = useState(embeddable.getInput().hidePanelTitles); const [panelDescription, setPanelDescription] = useState( @@ -75,6 +76,13 @@ export const CustomizePanelEditor = (props: CustomizePanelProps) => { ? (embeddable as Embeddable).getInput().timeRange : undefined ); + const initialFocusRef = useRef(null); + + useEffect(() => { + if (focusOnTitle && initialFocusRef.current) { + initialFocusRef.current.focus(); + } + }, [initialFocusRef, focusOnTitle]); const commonlyUsedRangesForDatePicker = props.commonlyUsedRanges ? props.commonlyUsedRanges.map( @@ -108,7 +116,7 @@ export const CustomizePanelEditor = (props: CustomizePanelProps) => { if (!editMode) return null; return ( - <> +
{ } > { )} /> - +
); }; @@ -292,7 +301,7 @@ export const CustomizePanelEditor = (props: CustomizePanelProps) => { - + {renderCustomTitleComponent()} {renderCustomTimeRangeComponent()} {renderFilterDetails()} diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/does_inherit_time_range.ts b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/does_inherit_time_range.ts index a14ca031c9fb1..7d21a79ab9425 100644 --- a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/does_inherit_time_range.ts +++ b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/does_inherit_time_range.ts @@ -6,8 +6,8 @@ * Side Public License, v 1. */ -import { Embeddable, IContainer, ContainerInput } from '../../../lib'; -import { TimeRangeInput } from './customize_panel_action'; +import { ContainerInput, Embeddable, IContainer } from '../../../lib'; +import { TimeRangeInput } from './time_range_helpers'; export function doesInheritTimeRange(embeddable: Embeddable) { if (!embeddable.parent) { diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/filters_details.tsx b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/filters_details.tsx index 2f151285fe488..300ed9253f0b9 100644 --- a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/filters_details.tsx +++ b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/filters_details.tsx @@ -89,6 +89,7 @@ export function FiltersDetails({ embeddable, editMode, onEdit }: FiltersDetailsP {queryString !== '' && ( 0 && ( ; +}) => { + // send the overlay ref to the root embeddable if it is capable of tracking overlays + const rootEmbeddable = embeddable.getRoot(); + const overlayTracker = tracksOverlays(rootEmbeddable) ? rootEmbeddable : undefined; + + const commonlyUsedRanges = core.uiSettings.get(UI_SETTINGS.TIMEPICKER_QUICK_RANGES); + const dateFormat = core.uiSettings.get(UI_SETTINGS.DATE_FORMAT); + + const { Provider: KibanaReactContextProvider } = createKibanaReactContext({ + uiSettings: core.uiSettings, + }); + + const onEdit = () => { + editPanel.execute({ embeddable }); + }; + + const handle = core.overlays.openFlyout( + toMountPoint( + + { + if (overlayTracker) overlayTracker.clearOverlays(); + handle.close(); + }} + onEdit={onEdit} + /> + , + { theme: core.theme, i18n: core.i18n } + ), + { + size: 's', + 'data-test-subj': 'customizePanel', + onClose: (overlayRef) => { + if (overlayTracker) overlayTracker.clearOverlays(); + overlayRef.close(); + }, + maxWidth: true, + } + ); + overlayTracker?.openOverlay(handle); +}; diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/time_range_helpers.ts b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/time_range_helpers.ts new file mode 100644 index 0000000000000..0b74809be8a08 --- /dev/null +++ b/src/plugins/embeddable/public/embeddable_panel/panel_actions/customize_panel_action/time_range_helpers.ts @@ -0,0 +1,53 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { TimeRange } from '@kbn/es-query'; + +import { Embeddable, EmbeddableInput, EmbeddableOutput, IEmbeddable } from '../../../lib'; + +const VISUALIZE_EMBEDDABLE_TYPE = 'visualization'; + +type VisualizeEmbeddable = IEmbeddable<{ id: string }, EmbeddableOutput & { visTypeName: string }>; + +function isVisualizeEmbeddable( + embeddable: IEmbeddable | VisualizeEmbeddable +): embeddable is VisualizeEmbeddable { + return embeddable.type === VISUALIZE_EMBEDDABLE_TYPE; +} + +export interface TimeRangeInput extends EmbeddableInput { + timeRange: TimeRange; +} + +export function hasTimeRange( + embeddable: IEmbeddable | Embeddable +): embeddable is Embeddable { + return (embeddable as Embeddable).getInput().timeRange !== undefined; +} + +export function isTimeRangeCompatible({ embeddable }: { embeddable: IEmbeddable }): boolean { + const isInputControl = + isVisualizeEmbeddable(embeddable) && + (embeddable as VisualizeEmbeddable).getOutput().visTypeName === 'input_control_vis'; + + const isMarkdown = + isVisualizeEmbeddable(embeddable) && + (embeddable as VisualizeEmbeddable).getOutput().visTypeName === 'markdown'; + + const isImage = embeddable.type === 'image'; + const isNavigation = embeddable.type === 'navigation'; + + return Boolean( + embeddable && + hasTimeRange(embeddable) && + !isInputControl && + !isMarkdown && + !isImage && + !isNavigation + ); +} diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_header.test.tsx b/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_header.test.tsx new file mode 100644 index 0000000000000..770ca76edbc33 --- /dev/null +++ b/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_header.test.tsx @@ -0,0 +1,171 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; + +import '@testing-library/jest-dom'; +import { render, screen } from '@testing-library/react'; + +import { applicationServiceMock } from '@kbn/core-application-browser-mocks'; +import userEvent from '@testing-library/user-event'; +import { ViewMode } from '../../../common'; +import { + ContactCardEmbeddable, + ContactCardEmbeddableFactory, + ContactCardEmbeddableInput, +} from '../../lib/test_samples'; +import { EditPanelAction } from '../panel_actions'; +import * as openCustomizePanel from '../panel_actions/customize_panel_action/open_customize_panel'; +import { EmbeddablePanelHeader } from './embeddable_panel_header'; + +const getEmbeddableFactory = jest.fn(); +const application = applicationServiceMock.createStartContract(); + +const editPanelAction = new EditPanelAction(getEmbeddableFactory, application); +const mockEmbeddableFactory = new ContactCardEmbeddableFactory((() => null) as any, {} as any); +editPanelAction.execute = jest.fn(); + +const DEFAULT_PANEL_TITLE = 'Panel title'; + +const createEmbeddable = async ( + initialInput?: Partial +): Promise => { + return await mockEmbeddableFactory.create({ + id: '20', + firstName: 'Bilbo', + lastName: 'Baggins', + title: DEFAULT_PANEL_TITLE, + ...initialInput, + }); +}; + +describe('view mode', () => { + test('renders as expected', async () => { + const mockEmbeddable = await createEmbeddable({ viewMode: ViewMode.VIEW }); + render( + + ); + const titleComponent = await screen.findByTestId('dashboardPanelTitle'); + expect(titleComponent).toHaveTextContent(DEFAULT_PANEL_TITLE); + }); + + test('renders tooltip + icon when description provided', async () => { + const mockEmbeddable = await createEmbeddable({ viewMode: ViewMode.VIEW }); + mockEmbeddable.updateOutput({ description: 'This is a description ' }); + render( + + ); + + expect(await screen.findByTestId('embeddablePanelTooltipAnchor')).toBeInTheDocument(); + expect(await screen.findByTestId('embeddablePanelTitleDescriptionIcon')).toBeInTheDocument(); + }); + + test('blank titles are hidden in view mode', async () => { + const mockEmbeddable = await createEmbeddable({ title: '', viewMode: ViewMode.VIEW }); + render( + + ); + + const header = await screen.findByTestId('embeddablePanelHeading'); + const titleComponent = screen.queryByTestId('dashboardPanelTitle'); + expect(header).not.toContainElement(titleComponent); + }); + + test('hiding an individual panel title hides it in view mode', async () => { + const mockEmbeddable = await createEmbeddable({ + viewMode: ViewMode.VIEW, + hidePanelTitles: true, + }); + render( + + ); + + const header = await screen.findByTestId('embeddablePanelHeading'); + const titleComponent = screen.queryByTestId('dashboardPanelTitle'); + expect(header).not.toContainElement(titleComponent); + }); +}); + +describe('edit mode', () => { + test('renders as expected', async () => { + const mockEmbeddable = await createEmbeddable({ viewMode: ViewMode.EDIT }); + render( + + ); + const titleComponent = await screen.findByTestId('dashboardPanelTitle'); + expect(titleComponent).toHaveTextContent(DEFAULT_PANEL_TITLE); + }); + + test('blank titles render [No title] in edit mode', async () => { + const mockEmbeddable = await createEmbeddable({ title: '', viewMode: ViewMode.EDIT }); + render( + + ); + const titleComponent = await screen.findByTestId('embeddablePanelTitleInner'); + expect(titleComponent).toHaveTextContent('[No Title]'); + }); + + test('hiding an individual panel title renders **only** the context menu button in edit mode', async () => { + const mockEmbeddable = await createEmbeddable({ + viewMode: ViewMode.EDIT, + hidePanelTitles: true, + }); + render( + + ); + const titleComponent = await screen.findByTestId('embeddablePanelHeading-'); + const innerTitleComponent = await screen.findByTestId('embeddablePanelTitleInner'); + expect(innerTitleComponent).toBeEmptyDOMElement(); + const menuComponent = await screen.findByTestId('embeddablePanelToggleMenuIcon'); + expect(titleComponent).toContainElement(menuComponent); + }); + + test('clicking title opens customize panel flyout', async () => { + const mockEmbeddable = await createEmbeddable({ viewMode: ViewMode.EDIT }); + render( + + ); + const titleComponent = await screen.findByTestId('embeddablePanelTitleLink'); + + const spy = jest.spyOn(openCustomizePanel, 'openCustomizePanelFlyout'); + userEvent.click(titleComponent); + expect(spy).toHaveBeenCalled(); + }); +}); diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_header.tsx b/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_header.tsx index ee27bbca0605a..3de27d6ad193c 100644 --- a/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_header.tsx +++ b/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_header.tsx @@ -85,7 +85,7 @@ export const EmbeddablePanelHeader = ({ if (!showPanelBar) { return ( -
+
{embeddablePanelContextMenu} {ariaLabelElement}
@@ -104,7 +104,7 @@ export const EmbeddablePanelHeader = ({ hideTitle={hideTitle} embeddable={embeddable} description={description} - customizePanelAction={universalActions.customizePanel} + editPanelAction={universalActions.editPanel} /> {showBadges && badgeComponents} diff --git a/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_title.tsx b/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_title.tsx index 734b420d04052..693215a3084ca 100644 --- a/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_title.tsx +++ b/src/plugins/embeddable/public/embeddable_panel/panel_header/embeddable_panel_title.tsx @@ -11,21 +11,22 @@ import React, { useMemo } from 'react'; import { EuiIcon, EuiLink, EuiToolTip } from '@elastic/eui'; import { IEmbeddable, ViewMode } from '../../lib'; -import { CustomizePanelAction } from '../panel_actions'; import { getEditTitleAriaLabel, placeholderTitle } from '../embeddable_panel_strings'; +import { EditPanelAction } from '../panel_actions'; +import { openCustomizePanelFlyout } from '../panel_actions/customize_panel_action/open_customize_panel'; export const EmbeddablePanelTitle = ({ viewMode, hideTitle, embeddable, description, - customizePanelAction, + editPanelAction, }: { hideTitle?: boolean; viewMode?: ViewMode; description?: string; embeddable: IEmbeddable; - customizePanelAction?: CustomizePanelAction; + editPanelAction?: EditPanelAction; }) => { const title = embeddable.getTitle(); @@ -39,32 +40,44 @@ export const EmbeddablePanelTitle = ({ if (viewMode === ViewMode.VIEW) { return {title}; } - if (customizePanelAction) { + if (editPanelAction) { return ( customizePanelAction.execute({ embeddable })} + onClick={() => + openCustomizePanelFlyout({ + editPanel: editPanelAction, + embeddable, + focusOnTitle: true, + }) + } > {title || placeholderTitle} ); } return null; - }, [customizePanelAction, embeddable, title, viewMode, hideTitle]); + }, [editPanelAction, embeddable, title, viewMode, hideTitle]); const titleComponentWithDescription = useMemo(() => { - if (!description) return {titleComponent}; + if (!description) + return ( + + {titleComponent} + + ); return ( - + {titleComponent}{' '} { this.appList = appList; @@ -168,13 +167,7 @@ export class EmbeddablePublicPlugin implements Plugin { - it('should not show custom time picker in flyout', async () => { - await dashboardPanelActions.removePanel(); - await PageObjects.dashboard.waitForRenderComplete(); - await dashboardAddPanel.clickMarkdownQuickButton(); - await PageObjects.visEditor.setMarkdownTxt('I am timeless!'); - await PageObjects.visEditor.clickGo(); - await PageObjects.visualize.saveVisualizationAndReturn(); - await PageObjects.dashboard.clickQuickSave(); - await dashboardPanelActions.customizePanel(); - await dashboardCustomizePanel.expectMissingCustomTimeRange(); - }); - }); }); } diff --git a/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts b/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts index 2c0ac33107fea..4d7950042783a 100644 --- a/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts +++ b/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts @@ -78,44 +78,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(panelTitle).to.equal(EMPTY_TITLE); await PageObjects.dashboard.clearUnsavedChanges(); }); - - it('blank titles are hidden in view mode', async () => { - await PageObjects.dashboard.clickCancelOutOfEditMode(); - - const titleVisibility = (await PageObjects.dashboard.getVisibilityOfPanelTitles())[0]; - expect(titleVisibility).to.be(false); - }); - - it('custom titles are visible in view mode', async () => { - await PageObjects.dashboard.switchToEditMode(); - await dashboardPanelActions.customizePanel(); - await dashboardCustomizePanel.setCustomPanelTitle(CUSTOM_TITLE); - await dashboardCustomizePanel.clickSaveButton(); - await PageObjects.dashboard.clickQuickSave(); - await PageObjects.dashboard.clickCancelOutOfEditMode(); - - const titleVisibility = (await PageObjects.dashboard.getVisibilityOfPanelTitles())[0]; - expect(titleVisibility).to.be(true); - }); - - it('hiding an individual panel title hides it in view mode', async () => { - await PageObjects.dashboard.switchToEditMode(); - await dashboardPanelActions.customizePanel(); - await dashboardCustomizePanel.clickToggleHidePanelTitle(); - await dashboardCustomizePanel.clickSaveButton(); - await PageObjects.dashboard.clickQuickSave(); - await PageObjects.dashboard.clickCancelOutOfEditMode(); - - const titleVisibility = (await PageObjects.dashboard.getVisibilityOfPanelTitles())[0]; - expect(titleVisibility).to.be(false); - - // undo the previous hide panel toggle (i.e. make the panel visible) to keep state consistent - await PageObjects.dashboard.switchToEditMode(); - await dashboardPanelActions.customizePanel(); - await dashboardCustomizePanel.clickToggleHidePanelTitle(); - await dashboardCustomizePanel.clickSaveButton(); - await PageObjects.dashboard.clickQuickSave(); - }); }); describe('by reference', () => {