From a4deee24fabf3d9dd9e365477c19f579912669c9 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Fri, 27 Sep 2024 09:35:31 +0200 Subject: [PATCH] [Discover] Dismiss flyouts when opening another one (#193865) - Closes https://github.com/elastic/kibana/issues/193452 ## Summary This PR makes sure that only one flyout is open at a time and automatically dismisses all others. ### 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] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) (cherry picked from commit 6fc017a597fc34158313b8537f6b6a2536833cba) --- packages/kbn-discover-utils/index.ts | 3 + .../src/utils/dismiss_flyouts.ts | 48 ++++++++ .../kbn-discover-utils/src/utils/index.ts | 1 + .../src/components/as_flyout/index.tsx | 1 + .../components/top_nav/discover_topnav.tsx | 8 ++ .../components/top_nav/get_top_nav_badges.tsx | 6 +- .../discover_grid_flyout.tsx | 9 +- .../public/chart/chart_config_panel.tsx | 11 +- .../query_string_input/esql_menu_popover.tsx | 18 ++- .../query_string_input/query_bar_top_row.tsx | 9 +- .../public/search_bar/create_search_bar.tsx | 1 + .../public/search_bar/search_bar.tsx | 2 + .../apps/discover/group8/_flyouts.ts | 104 ++++++++++++++++++ test/functional/apps/discover/group8/index.ts | 1 + test/functional/services/esql.ts | 19 ++++ 15 files changed, 230 insertions(+), 11 deletions(-) create mode 100644 packages/kbn-discover-utils/src/utils/dismiss_flyouts.ts create mode 100644 test/functional/apps/discover/group8/_flyouts.ts diff --git a/packages/kbn-discover-utils/index.ts b/packages/kbn-discover-utils/index.ts index ba7e832b8f1c6..ed6d58ca3da8d 100644 --- a/packages/kbn-discover-utils/index.ts +++ b/packages/kbn-discover-utils/index.ts @@ -56,6 +56,9 @@ export { getFieldValue, getVisibleColumns, canPrependTimeFieldColumn, + DiscoverFlyouts, + dismissAllFlyoutsExceptFor, + dismissFlyouts, } from './src'; export type { LogsContextService } from './src'; diff --git a/packages/kbn-discover-utils/src/utils/dismiss_flyouts.ts b/packages/kbn-discover-utils/src/utils/dismiss_flyouts.ts new file mode 100644 index 0000000000000..9ffccfe31cddd --- /dev/null +++ b/packages/kbn-discover-utils/src/utils/dismiss_flyouts.ts @@ -0,0 +1,48 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +export enum DiscoverFlyouts { + lensEdit = 'lensEdit', + docViewer = 'docViewer', + esqlDocs = 'esqlDocs', +} + +const AllDiscoverFlyouts = Object.values(DiscoverFlyouts); + +const getFlyoutCloseButton = (flyout: DiscoverFlyouts): HTMLElement | null => { + switch (flyout) { + case DiscoverFlyouts.lensEdit: + return document.getElementById('lnsCancelEditOnFlyFlyout'); + case DiscoverFlyouts.docViewer: + return document.querySelector('[data-test-subj="docViewerFlyoutCloseButton"]'); + case DiscoverFlyouts.esqlDocs: + return document.querySelector( + '[data-test-subj="esqlInlineDocumentationFlyout"] [data-test-subj="euiFlyoutCloseButton"]' + ); + } +}; + +export const dismissFlyouts = ( + selectedFlyouts: DiscoverFlyouts[] = AllDiscoverFlyouts, + excludedFlyout?: DiscoverFlyouts +) => { + selectedFlyouts.forEach((flyout) => { + if (flyout === excludedFlyout) { + return; + } + const closeButton = getFlyoutCloseButton(flyout); + if (closeButton) { + closeButton.click?.(); + } + }); +}; + +export const dismissAllFlyoutsExceptFor = (excludedFlyout: DiscoverFlyouts) => { + dismissFlyouts(AllDiscoverFlyouts, excludedFlyout); +}; diff --git a/packages/kbn-discover-utils/src/utils/index.ts b/packages/kbn-discover-utils/src/utils/index.ts index 201cd10fb4dee..01ef6b83f83a8 100644 --- a/packages/kbn-discover-utils/src/utils/index.ts +++ b/packages/kbn-discover-utils/src/utils/index.ts @@ -22,3 +22,4 @@ export * from './get_field_value'; export * from './calc_field_counts'; export * from './get_visible_columns'; export { isLegacyTableEnabled } from './is_legacy_table_enabled'; +export { DiscoverFlyouts, dismissAllFlyoutsExceptFor, dismissFlyouts } from './dismiss_flyouts'; diff --git a/packages/kbn-language-documentation/src/components/as_flyout/index.tsx b/packages/kbn-language-documentation/src/components/as_flyout/index.tsx index 583fb96c96cef..da6ff14134e1b 100644 --- a/packages/kbn-language-documentation/src/components/as_flyout/index.tsx +++ b/packages/kbn-language-documentation/src/components/as_flyout/index.tsx @@ -78,6 +78,7 @@ function DocumentationFlyout({ ownFocus onClose={() => onHelpMenuVisibilityChange(false)} aria-labelledby="esqlInlineDocumentationFlyout" + data-test-subj="esqlInlineDocumentationFlyout" type="push" size={DEFAULT_WIDTH} paddingSize="m" diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index b84b00439c801..45e8755ca3156 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -12,6 +12,7 @@ import { type DataView, DataViewType } from '@kbn/data-views-plugin/public'; import { DataViewPickerProps } from '@kbn/unified-search-plugin/public'; import { ENABLE_ESQL } from '@kbn/esql-utils'; import { TextBasedLanguages } from '@kbn/esql-utils'; +import { DiscoverFlyouts, dismissAllFlyoutsExceptFor } from '@kbn/discover-utils'; import { useSavedSearchInitial } from '../../state_management/discover_state_provider'; import { ESQL_TRANSITION_MODAL_KEY } from '../../../../../common/constants'; import { useInternalStateSelector } from '../../state_management/discover_internal_state_container'; @@ -233,6 +234,12 @@ export const DiscoverTopNav = ({ uiSettings, ]); + const onESQLDocsFlyoutVisibilityChanged = useCallback((isOpen: boolean) => { + if (isOpen) { + dismissAllFlyoutsExceptFor(DiscoverFlyouts.esqlDocs); + } + }, []); + const searchBarCustomization = useDiscoverCustomization('search_bar'); const SearchBar = useMemo( @@ -278,6 +285,7 @@ export const DiscoverTopNav = ({ ) : undefined } + onESQLDocsFlyoutVisibilityChanged={onESQLDocsFlyoutVisibilityChanged} /> {isESQLToDataViewTransitionModalVisible && ( diff --git a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_badges.tsx b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_badges.tsx index 22645fa8d7aa3..ee8187433ff44 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_badges.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_badges.tsx @@ -11,6 +11,7 @@ import type { TopNavMenuBadgeProps } from '@kbn/navigation-plugin/public'; import { getTopNavUnsavedChangesBadge } from '@kbn/unsaved-changes-badge'; import { getManagedContentBadge } from '@kbn/managed-content-badge'; import { i18n } from '@kbn/i18n'; +import { dismissFlyouts, DiscoverFlyouts } from '@kbn/discover-utils'; import { DiscoverStateContainer } from '../../state_management/discover_state'; import type { TopNavCustomization } from '../../../../customizations'; import { onSaveSearch } from './on_save_search'; @@ -47,10 +48,7 @@ export const getTopNavBadges = ({ entries.push({ data: getTopNavUnsavedChangesBadge({ onRevert: async () => { - const lensEditFlyoutCancelButton = document.getElementById('lnsCancelEditOnFlyFlyout'); - if (lensEditFlyoutCancelButton) { - lensEditFlyoutCancelButton.click?.(); - } + dismissFlyouts([DiscoverFlyouts.lensEdit]); await stateContainer.actions.undoSavedSearchChanges(); }, onSave: diff --git a/src/plugins/discover/public/components/discover_grid_flyout/discover_grid_flyout.tsx b/src/plugins/discover/public/components/discover_grid_flyout/discover_grid_flyout.tsx index 71904870a9e4f..9ad389381b90e 100644 --- a/src/plugins/discover/public/components/discover_grid_flyout/discover_grid_flyout.tsx +++ b/src/plugins/discover/public/components/discover_grid_flyout/discover_grid_flyout.tsx @@ -7,13 +7,14 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import React, { useMemo } from 'react'; +import React, { useEffect, useMemo } from 'react'; import type { DataView } from '@kbn/data-views-plugin/public'; -import { Filter, Query, AggregateQuery, isOfAggregateQueryType } from '@kbn/es-query'; +import { AggregateQuery, Filter, isOfAggregateQueryType, Query } from '@kbn/es-query'; import type { DataTableRecord } from '@kbn/discover-utils/types'; import type { DocViewFilterFn } from '@kbn/unified-doc-viewer/types'; import type { DataTableColumnsMeta } from '@kbn/unified-data-table'; import type { DocViewsRegistry } from '@kbn/unified-doc-viewer'; +import { DiscoverFlyouts, dismissAllFlyoutsExceptFor } from '@kbn/discover-utils'; import { UnifiedDocViewerFlyout } from '@kbn/unified-doc-viewer-plugin/public'; import { useDiscoverServices } from '../../hooks/use_discover_services'; import { useFlyoutActions } from './use_flyout_actions'; @@ -88,6 +89,10 @@ export function DiscoverGridFlyout({ return getDocViewer({ record: actualHit }); }, [flyoutCustomization, getDocViewerAccessor, actualHit]); + useEffect(() => { + dismissAllFlyoutsExceptFor(DiscoverFlyouts.docViewer); + }, []); + return ( { + if (flyoutElement) { + dismissAllFlyoutsExceptFor(DiscoverFlyouts.lensEdit); + } + }, [flyoutElement]); + + return flyoutElement; } diff --git a/src/plugins/unified_search/public/query_string_input/esql_menu_popover.tsx b/src/plugins/unified_search/public/query_string_input/esql_menu_popover.tsx index 379b5563917fa..cfffa8ae7f83a 100644 --- a/src/plugins/unified_search/public/query_string_input/esql_menu_popover.tsx +++ b/src/plugins/unified_search/public/query_string_input/esql_menu_popover.tsx @@ -22,7 +22,13 @@ import { FEEDBACK_LINK } from '@kbn/esql-utils'; import { LanguageDocumentationFlyout } from '@kbn/language-documentation'; import type { IUnifiedSearchPluginServices } from '../types'; -export const ESQLMenuPopover = () => { +export interface ESQLMenuPopoverProps { + onESQLDocsFlyoutVisibilityChanged?: (isOpen: boolean) => void; +} + +export const ESQLMenuPopover: React.FC = ({ + onESQLDocsFlyoutVisibilityChanged, +}) => { const kibana = useKibana(); const { docLinks } = kibana.services; @@ -34,6 +40,14 @@ export const ESQLMenuPopover = () => { setIsESQLMenuPopoverOpen(false); }, [isLanguageComponentOpen]); + const onHelpMenuVisibilityChange = useCallback( + (status: boolean) => { + setIsLanguageComponentOpen(status); + onESQLDocsFlyoutVisibilityChanged?.(status); + }, + [setIsLanguageComponentOpen, onESQLDocsFlyoutVisibilityChanged] + ); + const esqlPanelItems = useMemo(() => { const panelItems: EuiContextMenuPanelProps['items'] = []; panelItems.push( @@ -122,7 +136,7 @@ export const ESQLMenuPopover = () => { searchInDescription linkToDocumentation={docLinks?.links?.query?.queryESQL ?? ''} isHelpMenuOpen={isLanguageComponentOpen} - onHelpMenuVisibilityChange={setIsLanguageComponentOpen} + onHelpMenuVisibilityChange={onHelpMenuVisibilityChange} /> ); diff --git a/src/plugins/unified_search/public/query_string_input/query_bar_top_row.tsx b/src/plugins/unified_search/public/query_string_input/query_bar_top_row.tsx index 9805381800ccc..c4f04a3d01ce3 100644 --- a/src/plugins/unified_search/public/query_string_input/query_bar_top_row.tsx +++ b/src/plugins/unified_search/public/query_string_input/query_bar_top_row.tsx @@ -51,7 +51,7 @@ import { NoDataPopover } from './no_data_popover'; import { shallowEqual } from '../utils/shallow_equal'; import { AddFilterPopover } from './add_filter_popover'; import { DataViewPicker, DataViewPickerProps } from '../dataview_picker'; -import { ESQLMenuPopover } from './esql_menu_popover'; +import { ESQLMenuPopover, type ESQLMenuPopoverProps } from './esql_menu_popover'; import { FilterButtonGroup } from '../filter_bar/filter_button_group/filter_button_group'; import type { @@ -186,6 +186,7 @@ export interface QueryBarTopRowProps submitOnBlur?: boolean; renderQueryInputAppend?: () => React.ReactNode; disableExternalPadding?: boolean; + onESQLDocsFlyoutVisibilityChanged?: ESQLMenuPopoverProps['onESQLDocsFlyoutVisibilityChanged']; } export const SharingMetaFields = React.memo(function SharingMetaFields({ @@ -774,7 +775,11 @@ export const QueryBarTopRow = React.memo( wrap > {props.dataViewPickerOverride || renderDataViewsPicker()} - {Boolean(isQueryLangSelected) && } + {Boolean(isQueryLangSelected) && ( + + )} diff --git a/src/plugins/unified_search/public/search_bar/search_bar.tsx b/src/plugins/unified_search/public/search_bar/search_bar.tsx index 3ea5d0165df14..04e90347c59db 100644 --- a/src/plugins/unified_search/public/search_bar/search_bar.tsx +++ b/src/plugins/unified_search/public/search_bar/search_bar.tsx @@ -138,6 +138,7 @@ export interface SearchBarOwnProps { submitOnBlur?: boolean; renderQueryInputAppend?: () => React.ReactNode; + onESQLDocsFlyoutVisibilityChanged?: QueryBarTopRowProps['onESQLDocsFlyoutVisibilityChanged']; } export type SearchBarProps = SearchBarOwnProps & @@ -660,6 +661,7 @@ class SearchBarUI extends C suggestionsAbstraction={this.props.suggestionsAbstraction} renderQueryInputAppend={this.props.renderQueryInputAppend} disableExternalPadding={this.props.displayStyle === 'withBorders'} + onESQLDocsFlyoutVisibilityChanged={this.props.onESQLDocsFlyoutVisibilityChanged} /> ); diff --git a/test/functional/apps/discover/group8/_flyouts.ts b/test/functional/apps/discover/group8/_flyouts.ts new file mode 100644 index 0000000000000..d36c9f0766d24 --- /dev/null +++ b/test/functional/apps/discover/group8/_flyouts.ts @@ -0,0 +1,104 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../ftr_provider_context'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const esArchiver = getService('esArchiver'); + const { common, discover, timePicker, header } = getPageObjects([ + 'common', + 'discover', + 'timePicker', + 'header', + ]); + const kibanaServer = getService('kibanaServer'); + const security = getService('security'); + const retry = getService('retry'); + const dataGrid = getService('dataGrid'); + const esql = getService('esql'); + const testSubjects = getService('testSubjects'); + + describe('discover flyouts', function () { + async function isLensEditFlyoutOpen() { + return await testSubjects.exists('lnsChartSwitchPopover'); + } + + async function openLensEditFlyout() { + await testSubjects.click('unifiedHistogramEditFlyoutVisualization'); + await retry.waitFor('flyout', async () => { + return await isLensEditFlyoutOpen(); + }); + } + + before(async function () { + await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader']); + await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover'); + await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); + await kibanaServer.uiSettings.replace({ defaultIndex: 'logstash-*' }); + await timePicker.setDefaultAbsoluteRangeViaUiSettings(); + }); + + beforeEach(async function () { + await common.navigateToApp('discover'); + await header.waitUntilLoadingHasFinished(); + await discover.waitUntilSearchingHasFinished(); + await discover.selectTextBaseLang(); + await header.waitUntilLoadingHasFinished(); + await discover.waitUntilSearchingHasFinished(); + }); + + after(async () => { + await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover'); + await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional'); + await kibanaServer.uiSettings.replace({}); + await kibanaServer.savedObjects.cleanStandardList(); + }); + + it('doc viewer flyout should get dismissed on opening ESQL docs flyout', async function () { + await dataGrid.clickRowToggle({ rowIndex: 0 }); + expect(await dataGrid.isShowingDocViewer()).to.be(true); + await esql.openQuickReferenceFlyout(); + expect(await dataGrid.isShowingDocViewer()).to.be(false); + expect(await esql.isOpenQuickReferenceFlyout()).to.be(true); + }); + + it('doc viewer flyout should get dismissed on opening Lens Edit flyout', async function () { + await dataGrid.clickRowToggle({ rowIndex: 0 }); + expect(await dataGrid.isShowingDocViewer()).to.be(true); + await openLensEditFlyout(); + expect(await dataGrid.isShowingDocViewer()).to.be(false); + expect(await isLensEditFlyoutOpen()).to.be(true); + }); + + it('ESQL docs flyout should get dismissed on opening doc viewer flyout', async function () { + await esql.openQuickReferenceFlyout(); + expect(await esql.isOpenQuickReferenceFlyout()).to.be(true); + await dataGrid.clickRowToggle({ rowIndex: 0 }); + expect(await dataGrid.isShowingDocViewer()).to.be(true); + expect(await esql.isOpenQuickReferenceFlyout()).to.be(false); + }); + + it('ESQL docs flyout should get dismissed on opening Lens Edit flyout', async function () { + await esql.openQuickReferenceFlyout(); + expect(await esql.isOpenQuickReferenceFlyout()).to.be(true); + await openLensEditFlyout(); + expect(await isLensEditFlyoutOpen()).to.be(true); + expect(await esql.isOpenQuickReferenceFlyout()).to.be(false); + }); + + it('Lens Edit flyout should get dismissed on opening doc viewer flyout', async function () { + await openLensEditFlyout(); + expect(await isLensEditFlyoutOpen()).to.be(true); + await dataGrid.clickRowToggle({ rowIndex: 0 }); + expect(await dataGrid.isShowingDocViewer()).to.be(true); + expect(await isLensEditFlyoutOpen()).to.be(false); + }); + }); +} diff --git a/test/functional/apps/discover/group8/index.ts b/test/functional/apps/discover/group8/index.ts index 7eb0f9d3b0861..14b544e2c2015 100644 --- a/test/functional/apps/discover/group8/index.ts +++ b/test/functional/apps/discover/group8/index.ts @@ -24,5 +24,6 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./_default_route')); loadTestFile(require.resolve('./_hide_announcements')); + loadTestFile(require.resolve('./_flyouts')); }); } diff --git a/test/functional/services/esql.ts b/test/functional/services/esql.ts index 81ccb22d208e9..63836d2c5d2f5 100644 --- a/test/functional/services/esql.ts +++ b/test/functional/services/esql.ts @@ -50,4 +50,23 @@ export class ESQLService extends FtrService { const toggle = await row.findByTestSubject('ESQLEditor-queryHistory-runQuery-button'); await toggle.click(); } + + public async openHelpMenu() { + await this.testSubjects.click('esql-menu-button'); + await this.retry.waitFor('popover to appear', async () => { + return await this.testSubjects.exists('esql-quick-reference'); + }); + } + + public async isOpenQuickReferenceFlyout() { + return await this.testSubjects.exists('esqlInlineDocumentationFlyout'); + } + + public async openQuickReferenceFlyout() { + await this.openHelpMenu(); + await this.testSubjects.click('esql-quick-reference'); + await this.retry.waitFor('quick reference to appear', async () => { + return await this.isOpenQuickReferenceFlyout(); + }); + } }