From 6d42a3b40438c1d66020bff9b92ae6e33f9394eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 4 Oct 2024 11:50:28 +0100 Subject: [PATCH 01/20] Register global setting --- x-pack/plugins/spaces/server/plugin.ts | 2 ++ x-pack/plugins/spaces/server/ui_settings.ts | 25 +++++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 x-pack/plugins/spaces/server/ui_settings.ts diff --git a/x-pack/plugins/spaces/server/plugin.ts b/x-pack/plugins/spaces/server/plugin.ts index 2f8fb2ec30842..e36a6fb3cc7f1 100644 --- a/x-pack/plugins/spaces/server/plugin.ts +++ b/x-pack/plugins/spaces/server/plugin.ts @@ -35,6 +35,7 @@ import { SpacesClientService } from './spaces_client'; import type { SpacesServiceSetup, SpacesServiceStart } from './spaces_service'; import { SpacesService } from './spaces_service'; import type { SpacesRequestHandlerContext } from './types'; +import { getUiSettings } from './ui_settings'; import { registerSpacesUsageCollector } from './usage_collection'; import { UsageStatsService } from './usage_stats'; import { SpacesLicenseService } from '../common/licensing'; @@ -149,6 +150,7 @@ export class SpacesPlugin public setup(core: CoreSetup, plugins: PluginsSetup): SpacesPluginSetup { this.onCloud$.next(plugins.cloud !== undefined && plugins.cloud.isCloudEnabled); const spacesClientSetup = this.spacesClientService.setup({ config$: this.config$ }); + core.uiSettings.registerGlobal(getUiSettings()); const spacesServiceSetup = this.spacesService.setup({ basePath: core.http.basePath, diff --git a/x-pack/plugins/spaces/server/ui_settings.ts b/x-pack/plugins/spaces/server/ui_settings.ts new file mode 100644 index 0000000000000..d57822e028f2a --- /dev/null +++ b/x-pack/plugins/spaces/server/ui_settings.ts @@ -0,0 +1,25 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { schema } from '@kbn/config-schema'; +import type { UiSettingsParams } from '@kbn/core/types'; + +import { SHOW_SPACE_SOLUTION_TOUR_SETTING } from '../common/constants'; + +/** + * uiSettings definitions for Spaces + */ +export const getUiSettings = (): Record => { + return { + [SHOW_SPACE_SOLUTION_TOUR_SETTING]: { + schema: schema.boolean(), + value: true, + readonly: true, + readonlyMode: 'ui', + }, + }; +}; From 9871ea383119c4475b019326ee62946d07779d33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 4 Oct 2024 11:50:54 +0100 Subject: [PATCH 02/20] Add tour to the SpacesNavControl --- x-pack/plugins/spaces/common/constants.ts | 5 + .../spaces/public/nav_control/nav_control.tsx | 34 ++++++ .../nav_control/nav_control_popover.tsx | 113 +++++++++++++++--- 3 files changed, 138 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/spaces/common/constants.ts b/x-pack/plugins/spaces/common/constants.ts index 14932a93a06b7..232892ab7b9ad 100644 --- a/x-pack/plugins/spaces/common/constants.ts +++ b/x-pack/plugins/spaces/common/constants.ts @@ -52,3 +52,8 @@ export const API_VERSIONS = { v1: '2023-10-31', }, }; + +/** + * The setting to control whether the Space Solution Tour is shown. + */ +export const SHOW_SPACE_SOLUTION_TOUR_SETTING = 'showSpaceSolutionTour'; diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control.tsx index 7cb32fff01e1e..a98070b0139ae 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control.tsx @@ -8,20 +8,52 @@ import { EuiLoadingSpinner } from '@elastic/eui'; import React, { lazy, Suspense } from 'react'; import ReactDOM from 'react-dom'; +import { BehaviorSubject, from, of, switchMap } from 'rxjs'; import type { CoreStart } from '@kbn/core/public'; import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render'; +import { SHOW_SPACE_SOLUTION_TOUR_SETTING } from '../../common/constants'; import type { EventTracker } from '../analytics'; import type { ConfigType } from '../config'; import type { SpacesManager } from '../spaces_manager'; +function initTour(core: CoreStart, spacesManager: SpacesManager) { + const showTourUiSettingValue = core.settings.globalClient.get( + SHOW_SPACE_SOLUTION_TOUR_SETTING, + true + ); + const showTour$ = new BehaviorSubject(showTourUiSettingValue); + + const showTourObservable$ = from(spacesManager.getSpaces()).pipe( + switchMap((spaces) => { + if (spaces.length === 0 || spaces.length > 1) return of(false); // Don't show the tour if there are multiple spaces + + const defaultSpace = spaces[0]; + if (!defaultSpace.solution || defaultSpace.solution === 'classic') return of(false); // Don't show the tour if the default space is the classic solution + + return showTour$.asObservable(); + }) + ); + + const onFinishTour = () => { + core.settings.globalClient.set(SHOW_SPACE_SOLUTION_TOUR_SETTING, false).catch(() => { + // Silently swallow errors, the user will just see the tour again next time they load the page + }); + showTour$.next(false); + }; + + return { showTour$: showTourObservable$, onFinishTour }; +} + export function initSpacesNavControl( spacesManager: SpacesManager, core: CoreStart, config: ConfigType, eventTracker: EventTracker ) { + const { showTour$, onFinishTour } = initTour(core, spacesManager); + core.chrome.navControls.registerLeft({ order: 1000, mount(targetDomElement: HTMLElement) { @@ -47,6 +79,8 @@ export function initSpacesNavControl( navigateToUrl={core.application.navigateToUrl} allowSolutionVisibility={config.allowSolutionVisibility} eventTracker={eventTracker} + showTour$={showTour$} + onFinishTour={onFinishTour} /> , diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx index b9830b2063dd5..7e6b44dba04ea 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx @@ -7,20 +7,24 @@ import type { PopoverAnchorPosition, WithEuiThemeProps } from '@elastic/eui'; import { + EuiButtonEmpty, EuiHeaderSectionItemButton, + EuiLink, EuiLoadingSpinner, EuiPopover, + EuiText, + EuiTourStep, withEuiTheme, } from '@elastic/eui'; import React, { Component, lazy, Suspense } from 'react'; -import type { Subscription } from 'rxjs'; +import type { Observable, Subscription } from 'rxjs'; import type { ApplicationStart, Capabilities } from '@kbn/core/public'; import { i18n } from '@kbn/i18n'; import { SpacesDescription } from './components/spaces_description'; import { SpacesMenu } from './components/spaces_menu'; -import type { Space } from '../../common'; +import type { SolutionView, Space } from '../../common'; import type { EventTracker } from '../analytics'; import { getSpaceAvatarComponent } from '../space_avatar'; import type { SpacesManager } from '../spaces_manager'; @@ -40,6 +44,8 @@ interface Props { theme: WithEuiThemeProps['theme']; allowSolutionVisibility: boolean; eventTracker: EventTracker; + showTour$: Observable; + onFinishTour: () => void; } interface State { @@ -47,12 +53,15 @@ interface State { loading: boolean; activeSpace: Space | null; spaces: Space[]; + showTour: boolean; } const popoutContentId = 'headerSpacesMenuContent'; +const tourLearnMoreLink = 'https://ela.st/left-nav'; class NavControlPopoverUI extends Component { private activeSpace$?: Subscription; + private showTour$Sub?: Subscription; constructor(props: Props) { super(props); @@ -61,6 +70,7 @@ class NavControlPopoverUI extends Component { loading: false, activeSpace: null, spaces: [], + showTour: false, }; } @@ -72,15 +82,23 @@ class NavControlPopoverUI extends Component { }); }, }); + + this.showTour$Sub = this.props.showTour$.subscribe((showTour) => { + this.setState({ showTour }); + }); } public componentWillUnmount() { this.activeSpace$?.unsubscribe(); + this.showTour$Sub?.unsubscribe(); } public render() { const button = this.getActiveSpaceButton(); const { theme } = this.props; + const { activeSpace } = this.state; + const tourTexts = getTourTexts(activeSpace?.solution); + const isTourOpen = Boolean(activeSpace) && this.state.showTour && !this.state.showSpaceSelector; let element: React.ReactNode; if (this.state.loading || this.state.spaces.length < 2) { @@ -111,19 +129,45 @@ class NavControlPopoverUI extends Component { } return ( - +

{tourTexts.content}

+

+ + {tourTexts.learnMore} + +

+ + } + isStepOpen={isTourOpen} + minWidth={300} + maxWidth={360} + onFinish={this.props.onFinishTour} + step={1} + stepsTotal={1} + title={tourTexts.title} + anchorPosition="downCenter" + footerAction={ + + {tourTexts.closeBtn} + + } > - {element} -
+ + {element} + + ); } @@ -212,3 +256,44 @@ class NavControlPopoverUI extends Component { } export const NavControlPopover = withEuiTheme(NavControlPopoverUI); + +function getTourTexts(solution?: SolutionView) { + const solutionMap: Record = { + es: i18n.translate('xpack.spaces.navControl.tour.esSolution', { + defaultMessage: 'search', + }), + security: i18n.translate('xpack.spaces.navControl.tour.securitySolution', { + defaultMessage: 'security', + }), + oblt: i18n.translate('xpack.spaces.navControl.tour.obltSolution', { + defaultMessage: 'observability', + }), + classic: '', // Tour is not shown for the classic solution + }; + + const title = !!solution + ? i18n.translate('xpack.spaces.navControl.tour.title', { + defaultMessage: 'You chose the {solution} solution view', + values: { solution: solutionMap[solution] }, + }) + : ''; + + const content = !!solution + ? i18n.translate('xpack.spaces.navControl.tour.content', { + defaultMessage: + 'It provides all the analytics and {solution} features you need. You can switch views or return to the classic navigation from your space settings, or create other spaces with different views.', + values: { solution: solutionMap[solution] }, + }) + : ''; + + return { + title, + content, + closeBtn: i18n.translate('xpack.spaces.navControl.tour.closeBtn', { + defaultMessage: 'Close', + }), + learnMore: i18n.translate('xpack.spaces.navControl.tour.learnMore', { + defaultMessage: 'Learn more', + }), + }; +} From ff44a35297397a081abbfc66d9bb7c7d9514fe17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 4 Oct 2024 12:44:59 +0100 Subject: [PATCH 03/20] Add functional tests --- .../nav_control/nav_control_popover.tsx | 8 ++- x-pack/test/common/services/spaces.ts | 33 ++++++++++ .../solution_view_flag_enabled/index.ts | 1 + .../solution_tour.ts | 61 +++++++++++++++++++ 4 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx index 7e6b44dba04ea..6087747fe3f7d 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx @@ -149,10 +149,16 @@ class NavControlPopoverUI extends Component { title={tourTexts.title} anchorPosition="downCenter" footerAction={ - + {tourTexts.closeBtn} } + data-test-subj="spaceSolutionTour" > , + { overwrite = true }: { overwrite?: boolean } = {} + ) { + log.debug(`updating space ${id}`); + const { data, status, statusText } = await axios.put( + `/api/spaces/space/${id}?overwrite=${overwrite}`, + updatedSpace + ); + + if (status !== 200) { + throw new Error( + `Expected status code of 200, received ${status} ${statusText}: ${util.inspect(data)}` + ); + } + log.debug(`updated space ${id}`); + } + public async delete(spaceId: string) { log.debug(`deleting space id: ${spaceId}`); const { data, status, statusText } = await axios.delete(`/api/spaces/space/${spaceId}`); @@ -89,6 +108,20 @@ export function SpacesServiceProvider({ getService }: FtrProviderContext) { log.debug(`deleted space id: ${spaceId}`); } + public async getSpace(id: string) { + log.debug(`retrieving space ${id}`); + const { data, status, statusText } = await axios.get(`/api/spaces/space/${id}`); + + if (status !== 200) { + throw new Error( + `Expected status code of 200, received ${status} ${statusText}: ${util.inspect(data)}` + ); + } + log.debug(`retrieved space ${id}`); + + return data; + } + public async getAll() { log.debug('retrieving all spaces'); const { data, status, statusText } = await axios.get('/api/spaces/space'); diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/index.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/index.ts index 99ce8f2ab16e7..45a8f78387154 100644 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/index.ts +++ b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/index.ts @@ -10,5 +10,6 @@ import { FtrProviderContext } from '../../../ftr_provider_context'; export default function spacesApp({ loadTestFile }: FtrProviderContext) { describe('Spaces app (with solution view)', function spacesAppTestSuite() { loadTestFile(require.resolve('./create_edit_space')); + loadTestFile(require.resolve('./solution_tour')); }); } diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts new file mode 100644 index 0000000000000..a8f7ac782ca1f --- /dev/null +++ b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts @@ -0,0 +1,61 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { FtrProviderContext } from '../../../ftr_provider_context'; + +export default function ({ getPageObjects, getService }: FtrProviderContext) { + const kibanaServer = getService('kibanaServer'); + const PageObjects = getPageObjects(['common', 'settings', 'security', 'spaceSelector']); + const testSubjects = getService('testSubjects'); + const spacesService = getService('spaces'); + const browser = getService('browser'); + + describe('space solution tour', () => { + before(async () => { + await kibanaServer.savedObjects.cleanStandardList(); + }); + + after(async () => { + await kibanaServer.savedObjects.cleanStandardList(); + }); + + describe('solution tour', () => { + before(async () => { + await PageObjects.common.navigateToUrl('management', 'kibana/spaces', { + shouldUseHashForSubUrl: false, + }); + }); + + it('does not show the solution tour for the classic space', async () => { + await testSubjects.missingOrFail('spaceSolutionTour'); + }); + + it('does show the solution tour if the default space has a solution set', async () => { + const defaultSpace = await spacesService.getSpace('default'); + + await spacesService.update('default', { + ...defaultSpace, + solution: 'es', // set a solution + }); + + await browser.refresh(); + await testSubjects.existOrFail('spaceSolutionTour'); + + await testSubjects.click('closeTourBtn'); // close the tour + await PageObjects.common.sleep(1000); // wait to save the setting + + await browser.refresh(); + await testSubjects.missingOrFail('spaceSolutionTour'); // The tour does not appear after refresh + + await spacesService.update('default', { + ...defaultSpace, + solution: 'classic', // revert to not impact future tests + }); + }); + }); + }); +} From bca12fba7ba7cac9862740d60c81a9379d61118c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 4 Oct 2024 16:15:05 +0100 Subject: [PATCH 04/20] Update jest tests --- .../nav_control_popover.test.tsx.snap | 58 ++++++++------- .../nav_control/nav_control_popover.test.tsx | 72 +++++++++++++++++-- .../nav_control/nav_control_popover.tsx | 10 ++- 3 files changed, 105 insertions(+), 35 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/__snapshots__/nav_control_popover.test.tsx.snap b/x-pack/plugins/spaces/public/nav_control/__snapshots__/nav_control_popover.test.tsx.snap index af2221e460a32..2f52162a8bb70 100644 --- a/x-pack/plugins/spaces/public/nav_control/__snapshots__/nav_control_popover.test.tsx.snap +++ b/x-pack/plugins/spaces/public/nav_control/__snapshots__/nav_control_popover.test.tsx.snap @@ -4,44 +4,48 @@ exports[`NavControlPopover renders without crashing 1`] = `
- + +
diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx index 9b573615c65b9..9b0ccfb8e413f 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx @@ -18,7 +18,7 @@ import * as Rx from 'rxjs'; import { findTestSubject, mountWithIntl } from '@kbn/test-jest-helpers'; -import { NavControlPopover } from './nav_control_popover'; +import { NavControlPopover, type Props as NavControlPopoverProps } from './nav_control_popover'; import type { Space } from '../../common'; import { EventTracker } from '../analytics'; import { SpaceAvatarInternal } from '../space_avatar/space_avatar_internal'; @@ -49,7 +49,12 @@ const reportEvent = jest.fn(); const eventTracker = new EventTracker({ reportEvent }); describe('NavControlPopover', () => { - async function setup(spaces: Space[], allowSolutionVisibility = false, activeSpace?: Space) { + async function setup( + spaces: Space[], + allowSolutionVisibility = false, + activeSpace?: Space, + props?: Partial + ) { const spacesManager = spacesManagerMock.create(); spacesManager.getSpaces = jest.fn().mockResolvedValue(spaces); @@ -68,6 +73,9 @@ describe('NavControlPopover', () => { navigateToUrl={jest.fn()} allowSolutionVisibility={allowSolutionVisibility} eventTracker={eventTracker} + showTour$={Rx.of(false)} + onFinishTour={jest.fn()} + {...props} /> ); @@ -81,7 +89,7 @@ describe('NavControlPopover', () => { it('renders without crashing', () => { const spacesManager = spacesManagerMock.create(); - const { baseElement } = render( + const { baseElement, queryByTestId } = render( { navigateToUrl={jest.fn()} allowSolutionVisibility={false} eventTracker={eventTracker} + showTour$={Rx.of(false)} + onFinishTour={jest.fn()} /> ); expect(baseElement).toMatchSnapshot(); + expect(queryByTestId('spaceSolutionTour')).toBeNull(); }); it('renders a SpaceAvatar with the active space', async () => { @@ -117,6 +128,8 @@ describe('NavControlPopover', () => { navigateToUrl={jest.fn()} allowSolutionVisibility={false} eventTracker={eventTracker} + showTour$={Rx.of(false)} + onFinishTour={jest.fn()} /> ); @@ -223,20 +236,29 @@ describe('NavControlPopover', () => { }); it('can close its popover', async () => { + jest.useFakeTimers(); const wrapper = await setup(mockSpaces); + expect(findTestSubject(wrapper, 'spaceMenuPopoverPanel').exists()).toEqual(false); // closed + + // Open the popover await act(async () => { wrapper.find(EuiHeaderSectionItemButton).find('button').simulate('click'); }); wrapper.update(); - expect(wrapper.find(EuiPopover).props().isOpen).toEqual(true); + expect(findTestSubject(wrapper, 'spaceMenuPopoverPanel').exists()).toEqual(true); // open + // Close the popover await act(async () => { - wrapper.find(EuiPopover).props().closePopover(); + wrapper.find(EuiHeaderSectionItemButton).find('button').simulate('click'); + }); + act(() => { + jest.runAllTimers(); }); wrapper.update(); + expect(findTestSubject(wrapper, 'spaceMenuPopoverPanel').exists()).toEqual(false); // closed - expect(wrapper.find(EuiPopover).props().isOpen).toEqual(false); + jest.useRealTimers(); }); it('should render solution for spaces', async () => { @@ -301,4 +323,42 @@ describe('NavControlPopover', () => { space_id_prev: 'space-1', }); }); + + it('should show the solution view tour', async () => { + jest.useFakeTimers(); // the unerlying EUI tour component has a timeout that needs to be flushed for thei test to pass + + const spaces: Space[] = [ + { + id: 'space-1', + name: 'Space-1', + disabledFeatures: [], + solution: 'classic', + }, + ]; + + const activeSpace = spaces[0]; + const showTour$ = new Rx.BehaviorSubject(true); + const onFinishTour = jest.fn().mockImplementation(() => { + showTour$.next(false); + }); + + const wrapper = await setup(spaces, true /** allowSolutionVisibility **/, activeSpace, { + showTour$, + onFinishTour, + }); + + expect(findTestSubject(wrapper, 'spaceSolutionTour').exists()).toBe(true); + + act(() => { + findTestSubject(wrapper, 'closeTourBtn').simulate('click'); + }); + act(() => { + jest.runAllTimers(); + }); + wrapper.update(); + + expect(findTestSubject(wrapper, 'spaceSolutionTour').exists()).toBe(false); + + jest.useRealTimers(); + }); }); diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx index 6087747fe3f7d..ea5f6b63c23a1 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx @@ -34,7 +34,7 @@ const LazySpaceAvatar = lazy(() => getSpaceAvatarComponent().then((component) => ({ default: component })) ); -interface Props { +export interface Props { spacesManager: SpacesManager; anchorPosition: PopoverAnchorPosition; capabilities: Capabilities; @@ -158,7 +158,9 @@ class NavControlPopoverUI extends Component { {tourTexts.closeBtn}
} - data-test-subj="spaceSolutionTour" + panelProps={{ + 'data-test-subj': 'spaceSolutionTour', + }} > { repositionOnScroll ownFocus zIndex={Number(theme.euiTheme.levels.navigation) + 1} // it needs to sit above the collapsible nav menu + panelProps={{ + 'data-test-subj': 'spaceMenuPopoverPanel', + }} > {element} @@ -245,6 +250,7 @@ class NavControlPopoverUI extends Component { protected toggleSpaceSelector = () => { const isOpening = !this.state.showSpaceSelector; + if (isOpening) { this.loadSpaces(); } From 54a83642698e8c84b15bb3d579f41d5fdd0ec92c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 4 Oct 2024 16:58:03 +0100 Subject: [PATCH 05/20] Dont show the tour after editing the default space and set a solution --- .../spaces/public/nav_control/nav_control.tsx | 57 +++++++++++++++---- x-pack/plugins/spaces/server/ui_settings.ts | 1 - 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control.tsx index a98070b0139ae..6b25013575dbf 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control.tsx @@ -8,38 +8,71 @@ import { EuiLoadingSpinner } from '@elastic/eui'; import React, { lazy, Suspense } from 'react'; import ReactDOM from 'react-dom'; -import { BehaviorSubject, from, of, switchMap } from 'rxjs'; +import { BehaviorSubject, defer, from, of, shareReplay, switchMap } from 'rxjs'; import type { CoreStart } from '@kbn/core/public'; import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render'; +import type { SolutionView } from '../../common'; import { SHOW_SPACE_SOLUTION_TOUR_SETTING } from '../../common/constants'; import type { EventTracker } from '../analytics'; import type { ConfigType } from '../config'; import type { SpacesManager } from '../spaces_manager'; function initTour(core: CoreStart, spacesManager: SpacesManager) { - const showTourUiSettingValue = core.settings.globalClient.get( - SHOW_SPACE_SOLUTION_TOUR_SETTING, - true - ); - const showTour$ = new BehaviorSubject(showTourUiSettingValue); + const showTourUiSettingValue = core.settings.globalClient.get(SHOW_SPACE_SOLUTION_TOUR_SETTING); + const showTour$ = new BehaviorSubject(showTourUiSettingValue ?? true); - const showTourObservable$ = from(spacesManager.getSpaces()).pipe( - switchMap((spaces) => { - if (spaces.length === 0 || spaces.length > 1) return of(false); // Don't show the tour if there are multiple spaces + const allSpaces$ = defer(() => from(spacesManager.getSpaces())).pipe(shareReplay(1)); + + const hasOnlyOneSpace = (spaces: Array<{ solution?: SolutionView }>) => { + return spaces.length === 1; + }; - const defaultSpace = spaces[0]; - if (!defaultSpace.solution || defaultSpace.solution === 'classic') return of(false); // Don't show the tour if the default space is the classic solution + const isDefaultSpaceOnClassic = (spaces: Array<{ id: string; solution?: SolutionView }>) => { + const defaultSpace = spaces.find((space) => space.id === 'default'); + + if (!defaultSpace) { + // Don't show the tour if the default space doesn't exist (this should never happen) + return true; + } + + if (!defaultSpace.solution || defaultSpace.solution === 'classic') { + return true; + } + }; + + const showTourObservable$ = allSpaces$.pipe( + switchMap((spaces) => { + // Don't show the tour if there are multiple spaces or the default space is the classic solution + if (!hasOnlyOneSpace(spaces) || isDefaultSpaceOnClassic(spaces)) return of(false); return showTour$.asObservable(); }) ); - const onFinishTour = () => { + const hideTourInGlobalSettings = () => { core.settings.globalClient.set(SHOW_SPACE_SOLUTION_TOUR_SETTING, false).catch(() => { // Silently swallow errors, the user will just see the tour again next time they load the page }); + }; + + allSpaces$.subscribe((spaces) => { + if ( + showTourUiSettingValue === undefined && + hasOnlyOneSpace(spaces) && + isDefaultSpaceOnClassic(spaces) + ) { + // We have only one space and it's the default space with the classic solution. + // We don't want to show the tour after the user edits the default space and sets a solution as + // that means that he already understood the concept of solutions. + // We then immediately hide the tour in the global settings. + hideTourInGlobalSettings(); + } + }); + + const onFinishTour = () => { + hideTourInGlobalSettings(); showTour$.next(false); }; diff --git a/x-pack/plugins/spaces/server/ui_settings.ts b/x-pack/plugins/spaces/server/ui_settings.ts index d57822e028f2a..cfb6c996296da 100644 --- a/x-pack/plugins/spaces/server/ui_settings.ts +++ b/x-pack/plugins/spaces/server/ui_settings.ts @@ -17,7 +17,6 @@ export const getUiSettings = (): Record => { return { [SHOW_SPACE_SOLUTION_TOUR_SETTING]: { schema: schema.boolean(), - value: true, readonly: true, readonlyMode: 'ui', }, From a828c85446498bf542d65c64cb003f3358821569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Mon, 7 Oct 2024 11:17:44 +0100 Subject: [PATCH 06/20] Don't show tour after clicking on "Manage spaces" btn --- .../components/spaces_description.tsx | 4 ++-- .../nav_control/components/spaces_menu.tsx | 3 ++- .../spaces/public/nav_control/nav_control.tsx | 21 ++++++++++--------- .../nav_control/nav_control_popover.test.tsx | 1 - .../nav_control/nav_control_popover.tsx | 19 ++++++++++++----- 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/components/spaces_description.tsx b/x-pack/plugins/spaces/public/nav_control/components/spaces_description.tsx index 982e11ffbf4e7..03667f48f4166 100644 --- a/x-pack/plugins/spaces/public/nav_control/components/spaces_description.tsx +++ b/x-pack/plugins/spaces/public/nav_control/components/spaces_description.tsx @@ -20,7 +20,7 @@ import { getSpacesFeatureDescription } from '../../constants'; interface Props { id: string; isLoading: boolean; - toggleSpaceSelector: () => void; + onClickManageSpaceBtn: () => void; capabilities: Capabilities; navigateToApp: ApplicationStart['navigateToApp']; } @@ -45,7 +45,7 @@ export const SpacesDescription: FC = (props: Props) => { diff --git a/x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx b/x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx index 47f7d840b9bee..29d360fe91f3f 100644 --- a/x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx +++ b/x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx @@ -43,6 +43,7 @@ interface Props { spaces: Space[]; serverBasePath: string; toggleSpaceSelector: () => void; + onClickManageSpaceBtn: () => void; intl: InjectedIntl; capabilities: Capabilities; navigateToApp: ApplicationStart['navigateToApp']; @@ -218,7 +219,7 @@ class SpacesMenuUI extends Component { key="manageSpacesButton" className="spcMenu__manageButton" size="s" - onClick={this.props.toggleSpaceSelector} + onClick={this.props.onClickManageSpaceBtn} capabilities={this.props.capabilities} navigateToApp={this.props.navigateToApp} /> diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control.tsx index 6b25013575dbf..66dc1481270b3 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control.tsx @@ -21,12 +21,13 @@ import type { SpacesManager } from '../spaces_manager'; function initTour(core: CoreStart, spacesManager: SpacesManager) { const showTourUiSettingValue = core.settings.globalClient.get(SHOW_SPACE_SOLUTION_TOUR_SETTING); + const hasValueInUiSettings = showTourUiSettingValue !== undefined; const showTour$ = new BehaviorSubject(showTourUiSettingValue ?? true); const allSpaces$ = defer(() => from(spacesManager.getSpaces())).pipe(shareReplay(1)); - const hasOnlyOneSpace = (spaces: Array<{ solution?: SolutionView }>) => { - return spaces.length === 1; + const hasMultipleSpaces = (spaces: Array<{ solution?: SolutionView }>) => { + return spaces.length > 1; }; const isDefaultSpaceOnClassic = (spaces: Array<{ id: string; solution?: SolutionView }>) => { @@ -45,7 +46,7 @@ function initTour(core: CoreStart, spacesManager: SpacesManager) { const showTourObservable$ = allSpaces$.pipe( switchMap((spaces) => { // Don't show the tour if there are multiple spaces or the default space is the classic solution - if (!hasOnlyOneSpace(spaces) || isDefaultSpaceOnClassic(spaces)) return of(false); + if (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces)) return of(false); return showTour$.asObservable(); }) @@ -59,14 +60,14 @@ function initTour(core: CoreStart, spacesManager: SpacesManager) { allSpaces$.subscribe((spaces) => { if ( - showTourUiSettingValue === undefined && - hasOnlyOneSpace(spaces) && - isDefaultSpaceOnClassic(spaces) + !hasValueInUiSettings && + (hasMultipleSpaces(spaces) || (!hasMultipleSpaces(spaces) && isDefaultSpaceOnClassic(spaces))) ) { - // We have only one space and it's the default space with the classic solution. - // We don't want to show the tour after the user edits the default space and sets a solution as - // that means that he already understood the concept of solutions. - // We then immediately hide the tour in the global settings. + // If we have either (1) multiple space or (2) only one space and it's the default space with the classic solution, + // we don't want to show the tour later on. This can happen in the following scenarios: + // - the user deletes all the spaces but one (and that last space has a solution set) + // - the user edits the default space and sets a solution + // So we can immediately hide the tour in the global settings from now on. hideTourInGlobalSettings(); } }); diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx index 9b0ccfb8e413f..45f32e4e563cf 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx @@ -8,7 +8,6 @@ import { EuiFieldSearch, EuiHeaderSectionItemButton, - EuiPopover, EuiSelectable, EuiSelectableListItem, } from '@elastic/eui'; diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx index ea5f6b63c23a1..800565e1f9372 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx @@ -106,9 +106,13 @@ class NavControlPopoverUI extends Component { { + // No need to show the tour anymore, the user is taking action + this.props.onFinishTour(); + this.toggleSpaceSelector(); + }} /> ); } else { @@ -124,6 +128,11 @@ class NavControlPopoverUI extends Component { activeSpace={this.state.activeSpace} allowSolutionVisibility={this.props.allowSolutionVisibility} eventTracker={this.props.eventTracker} + onClickManageSpaceBtn={() => { + // No need to show the tour anymore, the user is taking action + this.props.onFinishTour(); + this.toggleSpaceSelector(); + }} /> ); } @@ -134,7 +143,7 @@ class NavControlPopoverUI extends Component {

{tourTexts.content}

- + {tourTexts.learnMore}

@@ -272,13 +281,13 @@ export const NavControlPopover = withEuiTheme(NavControlPopoverUI); function getTourTexts(solution?: SolutionView) { const solutionMap: Record = { es: i18n.translate('xpack.spaces.navControl.tour.esSolution', { - defaultMessage: 'search', + defaultMessage: 'Search', }), security: i18n.translate('xpack.spaces.navControl.tour.securitySolution', { - defaultMessage: 'security', + defaultMessage: 'Security', }), oblt: i18n.translate('xpack.spaces.navControl.tour.obltSolution', { - defaultMessage: 'observability', + defaultMessage: 'Observability', }), classic: '', // Tour is not shown for the classic solution }; From cf246a8c5524448476a50b4315c939db4928b8cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Mon, 7 Oct 2024 12:42:28 +0100 Subject: [PATCH 07/20] Add more functional tests --- .../nav_control/nav_control_popover.test.tsx | 2 +- .../solution_tour.ts | 83 +++++++++++++++++-- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx index 45f32e4e563cf..137aa1040de86 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx @@ -324,7 +324,7 @@ describe('NavControlPopover', () => { }); it('should show the solution view tour', async () => { - jest.useFakeTimers(); // the unerlying EUI tour component has a timeout that needs to be flushed for thei test to pass + jest.useFakeTimers(); // the unerlying EUI tour component has a timeout that needs to be flushed for the test to pass const spaces: Space[] = [ { diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts index a8f7ac782ca1f..2c4d3b6bea5e0 100644 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts +++ b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts @@ -5,6 +5,7 @@ * 2.0. */ +import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../ftr_provider_context'; export default function ({ getPageObjects, getService }: FtrProviderContext) { @@ -13,8 +14,25 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const testSubjects = getService('testSubjects'); const spacesService = getService('spaces'); const browser = getService('browser'); + const es = getService('es'); describe('space solution tour', () => { + let version: string | undefined; + + const removeGlobalSettings = async () => { + version = version ?? (await kibanaServer.version.get()); + + await es + .delete( + { id: `config-global:${version}`, index: '.kibana', refresh: true }, + { headers: { 'kbn-xsrf': 'spaces' } } + ) + .catch((error) => { + if (error.statusCode === 404) return; // ignore 404 errors + throw error; + }); + }; + before(async () => { await kibanaServer.savedObjects.cleanStandardList(); }); @@ -24,32 +42,87 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { }); describe('solution tour', () => { + let defaultSpace: any; + before(async () => { + defaultSpace = await spacesService.getSpace('default'); + await PageObjects.common.navigateToUrl('management', 'kibana/spaces', { shouldUseHashForSubUrl: false, }); + await PageObjects.common.sleep(1000); // wait to save the setting }); it('does not show the solution tour for the classic space', async () => { - await testSubjects.missingOrFail('spaceSolutionTour'); + await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); }); it('does show the solution tour if the default space has a solution set', async () => { - const defaultSpace = await spacesService.getSpace('default'); - await spacesService.update('default', { ...defaultSpace, solution: 'es', // set a solution }); + await removeGlobalSettings(); // Make sure we start from a clean state + await browser.refresh(); - await testSubjects.existOrFail('spaceSolutionTour'); + await testSubjects.existOrFail('spaceSolutionTour', { timeout: 3000 }); await testSubjects.click('closeTourBtn'); // close the tour await PageObjects.common.sleep(1000); // wait to save the setting await browser.refresh(); - await testSubjects.missingOrFail('spaceSolutionTour'); // The tour does not appear after refresh + await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); // The tour does not appear after refresh + + await spacesService.update('default', { + ...defaultSpace, + solution: 'classic', // revert to not impact future tests + }); + }); + + it('does now show the solution tour after updating the default space from classic to solution', async () => { + await spacesService.update('default', { + ...defaultSpace, + solution: 'es', // set a solution + }); + + await browser.refresh(); + + // The tour does not appear after refresh, even with the default space with a solution set + await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); + + await spacesService.update('default', { + ...defaultSpace, + solution: 'classic', // revert to not impact future tests + }); + }); + + it('does now show the solution tour after deleting spaces and leave only the default', async () => { + await spacesService.update('default', { + ...defaultSpace, + solution: 'es', // set a solution + }); + + await spacesService.create({ + id: 'foo-space', + name: 'Foo Space', + disabledFeatures: [], + color: '#AABBCC', + }); + + const allSpaces = await spacesService.getAll(); + expect(allSpaces).to.have.length(2); // Make sure we have 2 spaces + + await removeGlobalSettings(); // Make sure we start from a clean state + await browser.refresh(); + + await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); + + await spacesService.delete('foo-space'); + await browser.refresh(); + + // The tour does not appear after refresh, even with 1 space with a solution set + await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); await spacesService.update('default', { ...defaultSpace, From 6e8e7c89305b9b930dc355e3d40fab47cbb687ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Mon, 7 Oct 2024 14:42:28 +0100 Subject: [PATCH 08/20] Refactor --- .../solution_tour.ts | 60 +++++++++---------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts index 2c4d3b6bea5e0..0d3c6d4b7e0b4 100644 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts +++ b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts @@ -6,6 +6,7 @@ */ import expect from '@kbn/expect'; +import type { SolutionView } from '@kbn/spaces-plugin/common'; import { FtrProviderContext } from '../../../ftr_provider_context'; export default function ({ getPageObjects, getService }: FtrProviderContext) { @@ -15,6 +16,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const spacesService = getService('spaces'); const browser = getService('browser'); const es = getService('es'); + const log = getService('log'); describe('space solution tour', () => { let version: string | undefined; @@ -22,6 +24,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const removeGlobalSettings = async () => { version = version ?? (await kibanaServer.version.get()); + log.debug(`Deleting [config-global:${version}] doc from the .kibana index`); + await es .delete( { id: `config-global:${version}`, index: '.kibana', refresh: true }, @@ -42,30 +46,41 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { }); describe('solution tour', () => { - let defaultSpace: any; + let _defaultSpace: any = {}; + + const updateSolutionDefaultSpace = async (solution: SolutionView) => { + log.debug(`Updating default space solution: [${solution}].`); + + await spacesService.update('default', { + ..._defaultSpace, + solution, + }); + }; before(async () => { - defaultSpace = await spacesService.getSpace('default'); + _defaultSpace = await spacesService.getSpace('default'); await PageObjects.common.navigateToUrl('management', 'kibana/spaces', { shouldUseHashForSubUrl: false, }); + await PageObjects.common.sleep(1000); // wait to save the setting }); + afterEach(async () => { + await updateSolutionDefaultSpace('classic'); // revert to not impact future tests + }); + it('does not show the solution tour for the classic space', async () => { await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); }); it('does show the solution tour if the default space has a solution set', async () => { - await spacesService.update('default', { - ...defaultSpace, - solution: 'es', // set a solution - }); - + await updateSolutionDefaultSpace('es'); // set a solution + await PageObjects.common.sleep(500); await removeGlobalSettings(); // Make sure we start from a clean state - await browser.refresh(); + await testSubjects.existOrFail('spaceSolutionTour', { timeout: 3000 }); await testSubjects.click('closeTourBtn'); // close the tour @@ -73,35 +88,19 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await browser.refresh(); await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); // The tour does not appear after refresh - - await spacesService.update('default', { - ...defaultSpace, - solution: 'classic', // revert to not impact future tests - }); }); it('does now show the solution tour after updating the default space from classic to solution', async () => { - await spacesService.update('default', { - ...defaultSpace, - solution: 'es', // set a solution - }); - + await updateSolutionDefaultSpace('es'); // set a solution + await PageObjects.common.sleep(500); await browser.refresh(); // The tour does not appear after refresh, even with the default space with a solution set await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); - - await spacesService.update('default', { - ...defaultSpace, - solution: 'classic', // revert to not impact future tests - }); }); it('does now show the solution tour after deleting spaces and leave only the default', async () => { - await spacesService.update('default', { - ...defaultSpace, - solution: 'es', // set a solution - }); + await updateSolutionDefaultSpace('es'); // set a solution await spacesService.create({ id: 'foo-space', @@ -121,13 +120,8 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await spacesService.delete('foo-space'); await browser.refresh(); - // The tour does not appear after refresh, even with 1 space with a solution set + // The tour still does not appear after refresh, even with 1 space with a solution set await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); - - await spacesService.update('default', { - ...defaultSpace, - solution: 'classic', // revert to not impact future tests - }); }); }); }); From 77d3eb6ee1e6415f20cea99d7e0a7c0a00dffeb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Mon, 7 Oct 2024 15:34:02 +0100 Subject: [PATCH 09/20] Move tour logic to own folder and component --- .../spaces/public/nav_control/nav_control.tsx | 65 +----------- .../nav_control/nav_control_popover.tsx | 89 ++-------------- .../nav_control/solution_view_tour/index.ts | 10 ++ .../nav_control/solution_view_tour/lib.ts | 75 +++++++++++++ .../solution_view_tour/solution_view_tour.tsx | 100 ++++++++++++++++++ 5 files changed, 194 insertions(+), 145 deletions(-) create mode 100644 x-pack/plugins/spaces/public/nav_control/solution_view_tour/index.ts create mode 100644 x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts create mode 100644 x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control.tsx index 66dc1481270b3..1dc888333fdf5 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control.tsx @@ -8,78 +8,15 @@ import { EuiLoadingSpinner } from '@elastic/eui'; import React, { lazy, Suspense } from 'react'; import ReactDOM from 'react-dom'; -import { BehaviorSubject, defer, from, of, shareReplay, switchMap } from 'rxjs'; import type { CoreStart } from '@kbn/core/public'; import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render'; -import type { SolutionView } from '../../common'; -import { SHOW_SPACE_SOLUTION_TOUR_SETTING } from '../../common/constants'; +import { initTour } from './solution_view_tour'; import type { EventTracker } from '../analytics'; import type { ConfigType } from '../config'; import type { SpacesManager } from '../spaces_manager'; -function initTour(core: CoreStart, spacesManager: SpacesManager) { - const showTourUiSettingValue = core.settings.globalClient.get(SHOW_SPACE_SOLUTION_TOUR_SETTING); - const hasValueInUiSettings = showTourUiSettingValue !== undefined; - const showTour$ = new BehaviorSubject(showTourUiSettingValue ?? true); - - const allSpaces$ = defer(() => from(spacesManager.getSpaces())).pipe(shareReplay(1)); - - const hasMultipleSpaces = (spaces: Array<{ solution?: SolutionView }>) => { - return spaces.length > 1; - }; - - const isDefaultSpaceOnClassic = (spaces: Array<{ id: string; solution?: SolutionView }>) => { - const defaultSpace = spaces.find((space) => space.id === 'default'); - - if (!defaultSpace) { - // Don't show the tour if the default space doesn't exist (this should never happen) - return true; - } - - if (!defaultSpace.solution || defaultSpace.solution === 'classic') { - return true; - } - }; - - const showTourObservable$ = allSpaces$.pipe( - switchMap((spaces) => { - // Don't show the tour if there are multiple spaces or the default space is the classic solution - if (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces)) return of(false); - - return showTour$.asObservable(); - }) - ); - - const hideTourInGlobalSettings = () => { - core.settings.globalClient.set(SHOW_SPACE_SOLUTION_TOUR_SETTING, false).catch(() => { - // Silently swallow errors, the user will just see the tour again next time they load the page - }); - }; - - allSpaces$.subscribe((spaces) => { - if ( - !hasValueInUiSettings && - (hasMultipleSpaces(spaces) || (!hasMultipleSpaces(spaces) && isDefaultSpaceOnClassic(spaces))) - ) { - // If we have either (1) multiple space or (2) only one space and it's the default space with the classic solution, - // we don't want to show the tour later on. This can happen in the following scenarios: - // - the user deletes all the spaces but one (and that last space has a solution set) - // - the user edits the default space and sets a solution - // So we can immediately hide the tour in the global settings from now on. - hideTourInGlobalSettings(); - } - }); - - const onFinishTour = () => { - hideTourInGlobalSettings(); - showTour$.next(false); - }; - - return { showTour$: showTourObservable$, onFinishTour }; -} - export function initSpacesNavControl( spacesManager: SpacesManager, core: CoreStart, diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx index 800565e1f9372..d84fac2fdced4 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.tsx @@ -7,13 +7,9 @@ import type { PopoverAnchorPosition, WithEuiThemeProps } from '@elastic/eui'; import { - EuiButtonEmpty, EuiHeaderSectionItemButton, - EuiLink, EuiLoadingSpinner, EuiPopover, - EuiText, - EuiTourStep, withEuiTheme, } from '@elastic/eui'; import React, { Component, lazy, Suspense } from 'react'; @@ -24,7 +20,8 @@ import { i18n } from '@kbn/i18n'; import { SpacesDescription } from './components/spaces_description'; import { SpacesMenu } from './components/spaces_menu'; -import type { SolutionView, Space } from '../../common'; +import { SolutionViewTour } from './solution_view_tour'; +import type { Space } from '../../common'; import type { EventTracker } from '../analytics'; import { getSpaceAvatarComponent } from '../space_avatar'; import type { SpacesManager } from '../spaces_manager'; @@ -57,7 +54,6 @@ interface State { } const popoutContentId = 'headerSpacesMenuContent'; -const tourLearnMoreLink = 'https://ela.st/left-nav'; class NavControlPopoverUI extends Component { private activeSpace$?: Subscription; @@ -97,7 +93,7 @@ class NavControlPopoverUI extends Component { const button = this.getActiveSpaceButton(); const { theme } = this.props; const { activeSpace } = this.state; - const tourTexts = getTourTexts(activeSpace?.solution); + const isTourOpen = Boolean(activeSpace) && this.state.showTour && !this.state.showSpaceSelector; let element: React.ReactNode; @@ -138,38 +134,10 @@ class NavControlPopoverUI extends Component { } return ( - -

{tourTexts.content}

-

- - {tourTexts.learnMore} - -

-
- } - isStepOpen={isTourOpen} - minWidth={300} - maxWidth={360} - onFinish={this.props.onFinishTour} - step={1} - stepsTotal={1} - title={tourTexts.title} - anchorPosition="downCenter" - footerAction={ - - {tourTexts.closeBtn} - - } - panelProps={{ - 'data-test-subj': 'spaceSolutionTour', - }} + { > {element} - + ); } @@ -277,44 +245,3 @@ class NavControlPopoverUI extends Component { } export const NavControlPopover = withEuiTheme(NavControlPopoverUI); - -function getTourTexts(solution?: SolutionView) { - const solutionMap: Record = { - es: i18n.translate('xpack.spaces.navControl.tour.esSolution', { - defaultMessage: 'Search', - }), - security: i18n.translate('xpack.spaces.navControl.tour.securitySolution', { - defaultMessage: 'Security', - }), - oblt: i18n.translate('xpack.spaces.navControl.tour.obltSolution', { - defaultMessage: 'Observability', - }), - classic: '', // Tour is not shown for the classic solution - }; - - const title = !!solution - ? i18n.translate('xpack.spaces.navControl.tour.title', { - defaultMessage: 'You chose the {solution} solution view', - values: { solution: solutionMap[solution] }, - }) - : ''; - - const content = !!solution - ? i18n.translate('xpack.spaces.navControl.tour.content', { - defaultMessage: - 'It provides all the analytics and {solution} features you need. You can switch views or return to the classic navigation from your space settings, or create other spaces with different views.', - values: { solution: solutionMap[solution] }, - }) - : ''; - - return { - title, - content, - closeBtn: i18n.translate('xpack.spaces.navControl.tour.closeBtn', { - defaultMessage: 'Close', - }), - learnMore: i18n.translate('xpack.spaces.navControl.tour.learnMore', { - defaultMessage: 'Learn more', - }), - }; -} diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/index.ts b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/index.ts new file mode 100644 index 0000000000000..d85a76c586925 --- /dev/null +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/index.ts @@ -0,0 +1,10 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export { initTour } from './lib'; + +export { SolutionViewTour } from './solution_view_tour'; diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts new file mode 100644 index 0000000000000..080e210f31fce --- /dev/null +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts @@ -0,0 +1,75 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { BehaviorSubject, defer, from, of, shareReplay, switchMap } from 'rxjs'; + +import type { CoreStart } from '@kbn/core/public'; + +import type { SolutionView } from '../../../common'; +import { SHOW_SPACE_SOLUTION_TOUR_SETTING } from '../../../common/constants'; +import type { SpacesManager } from '../../spaces_manager'; + +export function initTour(core: CoreStart, spacesManager: SpacesManager) { + const showTourUiSettingValue = core.settings.globalClient.get(SHOW_SPACE_SOLUTION_TOUR_SETTING); + const hasValueInUiSettings = showTourUiSettingValue !== undefined; + const showTour$ = new BehaviorSubject(showTourUiSettingValue ?? true); + + const allSpaces$ = defer(() => from(spacesManager.getSpaces())).pipe(shareReplay(1)); + + const hasMultipleSpaces = (spaces: Array<{ solution?: SolutionView }>) => { + return spaces.length > 1; + }; + + const isDefaultSpaceOnClassic = (spaces: Array<{ id: string; solution?: SolutionView }>) => { + const defaultSpace = spaces.find((space) => space.id === 'default'); + + if (!defaultSpace) { + // Don't show the tour if the default space doesn't exist (this should never happen) + return true; + } + + if (!defaultSpace.solution || defaultSpace.solution === 'classic') { + return true; + } + }; + + const showTourObservable$ = allSpaces$.pipe( + switchMap((spaces) => { + // Don't show the tour if there are multiple spaces or the default space is the classic solution + if (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces)) return of(false); + + return showTour$.asObservable(); + }) + ); + + const hideTourInGlobalSettings = () => { + core.settings.globalClient.set(SHOW_SPACE_SOLUTION_TOUR_SETTING, false).catch(() => { + // Silently swallow errors, the user will just see the tour again next time they load the page + }); + }; + + allSpaces$.subscribe((spaces) => { + if ( + !hasValueInUiSettings && + (hasMultipleSpaces(spaces) || (!hasMultipleSpaces(spaces) && isDefaultSpaceOnClassic(spaces))) + ) { + // If we have either (1) multiple space or (2) only one space and it's the default space with the classic solution, + // we don't want to show the tour later on. This can happen in the following scenarios: + // - the user deletes all the spaces but one (and that last space has a solution set) + // - the user edits the default space and sets a solution + // So we can immediately hide the tour in the global settings from now on. + hideTourInGlobalSettings(); + } + }); + + const onFinishTour = () => { + hideTourInGlobalSettings(); + showTour$.next(false); + }; + + return { showTour$: showTourObservable$, onFinishTour }; +} diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx new file mode 100644 index 0000000000000..26ee93862ba09 --- /dev/null +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx @@ -0,0 +1,100 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { EuiButtonEmpty, EuiLink, EuiText, EuiTourStep } from '@elastic/eui'; +import React from 'react'; +import type { FC, PropsWithChildren } from 'react'; + +import { i18n } from '@kbn/i18n'; + +import type { SolutionView } from '../../../common'; + +const tourLearnMoreLink = 'https://ela.st/left-nav'; + +interface Props extends PropsWithChildren<{}> { + solution?: SolutionView; + isTourOpen: boolean; + onFinishTour: () => void; +} + +export const SolutionViewTour: FC = ({ children, solution, isTourOpen, onFinishTour }) => { + const tourTexts = getTourTexts(solution); + + return ( + +

{tourTexts.content}

+

+ + {tourTexts.learnMore} + +

+ + } + isStepOpen={isTourOpen} + minWidth={300} + maxWidth={360} + onFinish={onFinishTour} + step={1} + stepsTotal={1} + title={tourTexts.title} + anchorPosition="downCenter" + footerAction={ + + {tourTexts.closeBtn} + + } + panelProps={{ + 'data-test-subj': 'spaceSolutionTour', + }} + > + <>{children} +
+ ); +}; + +function getTourTexts(solution?: SolutionView) { + const solutionMap: Record = { + es: i18n.translate('xpack.spaces.navControl.tour.esSolution', { + defaultMessage: 'Search', + }), + security: i18n.translate('xpack.spaces.navControl.tour.securitySolution', { + defaultMessage: 'Security', + }), + oblt: i18n.translate('xpack.spaces.navControl.tour.obltSolution', { + defaultMessage: 'Observability', + }), + classic: '', // Tour is not shown for the classic solution + }; + + const title = !!solution + ? i18n.translate('xpack.spaces.navControl.tour.title', { + defaultMessage: 'You chose the {solution} solution view', + values: { solution: solutionMap[solution] }, + }) + : ''; + + const content = !!solution + ? i18n.translate('xpack.spaces.navControl.tour.content', { + defaultMessage: + 'It provides all the analytics and {solution} features you need. You can switch views or return to the classic navigation from your space settings, or create other spaces with different views.', + values: { solution: solutionMap[solution] }, + }) + : ''; + + return { + title, + content, + closeBtn: i18n.translate('xpack.spaces.navControl.tour.closeBtn', { + defaultMessage: 'Close', + }), + learnMore: i18n.translate('xpack.spaces.navControl.tour.learnMore', { + defaultMessage: 'Learn more', + }), + }; +} From c9fca06f0c99920fe5389cdb740728eaf9d3d999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Mon, 7 Oct 2024 17:32:22 +0100 Subject: [PATCH 10/20] Fix functional test --- .../apps/spaces/solution_view_flag_enabled/solution_tour.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts index 0d3c6d4b7e0b4..0ceaad4bdff38 100644 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts +++ b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts @@ -23,6 +23,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const removeGlobalSettings = async () => { version = version ?? (await kibanaServer.version.get()); + version = version.replace(/-SNAPSHOT$/, ''); log.debug(`Deleting [config-global:${version}] doc from the .kibana index`); From 14d59745e3116c03ed3d2f823fe070ef0b3dd37c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Tue, 8 Oct 2024 13:40:01 +0100 Subject: [PATCH 11/20] Move the learn more btn inline --- .../solution_view_tour/solution_view_tour.tsx | 86 ++++++++----------- 1 file changed, 38 insertions(+), 48 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx index 26ee93862ba09..837c29035acca 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx @@ -10,11 +10,33 @@ import React from 'react'; import type { FC, PropsWithChildren } from 'react'; import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n-react'; import type { SolutionView } from '../../../common'; const tourLearnMoreLink = 'https://ela.st/left-nav'; +const LearnMoreLink = () => ( + + {i18n.translate('xpack.spaces.navControl.tour.learnMore', { + defaultMessage: 'Learn more', + })} + +); + +const solutionMap: Record = { + es: i18n.translate('xpack.spaces.navControl.tour.esSolution', { + defaultMessage: 'Search', + }), + security: i18n.translate('xpack.spaces.navControl.tour.securitySolution', { + defaultMessage: 'Security', + }), + oblt: i18n.translate('xpack.spaces.navControl.tour.obltSolution', { + defaultMessage: 'Observability', + }), + classic: '', // Tour is not shown for the classic solution +}; + interface Props extends PropsWithChildren<{}> { solution?: SolutionView; isTourOpen: boolean; @@ -22,17 +44,21 @@ interface Props extends PropsWithChildren<{}> { } export const SolutionViewTour: FC = ({ children, solution, isTourOpen, onFinishTour }) => { - const tourTexts = getTourTexts(solution); + const solutionLabel = solutionMap[solution ?? 'classic']; return ( -

{tourTexts.content}

- - {tourTexts.learnMore} - + , + }} + />

} @@ -42,11 +68,16 @@ export const SolutionViewTour: FC = ({ children, solution, isTourOpen, on onFinish={onFinishTour} step={1} stepsTotal={1} - title={tourTexts.title} + title={i18n.translate('xpack.spaces.navControl.tour.title', { + defaultMessage: 'You chose the {solution} solution view', + values: { solution: solutionLabel }, + })} anchorPosition="downCenter" footerAction={ - {tourTexts.closeBtn} + {i18n.translate('xpack.spaces.navControl.tour.closeBtn', { + defaultMessage: 'Close', + })} } panelProps={{ @@ -57,44 +88,3 @@ export const SolutionViewTour: FC = ({ children, solution, isTourOpen, on
); }; - -function getTourTexts(solution?: SolutionView) { - const solutionMap: Record = { - es: i18n.translate('xpack.spaces.navControl.tour.esSolution', { - defaultMessage: 'Search', - }), - security: i18n.translate('xpack.spaces.navControl.tour.securitySolution', { - defaultMessage: 'Security', - }), - oblt: i18n.translate('xpack.spaces.navControl.tour.obltSolution', { - defaultMessage: 'Observability', - }), - classic: '', // Tour is not shown for the classic solution - }; - - const title = !!solution - ? i18n.translate('xpack.spaces.navControl.tour.title', { - defaultMessage: 'You chose the {solution} solution view', - values: { solution: solutionMap[solution] }, - }) - : ''; - - const content = !!solution - ? i18n.translate('xpack.spaces.navControl.tour.content', { - defaultMessage: - 'It provides all the analytics and {solution} features you need. You can switch views or return to the classic navigation from your space settings, or create other spaces with different views.', - values: { solution: solutionMap[solution] }, - }) - : ''; - - return { - title, - content, - closeBtn: i18n.translate('xpack.spaces.navControl.tour.closeBtn', { - defaultMessage: 'Close', - }), - learnMore: i18n.translate('xpack.spaces.navControl.tour.learnMore', { - defaultMessage: 'Learn more', - }), - }; -} From eae5945919a698b177c759d28c023d74427f22b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Wed, 9 Oct 2024 15:38:24 +0100 Subject: [PATCH 12/20] Update x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts Co-authored-by: Aleh Zasypkin --- .../plugins/spaces/public/nav_control/solution_view_tour/lib.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts index 080e210f31fce..318edc689d890 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts @@ -55,7 +55,7 @@ export function initTour(core: CoreStart, spacesManager: SpacesManager) { allSpaces$.subscribe((spaces) => { if ( !hasValueInUiSettings && - (hasMultipleSpaces(spaces) || (!hasMultipleSpaces(spaces) && isDefaultSpaceOnClassic(spaces))) + (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces)) ) { // If we have either (1) multiple space or (2) only one space and it's the default space with the classic solution, // we don't want to show the tour later on. This can happen in the following scenarios: From b66d6082a65f25347c22116daff969fcccfab286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Wed, 9 Oct 2024 16:08:57 +0100 Subject: [PATCH 13/20] Make it more explicit that the tour is not shown when solution is classic --- .../public/nav_control/solution_view_tour/lib.ts | 5 +---- .../solution_view_tour/solution_view_tour.tsx | 10 ++++++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts index 318edc689d890..5d17328aa06c0 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts @@ -53,10 +53,7 @@ export function initTour(core: CoreStart, spacesManager: SpacesManager) { }; allSpaces$.subscribe((spaces) => { - if ( - !hasValueInUiSettings && - (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces)) - ) { + if (!hasValueInUiSettings && (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces))) { // If we have either (1) multiple space or (2) only one space and it's the default space with the classic solution, // we don't want to show the tour later on. This can happen in the following scenarios: // - the user deletes all the spaces but one (and that last space has a solution set) diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx index 837c29035acca..51004972cbc25 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx @@ -43,8 +43,14 @@ interface Props extends PropsWithChildren<{}> { onFinishTour: () => void; } -export const SolutionViewTour: FC = ({ children, solution, isTourOpen, onFinishTour }) => { - const solutionLabel = solutionMap[solution ?? 'classic']; +export const SolutionViewTour: FC = ({ + children, + solution = 'classic', + isTourOpen: _isTourOpen, + onFinishTour, +}) => { + const solutionLabel = solutionMap[solution]; + const isTourOpen = solution === 'classic' ? false : _isTourOpen; return ( Date: Wed, 9 Oct 2024 17:31:38 +0100 Subject: [PATCH 14/20] Refactor using OnBoardingDefaultSolution --- .../solution_view_tour/solution_view_tour.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx index 51004972cbc25..986a620cec072 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx @@ -9,6 +9,7 @@ import { EuiButtonEmpty, EuiLink, EuiText, EuiTourStep } from '@elastic/eui'; import React from 'react'; import type { FC, PropsWithChildren } from 'react'; +import type { OnBoardingDefaultSolution } from '@kbn/cloud-plugin/common'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; @@ -24,7 +25,7 @@ const LearnMoreLink = () => ( ); -const solutionMap: Record = { +const solutionMap: Record = { es: i18n.translate('xpack.spaces.navControl.tour.esSolution', { defaultMessage: 'Search', }), @@ -34,7 +35,6 @@ const solutionMap: Record = { oblt: i18n.translate('xpack.spaces.navControl.tour.obltSolution', { defaultMessage: 'Observability', }), - classic: '', // Tour is not shown for the classic solution }; interface Props extends PropsWithChildren<{}> { @@ -45,12 +45,12 @@ interface Props extends PropsWithChildren<{}> { export const SolutionViewTour: FC = ({ children, - solution = 'classic', + solution, isTourOpen: _isTourOpen, onFinishTour, }) => { - const solutionLabel = solutionMap[solution]; - const isTourOpen = solution === 'classic' ? false : _isTourOpen; + const solutionLabel = solution && solution !== 'classic' ? solutionMap[solution] : ''; + const isTourOpen = solutionLabel ? _isTourOpen : false; return ( Date: Wed, 9 Oct 2024 18:38:23 +0100 Subject: [PATCH 15/20] Fix jest test --- .../spaces/public/nav_control/nav_control_popover.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx index 137aa1040de86..2bf4c86d82796 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx @@ -331,7 +331,7 @@ describe('NavControlPopover', () => { id: 'space-1', name: 'Space-1', disabledFeatures: [], - solution: 'classic', + solution: 'es', }, ]; From edc1252ba385be9af24ac26f699423ba71ea72f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 11 Oct 2024 15:46:50 +0100 Subject: [PATCH 16/20] Don't load spaces if the tour has been dismissed --- .../nav_control/solution_view_tour/lib.ts | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts index 5d17328aa06c0..061310e9e5c46 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { BehaviorSubject, defer, from, of, shareReplay, switchMap } from 'rxjs'; +import { BehaviorSubject, defer, from, map, of, shareReplay, switchMap } from 'rxjs'; import type { CoreStart } from '@kbn/core/public'; @@ -37,12 +37,19 @@ export function initTour(core: CoreStart, spacesManager: SpacesManager) { } }; - const showTourObservable$ = allSpaces$.pipe( - switchMap((spaces) => { - // Don't show the tour if there are multiple spaces or the default space is the classic solution - if (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces)) return of(false); + const showTourObservable$ = showTour$.pipe( + switchMap((showTour) => { + if (!showTour) return of(false); - return showTour$.asObservable(); + return allSpaces$.pipe( + map((spaces) => { + if (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces)) { + return false; + } + + return true; + }) + ); }) ); @@ -52,16 +59,18 @@ export function initTour(core: CoreStart, spacesManager: SpacesManager) { }); }; - allSpaces$.subscribe((spaces) => { - if (!hasValueInUiSettings && (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces))) { - // If we have either (1) multiple space or (2) only one space and it's the default space with the classic solution, - // we don't want to show the tour later on. This can happen in the following scenarios: - // - the user deletes all the spaces but one (and that last space has a solution set) - // - the user edits the default space and sets a solution - // So we can immediately hide the tour in the global settings from now on. - hideTourInGlobalSettings(); - } - }); + if (!hasValueInUiSettings) { + allSpaces$.subscribe((spaces) => { + if (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces)) { + // If we have either (1) multiple space or (2) only one space and it's the default space with the classic solution, + // we don't want to show the tour later on. This can happen in the following scenarios: + // - the user deletes all the spaces but one (and that last space has a solution set) + // - the user edits the default space and sets a solution + // So we can immediately hide the tour in the global settings from now on. + hideTourInGlobalSettings(); + } + }); + } const onFinishTour = () => { hideTourInGlobalSettings(); From 0889760ad2c44eeb3c9ecdbc9d97db94ef3b6494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 11 Oct 2024 15:57:22 +0100 Subject: [PATCH 17/20] Address CR feedback --- x-pack/test/common/services/spaces.ts | 4 ++-- .../spaces/solution_view_flag_enabled/solution_tour.ts | 10 +++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/x-pack/test/common/services/spaces.ts b/x-pack/test/common/services/spaces.ts index c80353d37ed1f..67da912fb6a54 100644 --- a/x-pack/test/common/services/spaces.ts +++ b/x-pack/test/common/services/spaces.ts @@ -108,9 +108,9 @@ export function SpacesServiceProvider({ getService }: FtrProviderContext) { log.debug(`deleted space id: ${spaceId}`); } - public async getSpace(id: string) { + public async get(id: string) { log.debug(`retrieving space ${id}`); - const { data, status, statusText } = await axios.get(`/api/spaces/space/${id}`); + const { data, status, statusText } = await axios.get(`/api/spaces/space/${id}`); if (status !== 200) { throw new Error( diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts index 0ceaad4bdff38..ac30641002c0e 100644 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts +++ b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts @@ -6,7 +6,7 @@ */ import expect from '@kbn/expect'; -import type { SolutionView } from '@kbn/spaces-plugin/common'; +import type { SolutionView, Space } from '@kbn/spaces-plugin/common'; import { FtrProviderContext } from '../../../ftr_provider_context'; export default function ({ getPageObjects, getService }: FtrProviderContext) { @@ -47,7 +47,11 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { }); describe('solution tour', () => { - let _defaultSpace: any = {}; + let _defaultSpace: Space | undefined = { + id: 'default', + name: 'Default', + disabledFeatures: [], + }; const updateSolutionDefaultSpace = async (solution: SolutionView) => { log.debug(`Updating default space solution: [${solution}].`); @@ -59,7 +63,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { }; before(async () => { - _defaultSpace = await spacesService.getSpace('default'); + _defaultSpace = await spacesService.get('default'); await PageObjects.common.navigateToUrl('management', 'kibana/spaces', { shouldUseHashForSubUrl: false, From 7d2d8a768f00430f744056e27b1a1366cd6aab42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Fri, 11 Oct 2024 16:01:27 +0100 Subject: [PATCH 18/20] Address CR changes --- .../solution_view_tour/solution_view_tour.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx index 986a620cec072..2985231499821 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx @@ -43,14 +43,11 @@ interface Props extends PropsWithChildren<{}> { onFinishTour: () => void; } -export const SolutionViewTour: FC = ({ - children, - solution, - isTourOpen: _isTourOpen, - onFinishTour, -}) => { +export const SolutionViewTour: FC = ({ children, solution, isTourOpen, onFinishTour }) => { const solutionLabel = solution && solution !== 'classic' ? solutionMap[solution] : ''; - const isTourOpen = solutionLabel ? _isTourOpen : false; + if (!solutionLabel) { + return children; + } return ( Date: Fri, 11 Oct 2024 18:37:42 +0100 Subject: [PATCH 19/20] Update snapshot --- .../nav_control_popover.test.tsx.snap | 58 +++++++++---------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/__snapshots__/nav_control_popover.test.tsx.snap b/x-pack/plugins/spaces/public/nav_control/__snapshots__/nav_control_popover.test.tsx.snap index 2f52162a8bb70..af2221e460a32 100644 --- a/x-pack/plugins/spaces/public/nav_control/__snapshots__/nav_control_popover.test.tsx.snap +++ b/x-pack/plugins/spaces/public/nav_control/__snapshots__/nav_control_popover.test.tsx.snap @@ -4,48 +4,44 @@ exports[`NavControlPopover renders without crashing 1`] = `
- + +
From 91a8d7165316a385d42e131272e1266af1cb9da9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Tue, 15 Oct 2024 11:10:31 +0100 Subject: [PATCH 20/20] Address CR changes --- .../nav_control/nav_control_popover.test.tsx | 2 +- .../nav_control/solution_view_tour/lib.ts | 19 +++++++++++-------- .../solution_view_tour/solution_view_tour.tsx | 3 ++- .../solution_tour.ts | 4 ++-- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx index 2bf4c86d82796..f1ba5c9f3f3cf 100644 --- a/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx +++ b/x-pack/plugins/spaces/public/nav_control/nav_control_popover.test.tsx @@ -324,7 +324,7 @@ describe('NavControlPopover', () => { }); it('should show the solution view tour', async () => { - jest.useFakeTimers(); // the unerlying EUI tour component has a timeout that needs to be flushed for the test to pass + jest.useFakeTimers(); // the underlying EUI tour component has a timeout that needs to be flushed for the test to pass const spaces: Space[] = [ { diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts index 061310e9e5c46..7936eea09dab6 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/lib.ts @@ -9,30 +9,33 @@ import { BehaviorSubject, defer, from, map, of, shareReplay, switchMap } from 'r import type { CoreStart } from '@kbn/core/public'; -import type { SolutionView } from '../../../common'; -import { SHOW_SPACE_SOLUTION_TOUR_SETTING } from '../../../common/constants'; +import type { Space } from '../../../common'; +import { + DEFAULT_SPACE_ID, + SHOW_SPACE_SOLUTION_TOUR_SETTING, + SOLUTION_VIEW_CLASSIC, +} from '../../../common/constants'; import type { SpacesManager } from '../../spaces_manager'; export function initTour(core: CoreStart, spacesManager: SpacesManager) { const showTourUiSettingValue = core.settings.globalClient.get(SHOW_SPACE_SOLUTION_TOUR_SETTING); - const hasValueInUiSettings = showTourUiSettingValue !== undefined; const showTour$ = new BehaviorSubject(showTourUiSettingValue ?? true); const allSpaces$ = defer(() => from(spacesManager.getSpaces())).pipe(shareReplay(1)); - const hasMultipleSpaces = (spaces: Array<{ solution?: SolutionView }>) => { + const hasMultipleSpaces = (spaces: Space[]) => { return spaces.length > 1; }; - const isDefaultSpaceOnClassic = (spaces: Array<{ id: string; solution?: SolutionView }>) => { - const defaultSpace = spaces.find((space) => space.id === 'default'); + const isDefaultSpaceOnClassic = (spaces: Space[]) => { + const defaultSpace = spaces.find((space) => space.id === DEFAULT_SPACE_ID); if (!defaultSpace) { // Don't show the tour if the default space doesn't exist (this should never happen) return true; } - if (!defaultSpace.solution || defaultSpace.solution === 'classic') { + if (!defaultSpace.solution || defaultSpace.solution === SOLUTION_VIEW_CLASSIC) { return true; } }; @@ -59,7 +62,7 @@ export function initTour(core: CoreStart, spacesManager: SpacesManager) { }); }; - if (!hasValueInUiSettings) { + if (showTourUiSettingValue !== false) { allSpaces$.subscribe((spaces) => { if (hasMultipleSpaces(spaces) || isDefaultSpaceOnClassic(spaces)) { // If we have either (1) multiple space or (2) only one space and it's the default space with the classic solution, diff --git a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx index 2985231499821..bf80bf92bdf4e 100644 --- a/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx +++ b/x-pack/plugins/spaces/public/nav_control/solution_view_tour/solution_view_tour.tsx @@ -14,6 +14,7 @@ import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; import type { SolutionView } from '../../../common'; +import { SOLUTION_VIEW_CLASSIC } from '../../../common/constants'; const tourLearnMoreLink = 'https://ela.st/left-nav'; @@ -44,7 +45,7 @@ interface Props extends PropsWithChildren<{}> { } export const SolutionViewTour: FC = ({ children, solution, isTourOpen, onFinishTour }) => { - const solutionLabel = solution && solution !== 'classic' ? solutionMap[solution] : ''; + const solutionLabel = solution && solution !== SOLUTION_VIEW_CLASSIC ? solutionMap[solution] : ''; if (!solutionLabel) { return children; } diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts index ac30641002c0e..852a2a83031cd 100644 --- a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts +++ b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/solution_tour.ts @@ -95,7 +95,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); // The tour does not appear after refresh }); - it('does now show the solution tour after updating the default space from classic to solution', async () => { + it('does not show the solution tour after updating the default space from classic to solution', async () => { await updateSolutionDefaultSpace('es'); // set a solution await PageObjects.common.sleep(500); await browser.refresh(); @@ -104,7 +104,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await testSubjects.missingOrFail('spaceSolutionTour', { timeout: 3000 }); }); - it('does now show the solution tour after deleting spaces and leave only the default', async () => { + it('does not show the solution tour after deleting spaces and leave only the default', async () => { await updateSolutionDefaultSpace('es'); // set a solution await spacesService.create({