diff --git a/x-pack/plugins/security_solution/common/constants.ts b/x-pack/plugins/security_solution/common/constants.ts index cc64b7e640f1f..91545e25057d7 100644 --- a/x-pack/plugins/security_solution/common/constants.ts +++ b/x-pack/plugins/security_solution/common/constants.ts @@ -432,5 +432,11 @@ export const LIMITED_CONCURRENCY_ROUTE_TAG_PREFIX = `${APP_ID}:limitedConcurrenc export const RULES_TABLE_MAX_PAGE_SIZE = 100; export const RULES_TABLE_PAGE_SIZE_OPTIONS = [5, 10, 20, 50, RULES_TABLE_MAX_PAGE_SIZE]; +/** + * A local storage key we use to store the state of the feature tour UI for the Rule Management page. + * + * NOTE: As soon as we want to show a new tour for features in the current Kibana version, + * we will need to update this constant with the corresponding version. + */ export const RULES_MANAGEMENT_FEATURE_TOUR_STORAGE_KEY = 'securitySolution.rulesManagementPage.newFeaturesTour.v8.1'; diff --git a/x-pack/plugins/security_solution/cypress/tasks/login.ts b/x-pack/plugins/security_solution/cypress/tasks/login.ts index de68a3f41d57d..55b1eb973d4fb 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/login.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/login.ts @@ -290,7 +290,7 @@ export const getEnvAuth = (): User => { * It prevents tour to appear during tests and cover UI elements * @param window - browser's window object */ -const disableRulesFeatureTour = (window: Window) => { +const disableFeatureTourForRuleManagementPage = (window: Window) => { const tourConfig = { isTourActive: false, }; @@ -317,7 +317,7 @@ export const loginAndWaitForPage = ( if (onBeforeLoadCallback) { onBeforeLoadCallback(win); } - disableRulesFeatureTour(win); + disableFeatureTourForRuleManagementPage(win); }, } ); @@ -333,7 +333,7 @@ export const waitForPage = (url: string) => { export const loginAndWaitForPageWithoutDateRange = (url: string, role?: ROLES) => { login(role); cy.visit(role ? getUrlWithRoute(role, url) : url, { - onBeforeLoad: disableRulesFeatureTour, + onBeforeLoad: disableFeatureTourForRuleManagementPage, }); cy.get('[data-test-subj="headerGlobalNav"]', { timeout: 120000 }); }; @@ -341,7 +341,7 @@ export const loginAndWaitForPageWithoutDateRange = (url: string, role?: ROLES) = export const loginWithUserAndWaitForPageWithoutDateRange = (url: string, user: User) => { loginWithUser(user); cy.visit(constructUrlWithUser(user, url), { - onBeforeLoad: disableRulesFeatureTour, + onBeforeLoad: disableFeatureTourForRuleManagementPage, }); cy.get('[data-test-subj="headerGlobalNav"]', { timeout: 120000 }); }; @@ -351,7 +351,7 @@ export const loginAndWaitForTimeline = (timelineId: string, role?: ROLES) => { login(role); cy.visit(role ? getUrlWithRoute(role, route) : route, { - onBeforeLoad: disableRulesFeatureTour, + onBeforeLoad: disableFeatureTourForRuleManagementPage, }); cy.get('[data-test-subj="headerGlobalNav"]'); cy.get(TIMELINE_FLYOUT_BODY).should('be.visible'); diff --git a/x-pack/plugins/security_solution/public/common/components/utility_bar/utility_bar_action.tsx b/x-pack/plugins/security_solution/public/common/components/utility_bar/utility_bar_action.tsx index 58a84d8dc8548..60d895e417ce7 100644 --- a/x-pack/plugins/security_solution/public/common/components/utility_bar/utility_bar_action.tsx +++ b/x-pack/plugins/security_solution/public/common/components/utility_bar/utility_bar_action.tsx @@ -57,8 +57,8 @@ const Popover = React.memo( iconSide={iconSide} iconSize={iconSize} iconType={iconType} - onClick={handleLinkIconClick} disabled={disabled} + onClick={handleLinkIconClick} > {children} @@ -119,7 +119,6 @@ export const UtilityBarAction = React.memo( {popoverContent ? ( ( ownFocus={ownFocus} popoverPanelPaddingSize={popoverPanelPaddingSize} popoverContent={popoverContent} + onClick={onClick} > {children} diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/feature_tour/README.md b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/feature_tour/README.md new file mode 100644 index 0000000000000..282ee8c46cd9e --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/feature_tour/README.md @@ -0,0 +1,56 @@ +# Feature Tour on the Rule Management Page + +This folder contains an implementation of a feature tour UI for new features introduced in `8.1.0`. +This implementaion is currently unused - all usages have been removed from React components. +We might revisit this implementation in the next releases when we have something new for the user +to demonstrate on the Rule Management Page. + +## A new way of building tours + +The EUI Tour has evolved and continues to do so. + +EUI folks have implemented a new programming model for defining tour steps and binding them to +UI elements on a page ([ticket][1], [PR][2]). When we revisit the Tour UI, we should build it +differently - using the new `anchor` property and consolidating all the tour steps and logic +in a single component. We shouldn't need to wrap the page with the provider anymore. And there's +[a chance][3] that this implementation based on query selectors will have fewer UI glitches. + +New features and fixes to track: + +- Support for previous, next and go to step [#4831][4] +- Built-in 'Next' button [#5715][5] +- Popover on the EuiTour component doesn't respect the anchorPosition prop [#5731][6] + +## How to revive this tour for the next release (if needed) + +1. Update Kibana version in `RULES_MANAGEMENT_FEATURE_TOUR_STORAGE_KEY`. + Set it to a version you're going to implement a feature tour for. + +1. Define steps for your tour. See `RulesFeatureTourContextProvider` and `stepsConfig`. + +1. Rewrite the implementation using the new `anchor` property and targeting UI elements + from steps using query selectors. Consolidate all the steps and their `` + components in a single `RuleManagementPageFeatureTour` component. Render this component + in the Rule Management page. Get rid of `RulesFeatureTourContextProvider` - we shouldn't + need to wrap the page and pass anything down the tree anymore. + +1. Consider abstracting away persistence in Local Storage and other functionality that + may be common to tours on different pages. + +## Useful links + +Docs: [`EuiTour`](https://elastic.github.io/eui/#/display/tour). + +For reference, PRs where this Tour has been introduced or changed: + +- added in `8.1.0` ([PR](https://github.com/elastic/kibana/pull/124343)) +- removed in `8.2.0` ([PR](https://github.com/elastic/kibana/pull/128398)) + + + +[1]: https://github.com/elastic/kibana/issues/124052 +[2]: https://github.com/elastic/eui/pull/5696 +[3]: https://github.com/elastic/eui/issues/5731#issuecomment-1075202910 +[4]: https://github.com/elastic/eui/issues/4831 +[5]: https://github.com/elastic/eui/issues/5715 +[6]: https://github.com/elastic/eui/issues/5731 diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_feature_tour_context.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/feature_tour/rules_feature_tour_context.tsx similarity index 52% rename from x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_feature_tour_context.tsx rename to x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/feature_tour/rules_feature_tour_context.tsx index 6c1d5a0de7a54..aaa483e49fca7 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_feature_tour_context.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/feature_tour/rules_feature_tour_context.tsx @@ -6,38 +6,45 @@ */ import React, { createContext, useContext, useEffect, useMemo, FC } from 'react'; - -import { noop } from 'lodash'; import { - useEuiTour, + EuiButtonIcon, + EuiFlexGroup, + EuiFlexItem, + EuiSpacer, EuiTourState, EuiStatelessTourStep, - EuiSpacer, - EuiButton, EuiTourStepProps, + EuiTourActions, + useEuiTour, } from '@elastic/eui'; -import { invariant } from '../../../../../../common/utils/invariant'; -import { RULES_MANAGEMENT_FEATURE_TOUR_STORAGE_KEY } from '../../../../../../common/constants'; -import { useKibana } from '../../../../../common/lib/kibana'; -import * as i18n from '../translations'; +import { noop } from 'lodash'; +import { invariant } from '../../../../../../../common/utils/invariant'; +import { useKibana } from '../../../../../../common/lib/kibana'; +import { RULES_MANAGEMENT_FEATURE_TOUR_STORAGE_KEY } from '../../../../../../../common/constants'; + +import * as i18n from './translations'; export interface RulesFeatureTourContextType { - steps: { - inMemoryTableStepProps: EuiTourStepProps; - bulkActionsStepProps: EuiTourStepProps; - }; - goToNextStep: () => void; - finishTour: () => void; + steps: EuiTourStepProps[]; + actions: EuiTourActions; } const TOUR_POPOVER_WIDTH = 360; -const featuresTourSteps: EuiStatelessTourStep[] = [ +const tourConfig: EuiTourState = { + currentTourStep: 1, + isTourActive: true, + tourPopoverWidth: TOUR_POPOVER_WIDTH, + tourSubtitle: i18n.TOUR_TITLE, +}; + +// This is an example. Replace with the steps for your particular version. Don't forget to use i18n. +const stepsConfig: EuiStatelessTourStep[] = [ { step: 1, - title: i18n.FEATURE_TOUR_IN_MEMORY_TABLE_STEP_TITLE, - content: <>, + title: 'A new feature', + content:

{'This feature allows for...'}

, stepsTotal: 2, children: <>, onFinish: noop, @@ -45,8 +52,8 @@ const featuresTourSteps: EuiStatelessTourStep[] = [ }, { step: 2, - title: i18n.FEATURE_TOUR_BULK_ACTIONS_STEP_TITLE, - content:

{i18n.FEATURE_TOUR_BULK_ACTIONS_STEP}

, + title: 'Another feature', + content:

{'This another feature allows for...'}

, stepsTotal: 2, children: <>, onFinish: noop, @@ -55,13 +62,6 @@ const featuresTourSteps: EuiStatelessTourStep[] = [ }, ]; -const tourConfig: EuiTourState = { - currentTourStep: 1, - isTourActive: true, - tourPopoverWidth: TOUR_POPOVER_WIDTH, - tourSubtitle: i18n.FEATURE_TOUR_TITLE, -}; - const RulesFeatureTourContext = createContext(null); /** @@ -71,7 +71,8 @@ const RulesFeatureTourContext = createContext { const { storage } = useKibana().services; - const initialStore = useMemo( + + const restoredState = useMemo( () => ({ ...tourConfig, ...(storage.get(RULES_MANAGEMENT_FEATURE_TOUR_STORAGE_KEY) ?? tourConfig), @@ -79,43 +80,51 @@ export const RulesFeatureTourContextProvider: FC = ({ children }) => { [storage] ); - const [stepProps, actions, reducerState] = useEuiTour(featuresTourSteps, initialStore); - - const finishTour = actions.finishTour; - const goToNextStep = actions.incrementStep; - - const inMemoryTableStepProps = useMemo( - () => ({ - ...stepProps[0], - content: ( - <> -

{i18n.FEATURE_TOUR_IN_MEMORY_TABLE_STEP}

- - - {i18n.FEATURE_TOUR_IN_MEMORY_TABLE_STEP_NEXT} - - - ), - }), - [stepProps, goToNextStep] + const [tourSteps, tourActions, tourState] = useEuiTour(stepsConfig, restoredState); + + const enhancedSteps = useMemo(() => { + return tourSteps.map((item, index, array) => { + return { + ...item, + content: ( + <> + {item.content} + + + + + + + + + + + ), + }; + }); + }, [tourSteps, tourActions]); + + const providerValue = useMemo( + () => ({ steps: enhancedSteps, actions: tourActions }), + [enhancedSteps, tourActions] ); useEffect(() => { - const { isTourActive, currentTourStep } = reducerState; + const { isTourActive, currentTourStep } = tourState; storage.set(RULES_MANAGEMENT_FEATURE_TOUR_STORAGE_KEY, { isTourActive, currentTourStep }); - }, [reducerState, storage]); - - const providerValue = useMemo( - () => ({ - steps: { - inMemoryTableStepProps, - bulkActionsStepProps: stepProps[1], - }, - finishTour, - goToNextStep, - }), - [finishTour, goToNextStep, inMemoryTableStepProps, stepProps] - ); + }, [tourState, storage]); return ( diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/feature_tour/translations.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/feature_tour/translations.ts new file mode 100644 index 0000000000000..bfcda64bb13dd --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/feature_tour/translations.ts @@ -0,0 +1,15 @@ +/* + * 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 { i18n } from '@kbn/i18n'; + +export const TOUR_TITLE = i18n.translate( + 'xpack.securitySolution.detectionEngine.rules.allRules.featureTour.tourTitle', + { + defaultMessage: "What's new", + } +); diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx index 6092ec2a134d1..3b24dda539174 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx @@ -13,7 +13,6 @@ import { TestProviders } from '../../../../../common/mock'; import '../../../../../common/mock/formatted_relative'; import '../../../../../common/mock/match_media'; import { AllRules } from './index'; -import { RulesFeatureTourContextProvider } from './rules_feature_tour_context'; jest.mock('../../../../../common/components/link_to'); jest.mock('../../../../../common/lib/kibana'); @@ -68,8 +67,7 @@ describe('AllRules', () => { rulesNotInstalled={0} rulesNotUpdated={0} /> - , - { wrappingComponent: RulesFeatureTourContextProvider } + ); await waitFor(() => { @@ -92,8 +90,7 @@ describe('AllRules', () => { rulesNotInstalled={0} rulesNotUpdated={0} /> - , - { wrappingComponent: RulesFeatureTourContextProvider } + ); await waitFor(() => { diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.tsx index 6bb9927c8ab82..e8c7742125c74 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.tsx @@ -45,7 +45,7 @@ export const AllRules = React.memo( return ( <> - + = ({ - children, - stepProps, -}) => { - if (!stepProps) { - return <>{children}; - } - - return ( - - <>{children} - - ); -}; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_table_toolbar.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_table_toolbar.tsx index 966cb726c8711..261e14fd1411b 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_table_toolbar.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_table_toolbar.tsx @@ -10,8 +10,6 @@ import React from 'react'; import styled from 'styled-components'; import { useRulesTableContext } from './rules_table/rules_table_context'; import * as i18n from '../translations'; -import { useRulesFeatureTourContext } from './rules_feature_tour_context'; -import { OptionalEuiTourStep } from './optional_eui_tour_step'; const ToolbarLayout = styled.div` display: grid; @@ -24,7 +22,6 @@ const ToolbarLayout = styled.div` interface RulesTableToolbarProps { activeTab: AllRulesTabs; onTabChange: (tab: AllRulesTabs) => void; - loading: boolean; } export enum AllRulesTabs { @@ -46,17 +43,12 @@ const allRulesTabs = [ ]; export const RulesTableToolbar = React.memo( - ({ onTabChange, activeTab, loading }) => { + ({ onTabChange, activeTab }) => { const { state: { isInMemorySorting }, actions: { setIsInMemorySorting }, } = useRulesTableContext(); - const { - steps: { inMemoryTableStepProps }, - goToNextStep, - } = useRulesFeatureTourContext(); - return ( @@ -72,22 +64,13 @@ export const RulesTableToolbar = React.memo( ))} - {/* delaying render of tour due to EuiPopover can't react to layout changes - https://github.com/elastic/kibana/pull/124343#issuecomment-1032467614 */} - - - { - if (inMemoryTableStepProps.isStepOpen) { - goToNextStep(); - } - setIsInMemorySorting(e.target.checked); - }} - /> - - + + setIsInMemorySorting(e.target.checked)} + /> + ); } diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/utility_bar.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/utility_bar.tsx index a936e84cee00a..6d9c2f92b214e 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/utility_bar.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/utility_bar.tsx @@ -22,9 +22,6 @@ import { UtilityBarText, } from '../../../../../common/components/utility_bar'; import * as i18n from '../translations'; -import { useRulesFeatureTourContextOptional } from './rules_feature_tour_context'; - -import { OptionalEuiTourStep } from './optional_eui_tour_step'; interface AllRulesUtilityBarProps { canBulkEdit: boolean; @@ -58,9 +55,6 @@ export const AllRulesUtilityBar = React.memo( isBulkActionInProgress, hasDisabledActions, }) => { - // use optional rulesFeatureTourContext as AllRulesUtilityBar can be used outside the context - const featureTour = useRulesFeatureTourContextOptional(); - const handleGetBulkItemsPopoverContent = useCallback( (closePopover: () => void): JSX.Element | null => { if (onGetBulkItemsPopoverContent != null) { @@ -140,24 +134,17 @@ export const AllRulesUtilityBar = React.memo( )} {canBulkEdit && ( - - { - if (featureTour?.steps?.bulkActionsStepProps?.isStepOpen) { - featureTour?.finishTour(); - } - }} - > - {i18n.BATCH_ACTIONS} - - + + {i18n.BATCH_ACTIONS} + )} { showExceptionsCheckBox showCheckBox /> - - - - - - {loadPrebuiltRulesAndTemplatesButton && ( - {loadPrebuiltRulesAndTemplatesButton} - )} - {reloadPrebuiltRulesAndTemplatesButton && ( - {reloadPrebuiltRulesAndTemplatesButton} - )} - - - - {i18n.UPLOAD_VALUE_LISTS} - - - - + + + + + + {loadPrebuiltRulesAndTemplatesButton && ( + {loadPrebuiltRulesAndTemplatesButton} + )} + {reloadPrebuiltRulesAndTemplatesButton && ( + {reloadPrebuiltRulesAndTemplatesButton} + )} + + - {i18n.IMPORT_RULE} + {i18n.UPLOAD_VALUE_LISTS} - - - - {i18n.ADD_NEW_RULE} - - - - - {(prePackagedRuleStatus === 'ruleNeedUpdate' || - prePackagedTimelineStatus === 'timelineNeedUpdate') && ( - - )} - + + + + {i18n.IMPORT_RULE} + + + + + {i18n.ADD_NEW_RULE} + + + + + {(prePackagedRuleStatus === 'ruleNeedUpdate' || + prePackagedTimelineStatus === 'timelineNeedUpdate') && ( + - - - + )} + + + + ); diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/translations.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/translations.ts index 3a9f233d9bffb..f2f3ef2828e9b 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/translations.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/translations.ts @@ -88,50 +88,6 @@ export const EDIT_PAGE_TITLE = i18n.translate( } ); -export const FEATURE_TOUR_TITLE = i18n.translate( - 'xpack.securitySolution.detectionEngine.rules.allRules.featureTour.tourTitle', - { - defaultMessage: "What's new", - } -); - -export const FEATURE_TOUR_IN_MEMORY_TABLE_STEP = i18n.translate( - 'xpack.securitySolution.detectionEngine.rules.allRules.featureTour.inMemoryTableStepDescription', - { - defaultMessage: - 'The experimental rules table view allows for advanced sorting and filtering capabilities.', - } -); - -export const FEATURE_TOUR_IN_MEMORY_TABLE_STEP_TITLE = i18n.translate( - 'xpack.securitySolution.detectionEngine.rules.allRules.featureTour.inMemoryTableStepTitle', - { - defaultMessage: 'Step 1', - } -); - -export const FEATURE_TOUR_IN_MEMORY_TABLE_STEP_NEXT = i18n.translate( - 'xpack.securitySolution.detectionEngine.rules.allRules.featureTour.inMemoryTableStepNextButtonTitle', - { - defaultMessage: 'Ok, got it', - } -); - -export const FEATURE_TOUR_BULK_ACTIONS_STEP_TITLE = i18n.translate( - 'xpack.securitySolution.detectionEngine.rules.allRules.featureTour.bulkActionsStepTitle', - { - defaultMessage: 'Step 2', - } -); - -export const FEATURE_TOUR_BULK_ACTIONS_STEP = i18n.translate( - 'xpack.securitySolution.detectionEngine.rules.allRules.featureTour.bulkActionsStepDescription', - { - defaultMessage: - 'You can now bulk update index patterns and tags for multiple custom rules at once.', - } -); - export const REFRESH = i18n.translate( 'xpack.securitySolution.detectionEngine.rules.allRules.refreshTitle', {