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.

Sign up for GitHub

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] Timeline UI refactor revision - Design feedback #173015

Merged
merged 17 commits into from
Dec 13, 2023
Merged

[Security Solution] Timeline UI refactor revision - Design feedback #173015

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6241ac7
fix: action positions + tour
logeekal Dec 11, 2023
d1bbc9a
tests: add cypress test for bottom bar timeline action
logeekal Dec 11, 2023
f0642f5
fix: add divider between actions
logeekal Dec 11, 2023
3bf2eae
Merge branch 'main' into fix/design_review_feedback
logeekal Dec 11, 2023
9613d4b
fix: tests + types
logeekal Dec 11, 2023
95d307a
fix: types
logeekal Dec 11, 2023
345f6ba
Merge branch 'main' into fix/design_review_feedback
logeekal Dec 11, 2023
0027965
Merge branch 'main' into fix/design_review_feedback
logeekal Dec 11, 2023
97a905c
fix: review feedback
logeekal Dec 12, 2023
8d41f05
fix: unnecessary changes
logeekal Dec 12, 2023
9e951fe
fix: docs feedback
logeekal Dec 13, 2023
e23eae5
fix: better data-test-subj
logeekal Dec 13, 2023
266bffb
Merge branch 'main' into fix/design_review_feedback
logeekal Dec 13, 2023
abe2515
fix: Pascal case for timeline
logeekal Dec 13, 2023
ccf80d9
fix: tests
logeekal Dec 13, 2023
0f52f3f
update creation.cy.ts test
michaelolo24 Dec 13, 2023
ae4bcf6
Merge branch 'main' into fix/design_review_feedback
logeekal Dec 13, 2023
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/public/common/components/super_date_picker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import dateMath from '@kbn/datemath';
import type {
EuiSuperDatePickerProps,
EuiSuperDatePickerRecentRange,
EuiSuperUpdateButtonProps,
OnRefreshChangeProps,
OnRefreshProps,
OnTimeChangeProps,
Expand Down Expand Up @@ -42,6 +43,10 @@ import {
} from './selectors';
import type { Inputs } from '../../store/inputs/model';

const refreshButtonProps: EuiSuperUpdateButtonProps = {
fill: false,
};

const MAX_RECENTLY_USED_RANGES = 9;

interface Range {
Expand Down Expand Up @@ -219,6 +224,7 @@ export const SuperDatePickerComponent = React.memo<SuperDatePickerProps>(
isDisabled={disabled}
width={width}
compressed={compressed}
updateButtonProps={refreshButtonProps}
/>
);
},
Expand Down
82 changes: 61 additions & 21 deletions ...ns/security_solution/public/timelines/components/flyout/__snapshots__/index.test.tsx.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 17 additions & 5 deletions x-pack/plugins/security_solution/public/timelines/components/flyout/action_menu/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import React from 'react';
import styled from 'styled-components';
import { useKibana } from '../../../../common/lib/kibana/kibana_react';
import { APP_ID } from '../../../../../common';
import type { TimelineTabs } from '../../../../../common/types';
Expand All @@ -25,6 +26,12 @@ interface TimelineActionMenuProps {
activeTab: TimelineTabs;
}

const VerticalDivider = styled.span`
width: 0px;
height: 20px;
border-left: 1px solid ${({ theme }) => theme.eui.euiColorLightShade};
`;

const TimelineActionMenuComponent = ({
mode = 'normal',
timelineId,
Expand All @@ -49,11 +56,6 @@ const TimelineActionMenuComponent = ({
<EuiFlexItem data-test-subj="open-timeline-action">
<OpenTimelineAction />
</EuiFlexItem>
{userCasesPermissions.create && userCasesPermissions.read ? (
<EuiFlexItem>
<AddToCaseButton timelineId={timelineId} />
</EuiFlexItem>
) : null}
<EuiFlexItem data-test-subj="inspect-timeline-action">
<InspectButton
compact={mode === 'compact'}
Expand All @@ -63,6 +65,16 @@ const TimelineActionMenuComponent = ({
title=""
/>
</EuiFlexItem>
{userCasesPermissions.create && userCasesPermissions.read ? (
<>
<EuiFlexItem>
<VerticalDivider />
</EuiFlexItem>
<EuiFlexItem>
<AddToCaseButton timelineId={timelineId} />
</EuiFlexItem>
</>
) : null}
PhilippeOberti marked this conversation as resolved.
Show resolved Hide resolved
<EuiFlexItem data-test-subj="save-timeline-action">
<SaveTimelineButton timelineId={timelineId} />
</EuiFlexItem>
Expand Down
2 changes: 1 addition & 1 deletion ...lugins/security_solution/public/timelines/components/flyout/add_timeline_button/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ const AddTimelineButtonComponent: React.FC<AddTimelineButtonComponentProps> = ({
className={ADD_TIMELINE_BUTTON_CLASS_NAME}
data-test-subj="settings-plus-in-circle"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you end up updating the PR, mind giving this a more timeline related data-test-subj? Something like new-timeline-header-action or something specific to timeline that doesn't conflict with the existing new-timeline-action? No worries, if you don't make any additional changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure @michaelolo24 , I will check the impact on test, if it is not that big. I will go ahead and change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelolo24 What do you think about timeline-create-open-control?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works perfectly, thanks!

iconType="plusInCircle"
iconSize="m"
color="primary"
size="m"
onClick={onButtonClick}
aria-label={i18n.ADD_TIMELINE}
/>
Expand Down
12 changes: 9 additions & 3 deletions x-pack/plugins/security_solution/public/timelines/components/flyout/header/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { TimelineActionMenu } from '../action_menu';
import { AddToFavoritesButton } from '../../timeline/properties/helpers';
import { TimelineStatusInfo } from './timeline_status_info';
import { timelineDefaults } from '../../../store/timeline/defaults';
import { AddTimelineButton } from '../add_timeline_button';

interface FlyoutHeaderPanelProps {
timelineId: string;
Expand Down Expand Up @@ -141,6 +142,14 @@ const FlyoutHeaderPanelComponent: React.FC<FlyoutHeaderPanelProps> = ({ timeline
>
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="xs" alignItems="center" responsive={false}>
{!show ? (
<EuiFlexItem grow={false}>
<AddTimelineButton timelineId={timelineId} />
</EuiFlexItem>
) : null}
PhilippeOberti marked this conversation as resolved.
Show resolved Hide resolved
<EuiFlexItem grow={false}>
<AddToFavoritesButton timelineId={timelineId} compact />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<ActiveTimelinesContainer grow={false}>
<ActiveTimelines
Expand All @@ -154,9 +163,6 @@ const FlyoutHeaderPanelComponent: React.FC<FlyoutHeaderPanelProps> = ({ timeline
<EuiFlexItem grow={false}>
<TimelineStatusInfo status={timelineStatus} updated={updated} changed={changed} />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<AddToFavoritesButton timelineId={timelineId} compact />
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
{show && (
Expand Down
14 changes: 1 addition & 13 deletions ...security_solution/public/timelines/components/flyout/header/timeline_status_info.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,6 @@ describe('TestComponent', () => {

it('should render the status correctly when timeline has unsaved changes', () => {
render(<TestComponent status={TimelineStatus.active} changed={true} updated={Date.now()} />);
expect(screen.getByText('Has unsaved changes')).toBeVisible();
});

it('should render the status correctly when timeline is saved', () => {
const updatedTime = Date.now();
render(<TestComponent status={TimelineStatus.active} updated={updatedTime} />);
expect(screen.getByText('Saved')).toBeVisible();
});

it('should render the status correctly when timeline is saved some time ago', () => {
const updatedTime = Date.now() - 10000;
render(<TestComponent status={TimelineStatus.active} updated={updatedTime} />);
expect(screen.getByTestId('timeline-status')).toHaveTextContent(/Saved10 seconds ago/);
expect(screen.getByText('Unsaved changes')).toBeVisible();
});
});
21 changes: 6 additions & 15 deletions ...gins/security_solution/public/timelines/components/flyout/header/timeline_status_info.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
*/

import React from 'react';
import { EuiTextColor, EuiText } from '@elastic/eui';
import { FormattedRelative } from '@kbn/i18n-react';
import { EuiText, EuiBadge } from '@elastic/eui';

import styled from 'styled-components';
import { TimelineStatus } from '../../../../../common/api/timeline';
Expand All @@ -29,21 +28,13 @@ export const TimelineStatusInfo = React.memo<TimelineStatusInfoProps>(

let statusContent: React.ReactNode = null;
if (isUnsaved || !updated) {
statusContent = <EuiTextColor color="warning">{i18n.UNSAVED}</EuiTextColor>;
statusContent = <EuiBadge color="warning">{i18n.UNSAVED}</EuiBadge>;
} else if (changed) {
statusContent = <EuiTextColor color="warning">{i18n.UNSAVED_CHANGES}</EuiTextColor>;
} else {
statusContent = (
<>
{i18n.SAVED}
<FormattedRelative
data-test-subj="timeline-status"
key="timeline-status-autosaved"
value={new Date(updated)}
/>
</>
);
statusContent = <EuiBadge color="warning">{i18n.UNSAVED_CHANGES}</EuiBadge>;
}

if (!statusContent) return null;

return (
<NoWrapText size="xs" data-test-subj="timeline-status">
{statusContent}
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/public/timelines/components/flyout/header/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const SAVED = i18n.translate('xpack.securitySolution.timeline.properties.
export const UNSAVED_CHANGES = i18n.translate(
'xpack.securitySolution.timeline.properties.hasChangesLabel',
{
defaultMessage: 'Has unsaved changes',
defaultMessage: 'Unsaved changes',
}
);

Expand Down
20 changes: 18 additions & 2 deletions x-pack/plugins/security_solution/public/timelines/components/timeline/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { defaultHeaders } from './body/column_headers/default_headers';
import type { CellValueElementProps } from './cell_rendering';
import { SourcererScopeName } from '../../../common/store/sourcerer/model';
import { FlyoutHeaderPanel } from '../flyout/header';
import type { TimelineId, RowRenderer } from '../../../../common/types/timeline';
import type { TimelineId, RowRenderer, TimelineTabs } from '../../../../common/types/timeline';
import { TimelineType } from '../../../../common/api/timeline';
import { useDeepEqualSelector, useShallowEqualSelector } from '../../../common/hooks/use_selector';
import { activeTimeline } from '../../containers/active_timeline_context';
Expand Down Expand Up @@ -82,6 +82,7 @@ const StatefulTimelineComponent: React.FC<Props> = ({
initialized,
show: isOpen,
isLoading,
activeTab,
} = useDeepEqualSelector((state) =>
pick(
[
Expand All @@ -95,6 +96,7 @@ const StatefulTimelineComponent: React.FC<Props> = ({
'initialized',
'show',
'isLoading',
'activeTab',
],
getTimeline(state, timelineId) ?? timelineDefaults
)
Expand Down Expand Up @@ -195,6 +197,18 @@ const StatefulTimelineComponent: React.FC<Props> = ({

const showTimelineTour = isOpen && !isLoading && canEditTimeline;

const handleSwitchToTab = useCallback(
(tab: TimelineTabs) => {
dispatch(
timelineActions.setActiveTabTimeline({
id: timelineId,
activeTab: tab,
})
);
},
[timelineId, dispatch]
);

return (
<TimelineContext.Provider value={timelineContext}>
<TimelineContainer
Expand Down Expand Up @@ -230,7 +244,9 @@ const StatefulTimelineComponent: React.FC<Props> = ({
/>
</div>
</TimelineContainer>
{showTimelineTour ? <TimelineTour /> : null}
{showTimelineTour ? (
<TimelineTour activeTab={activeTab} switchToTab={handleSwitchToTab} />
) : null}
PhilippeOberti marked this conversation as resolved.
Show resolved Hide resolved
</TimelineContext.Provider>
);
};
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/security_solution/public/timelines/components/timeline/properties/helpers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { useShallowEqualSelector } from '../../../../common/hooks/use_selector';
import * as i18n from './translations';
import { useCreateTimelineButton } from './use_create_timeline';
import { timelineDefaults } from '../../../store/timeline/defaults';
import { TIMELINE_TOUR_CONFIG_ANCHORS } from '../tour/step_config';

const NotesCountBadge = styled(EuiBadge)`
margin-left: 5px;
Expand Down Expand Up @@ -56,7 +57,9 @@ const AddToFavoritesButtonComponent: React.FC<AddToFavoritesButtonProps> = ({

return compact ? (
<EuiButtonIcon
id={TIMELINE_TOUR_CONFIG_ANCHORS.ADD_TO_FAVORITES}
iconType={isFavorite ? 'starFilled' : 'starEmpty'}
iconSize="m"
isSelected={isFavorite}
onClick={handleClick}
data-test-subj={`timeline-favorite-${isFavorite ? 'filled' : 'empty'}-star`}
Expand All @@ -66,6 +69,7 @@ const AddToFavoritesButtonComponent: React.FC<AddToFavoritesButtonProps> = ({
/>
) : (
<EuiButton
id={TIMELINE_TOUR_CONFIG_ANCHORS.ADD_TO_FAVORITES}
isSelected={isFavorite}
fill={isFavorite}
iconType={isFavorite ? 'starFilled' : 'starEmpty'}
Expand Down
11 changes: 10 additions & 1 deletion x-pack/plugins/security_solution/public/timelines/components/timeline/tour/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,18 @@ import { TIMELINE_TOUR_CONFIG_ANCHORS } from './step_config';
import { useIsElementMounted } from '../../../../detection_engine/rule_management_ui/components/rules_table/rules_table/guided_onboarding/use_is_element_mounted';
import { render, screen, waitFor, fireEvent } from '@testing-library/react';
import { TestProviders } from '../../../../common/mock';
import { TimelineTabs } from '../../../../../common/types';

jest.mock(
'../../../../detection_engine/rule_management_ui/components/rules_table/rules_table/guided_onboarding/use_is_element_mounted'
);

const switchTabMock = jest.fn();

const TestComponent = () => {
return (
<TestProviders>
<TimelineTour />
<TimelineTour activeTab={TimelineTabs.query} switchToTab={switchTabMock} />
{Object.values(TIMELINE_TOUR_CONFIG_ANCHORS).map((anchor) => {
return <div id={anchor} key={anchor} />;
})}
Expand Down Expand Up @@ -58,6 +61,12 @@ describe('Timeline Tour', () => {

fireEvent.click(screen.getByText('Next'));

await waitFor(() => {
expect(screen.getByTestId('timeline-tour-step-4')).toBeVisible();
});

fireEvent.click(screen.getByText('Next'));

await waitFor(() => {
expect(screen.queryByText('Finish tour')).toBeVisible();
});
Expand Down
Loading