Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Security Solution][Detections] Removes the Tour for the new features on the Rule Management page introduced in 8.1 #128398

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions x-pack/plugins/security_solution/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
banderror marked this conversation as resolved.
Show resolved Hide resolved
export const RULES_MANAGEMENT_FEATURE_TOUR_STORAGE_KEY =
'securitySolution.rulesManagementPage.newFeaturesTour.v8.1';
10 changes: 5 additions & 5 deletions x-pack/plugins/security_solution/cypress/tasks/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
banderror marked this conversation as resolved.
Show resolved Hide resolved
const tourConfig = {
isTourActive: false,
};
Expand All @@ -317,7 +317,7 @@ export const loginAndWaitForPage = (
if (onBeforeLoadCallback) {
onBeforeLoadCallback(win);
}
disableRulesFeatureTour(win);
disableFeatureTourForRuleManagementPage(win);
},
}
);
Expand All @@ -333,15 +333,15 @@ 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 });
};

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 });
};
Expand All @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ const Popover = React.memo<UtilityBarActionProps>(
iconSide={iconSide}
iconSize={iconSize}
iconType={iconType}
onClick={handleLinkIconClick}
disabled={disabled}
onClick={handleLinkIconClick}
>
{children}
</LinkIcon>
Expand Down Expand Up @@ -119,7 +119,6 @@ export const UtilityBarAction = React.memo<UtilityBarActionProps>(
<BarAction data-test-subj={dataTestSubj}>
{popoverContent ? (
<Popover
onClick={onClick}
dataTestSubj={`${dataTestSubj}-popover`}
disabled={disabled}
color={color}
Expand All @@ -129,6 +128,7 @@ export const UtilityBarAction = React.memo<UtilityBarActionProps>(
ownFocus={ownFocus}
popoverPanelPaddingSize={popoverPanelPaddingSize}
popoverContent={popoverContent}
onClick={onClick}
>
{children}
</Popover>
Expand Down
Original file line number Diff line number Diff line change
@@ -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 `<EuiTourStep>`
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))

<!-- Links -->

[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
Original file line number Diff line number Diff line change
Expand Up @@ -6,47 +6,54 @@
*/

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: <p>{'This feature allows for...'}</p>,
stepsTotal: 2,
children: <></>,
onFinish: noop,
maxWidth: TOUR_POPOVER_WIDTH,
},
{
step: 2,
title: i18n.FEATURE_TOUR_BULK_ACTIONS_STEP_TITLE,
content: <p>{i18n.FEATURE_TOUR_BULK_ACTIONS_STEP}</p>,
title: 'Another feature',
content: <p>{'This another feature allows for...'}</p>,
stepsTotal: 2,
children: <></>,
onFinish: noop,
Expand All @@ -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<RulesFeatureTourContextType | null>(null);

/**
Expand All @@ -71,51 +71,60 @@ const RulesFeatureTourContext = createContext<RulesFeatureTourContextType | null
*/
export const RulesFeatureTourContextProvider: FC = ({ children }) => {
const { storage } = useKibana().services;
const initialStore = useMemo<EuiTourState>(

const restoredState = useMemo<EuiTourState>(
() => ({
...tourConfig,
...(storage.get(RULES_MANAGEMENT_FEATURE_TOUR_STORAGE_KEY) ?? tourConfig),
}),
[storage]
);

const [stepProps, actions, reducerState] = useEuiTour(featuresTourSteps, initialStore);

const finishTour = actions.finishTour;
const goToNextStep = actions.incrementStep;

const inMemoryTableStepProps = useMemo(
() => ({
...stepProps[0],
content: (
<>
<p>{i18n.FEATURE_TOUR_IN_MEMORY_TABLE_STEP}</p>
<EuiSpacer />
<EuiButton color="primary" onClick={goToNextStep}>
{i18n.FEATURE_TOUR_IN_MEMORY_TABLE_STEP_NEXT}
</EuiButton>
</>
),
}),
[stepProps, goToNextStep]
const [tourSteps, tourActions, tourState] = useEuiTour(stepsConfig, restoredState);

const enhancedSteps = useMemo<EuiTourStepProps[]>(() => {
return tourSteps.map((item, index, array) => {
return {
...item,
content: (
<>
{item.content}
<EuiSpacer size="s" />
<EuiFlexGroup responsive={false} gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
<EuiButtonIcon
iconType="arrowLeft"
aria-label="Go to previous step"
display="empty"
disabled={index === 0}
onClick={tourActions.decrementStep}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonIcon
iconType="arrowRight"
aria-label="Go to next step"
display="base"
disabled={index === array.length - 1}
onClick={tourActions.incrementStep}
/>
</EuiFlexItem>
</EuiFlexGroup>
</>
),
};
});
}, [tourSteps, tourActions]);

const providerValue = useMemo<RulesFeatureTourContextType>(
() => ({ 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 (
<RulesFeatureTourContext.Provider value={providerValue}>
Expand Down
Original file line number Diff line number Diff line change
@@ -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",
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -68,8 +67,7 @@ describe('AllRules', () => {
rulesNotInstalled={0}
rulesNotUpdated={0}
/>
</TestProviders>,
{ wrappingComponent: RulesFeatureTourContextProvider }
</TestProviders>
);

await waitFor(() => {
Expand All @@ -92,8 +90,7 @@ describe('AllRules', () => {
rulesNotInstalled={0}
rulesNotUpdated={0}
/>
</TestProviders>,
{ wrappingComponent: RulesFeatureTourContextProvider }
</TestProviders>
);

await waitFor(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const AllRules = React.memo<AllRulesProps>(

return (
<>
<RulesTableToolbar activeTab={activeTab} onTabChange={setActiveTab} loading={loading} />
<RulesTableToolbar activeTab={activeTab} onTabChange={setActiveTab} />
<EuiSpacer />
<RulesTables
createPrePackagedRules={createPrePackagedRules}
Expand Down
Loading