Skip to content

Commit

Permalink
## [Security Solution] Updates Timeline tooltips to use action words …
Browse files Browse the repository at this point in the history
…/ fewer words

- This PR updates the row-level action button tooltips in Timeline, per issue <#126973>, to use action words / fewer words. This brings the tooltips up to date with the latest [EUI Guidelines](https://elastic.github.io/eui/#/navigation/button/guidelines).
- The tooltips now differentiate between events and alerts. (The original tooltips were written before the Security Solution shipped with a detection engine, so there was no prior distinction.)

### Alert tooltips

This section contains before / after screenshots for alerts.

#### Before: Unpinned alert

![01-BEFORE-unpinned-alert](https://user-images.githubusercontent.com/4459398/169910015-ec5789c2-a691-4e48-b375-c95d15db8378.png)

#### After: Unpinned alert

![01-AFTER-unpinned-alert](https://user-images.githubusercontent.com/4459398/169910149-0cdd8c65-a9b0-43b6-8550-cd0d9c512b17.png)

#### Before: Pinned alert

![02-BEFORE-pinned-alert](https://user-images.githubusercontent.com/4459398/169910207-27aa3307-d33d-4fb5-9ff7-042a6adf030e.png)

#### After: Pinned alert

![02-AFTER-pinned-alert](https://user-images.githubusercontent.com/4459398/169910265-087a3779-2330-437d-b83f-d887eb4adaf9.png)

#### Before: Add a note to an alert

![03-BEFORE-alert-add-note](https://user-images.githubusercontent.com/4459398/169910347-39ca30db-a010-4739-b67f-464851730218.png)

#### After: Add a note to an alert

![03-AFTER-alert-add-note](https://user-images.githubusercontent.com/4459398/169910407-510ff4f3-e0ee-4649-8a44-5b7411a5b629.png)

#### Before: A pinned alert with notes

![04-BEFORE-pinned-alert-with-notes](https://user-images.githubusercontent.com/4459398/169910491-59869c08-2b2b-4c42-a39b-4d4efc05e668.png)

#### After: A pinned alert with notes

![04-AFTER-pinned-alert-with-notes](https://user-images.githubusercontent.com/4459398/169910573-7ae31f88-1199-4744-ae5e-54b40d3c34cb.png)

#### Before: A timeline template with alerts

![05-BEFORE-alert-template](https://user-images.githubusercontent.com/4459398/169910699-03ead1dd-d543-4687-a6f3-14b1b95023aa.png)

#### After: A timeline template with alerts

![05-AFTER-alert-template](https://user-images.githubusercontent.com/4459398/169910740-120eb087-111f-42e9-bb24-af3c344e68c7.png)

### Event tooltips

This section contains before / after screenshots for events.

#### Before: Unpinned event

![06-BEFORE-unpinned-event](https://user-images.githubusercontent.com/4459398/169911532-047d07e2-b4b2-4719-a1a9-fe67c9e04162.png)

#### After: Unpinned event

![06-AFTER-unpinned-event](https://user-images.githubusercontent.com/4459398/169911561-3c452e5b-23de-4144-b0c9-3e805fbd4021.png)

#### Before: Pinned event

![07-BEFORE-pinned-event](https://user-images.githubusercontent.com/4459398/169911699-ffde9799-6120-49e0-8012-37dd687bba31.png)

#### After: Pinned event

![07-AFTER-pinned-event](https://user-images.githubusercontent.com/4459398/169911741-48eeba36-a2c3-4a53-b5f6-57c376b82aa0.png)

#### Before: Add a note to event

![08-BEFORE-event-add-note](https://user-images.githubusercontent.com/4459398/169911867-4139d719-dd5d-4e1b-a415-08863cbf9897.png)

#### After: Add a note to an event

![08-AFTER-event-add-note](https://user-images.githubusercontent.com/4459398/169911895-ea537e8e-9457-4f30-adcc-7ca03c35ea68.png)

#### Before: A pinned event with notes

![09-BEFORE-pinned-event-with-notes](https://user-images.githubusercontent.com/4459398/169911931-9bb662b7-21ea-414b-99c2-46706763ac01.png)

#### After: A pinned event with notes

![09-AFTER-pinned-event-with-notes](https://user-images.githubusercontent.com/4459398/169911975-fc7901ff-61d2-4b6d-b47f-62bf38d4ca16.png)

#### Before: A timeline template with events

![10-BEFORE-event-template](https://user-images.githubusercontent.com/4459398/169911994-4df648ad-bfc7-44ec-bcd5-602a6edb6ca1.png)

#### After: A timeline template with events

![10-AFTER-event-template](https://user-images.githubusercontent.com/4459398/169912017-ac54d03d-11ab-4116-ab33-6d20d5cbba37.png)
  • Loading branch information
andrew-goldstein committed May 23, 2022
1 parent 93de448 commit bbf87c5
Show file tree
Hide file tree
Showing 13 changed files with 313 additions and 108 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import { mount } from 'enzyme';
import React from 'react';

import { TestProviders, mockTimelineModel, mockTimelineData } from '../../../../../common/mock';
import { Actions } from '.';
import { Actions, isAlert } from '.';
import { Ecs } from '../../../../../../common/ecs';
import { mockTimelines } from '../../../../../common/mock/mock_timelines_plugin';
import { useIsExperimentalFeatureEnabled } from '../../../../../common/hooks/use_experimental_features';
import { mockCasesContract } from '@kbn/cases-plugin/public/mocks';
Expand Down Expand Up @@ -221,4 +222,30 @@ describe('Actions', () => {
expect(wrapper.find('[data-test-subj="view-in-analyzer"]').exists()).toBe(false);
});
});

describe('isAlert', () => {
test('it returns true when the ecsData is an alert', () => {
const ecsData: Ecs = {
...mockTimelineData[0].ecs,
kibana: {
alert: {
rule: {
parameters: {},
uuid: ['CBDCAB79-BEDC-4A08-8AC2-29D7500063B9'],
},
},
},
};

expect(isAlert(ecsData)).toBe(true);
});

test('it returns false when ecsData is NOT an alert', () => {
const ecsData: Ecs = {
...mockTimelineData[0].ecs,
};

expect(isAlert(ecsData)).toBe(false);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
useGlobalFullScreen,
useTimelineFullScreen,
} from '../../../../../common/containers/use_full_screen';
import { Ecs } from '../../../../../../common/ecs';
import {
TimelineId,
ActionProps,
Expand All @@ -40,6 +41,8 @@ import { timelineActions, timelineSelectors } from '../../../../store/timeline';
import { timelineDefaults } from '../../../../store/timeline/defaults';
import { isInvestigateInResolverActionEnabled } from '../../../../../detections/components/alerts_table/timeline_actions/investigate_in_resolver';

export const isAlert = (ecsData: Ecs): boolean => getEventType(ecsData) === 'signal';

const ActionsContainer = styled.div`
align-items: center;
display: flex;
Expand Down Expand Up @@ -221,6 +224,7 @@ const ActionsComponent: React.FC<ActionProps> = ({
/>
<PinEventAction
ariaLabel={i18n.PIN_EVENT_FOR_ROW({ ariaRowindex, columnValues, isEventPinned })}
isAlert={isAlert(ecsData)}
key="pin-event"
onPinClicked={handlePinClicked}
noteIds={eventIdToNoteIds ? eventIdToNoteIds[eventId] || emptyNotes : emptyNotes}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { TimelineType } from '../../../../../../common/types/timeline';

interface PinEventActionProps {
ariaLabel?: string;
isAlert: boolean;
noteIds: string[];
onPinClicked: () => void;
eventIsPinned: boolean;
Expand All @@ -24,6 +25,7 @@ interface PinEventActionProps {

const PinEventActionComponent: React.FC<PinEventActionProps> = ({
ariaLabel,
isAlert,
noteIds,
onPinClicked,
eventIsPinned,
Expand All @@ -33,10 +35,11 @@ const PinEventActionComponent: React.FC<PinEventActionProps> = ({
() =>
getPinTooltip({
isPinned: eventIsPinned,
isAlert,
eventHasNotes: eventHasNotes(noteIds),
timelineType,
}),
[eventIsPinned, noteIds, timelineType]
[eventIsPinned, isAlert, noteIds, timelineType]
);

return (
Expand All @@ -47,6 +50,7 @@ const PinEventActionComponent: React.FC<PinEventActionProps> = ({
ariaLabel={ariaLabel}
allowUnpinning={!eventHasNotes(noteIds)}
data-test-subj="pin-event"
isAlert={isAlert}
onClick={onPinClicked}
pinned={eventIsPinned}
timelineType={timelineType}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const getEventsSelectOptions = (): EventsSelectOption[] => [
dropdownDisplay: (
<PinActionItem>
<PinIconContainer>
<Pin allowUnpinning={true} pinned={true} />
<Pin allowUnpinning={true} isAlert={false} pinned={true} />
</PinIconContainer>
<DropdownDisplay text={i18n.PIN_SELECTED} />
</PinActionItem>
Expand All @@ -99,7 +99,7 @@ export const getEventsSelectOptions = (): EventsSelectOption[] => [
dropdownDisplay: (
<PinActionItem>
<PinIconContainer>
<Pin allowUnpinning={true} pinned={false} />
<Pin allowUnpinning={true} isAlert={false} pinned={false} />
</PinIconContainer>
<DropdownDisplay text={i18n.UNPIN_SELECTED} />
</PinActionItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,37 +196,113 @@ describe('helpers', () => {
describe('getPinTooltip', () => {
test('it indicates the event may NOT be unpinned when `isPinned` is `true` and the event has notes', () => {
expect(
getPinTooltip({ isPinned: true, eventHasNotes: true, timelineType: TimelineType.default })
getPinTooltip({
isAlert: false,
isPinned: true,
eventHasNotes: true,
timelineType: TimelineType.default,
})
).toEqual('This event cannot be unpinned because it has notes');
});

test('it indicates the alert may NOT be unpinned when `isPinned` is `true` and the alert has notes', () => {
expect(
getPinTooltip({
isAlert: true,
isPinned: true,
eventHasNotes: true,
timelineType: TimelineType.default,
})
).toEqual('This alert cannot be unpinned because it has notes');
});

test('it indicates the event is pinned when `isPinned` is `true` and the event does NOT have notes', () => {
expect(
getPinTooltip({ isPinned: true, eventHasNotes: false, timelineType: TimelineType.default })
).toEqual('Pinned event');
getPinTooltip({
isAlert: false,
isPinned: true,
eventHasNotes: false,
timelineType: TimelineType.default,
})
).toEqual('Unpin event');
});

test('it indicates the alert is pinned when `isPinned` is `true` and the alert does NOT have notes', () => {
expect(
getPinTooltip({
isAlert: true,
isPinned: true,
eventHasNotes: false,
timelineType: TimelineType.default,
})
).toEqual('Unpin alert');
});

test('it indicates the event is NOT pinned when `isPinned` is `false` and the event has notes', () => {
expect(
getPinTooltip({ isPinned: false, eventHasNotes: true, timelineType: TimelineType.default })
).toEqual('Unpinned event');
getPinTooltip({
isAlert: false,
isPinned: false,
eventHasNotes: true,
timelineType: TimelineType.default,
})
).toEqual('Pin event');
});

test('it indicates the alert is NOT pinned when `isPinned` is `false` and the alert has notes', () => {
expect(
getPinTooltip({
isAlert: true,
isPinned: false,
eventHasNotes: true,
timelineType: TimelineType.default,
})
).toEqual('Pin alert');
});

test('it indicates the event is NOT pinned when `isPinned` is `false` and the event does NOT have notes', () => {
expect(
getPinTooltip({ isPinned: false, eventHasNotes: false, timelineType: TimelineType.default })
).toEqual('Unpinned event');
getPinTooltip({
isAlert: false,
isPinned: false,
eventHasNotes: false,
timelineType: TimelineType.default,
})
).toEqual('Pin event');
});

test('it indicates the alert is NOT pinned when `isPinned` is `false` and the alert does NOT have notes', () => {
expect(
getPinTooltip({
isAlert: true,
isPinned: false,
eventHasNotes: false,
timelineType: TimelineType.default,
})
).toEqual('Pin alert');
});

test('it indicates the event is disabled if timelineType is template', () => {
expect(
getPinTooltip({
isAlert: false,
isPinned: false,
eventHasNotes: false,
timelineType: TimelineType.template,
})
).toEqual('This event may not be pinned while editing a template timeline');
});

test('it indicates the alert is disabled if timelineType is template', () => {
expect(
getPinTooltip({
isAlert: true,
isPinned: false,
eventHasNotes: false,
timelineType: TimelineType.template,
})
).toEqual('This alert may not be pinned while editing a template timeline');
});
});

describe('eventIsPinned', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,27 @@ export const stringifyEvent = (ecs: Ecs): string => JSON.stringify(ecs, omitType
export const eventHasNotes = (noteIds: string[]): boolean => !isEmpty(noteIds);

export const getPinTooltip = ({
isAlert,
isPinned,
// eslint-disable-next-line @typescript-eslint/no-shadow
eventHasNotes,
timelineType,
}: {
isAlert: boolean;
isPinned: boolean;
eventHasNotes: boolean;
timelineType: TimelineTypeLiteral;
}) =>
timelineType === TimelineType.template
? i18n.DISABLE_PIN
: isPinned && eventHasNotes
? i18n.PINNED_WITH_NOTES
: isPinned
? i18n.PINNED
: i18n.UNPINNED;
}) => {
if (timelineType === TimelineType.template) {
return i18n.DISABLE_PIN(isAlert);
} else if (isPinned && eventHasNotes) {
return i18n.PINNED_WITH_NOTES(isAlert);
} else if (isPinned) {
return i18n.PINNED(isAlert);
} else {
return i18n.UNPINNED(isAlert);
}
};

export interface IsPinnedParams {
eventId: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
import { i18n } from '@kbn/i18n';

export const NOTES_TOOLTIP = i18n.translate(
'xpack.securitySolution.timeline.body.notes.addOrViewNotesForThisEventTooltip',
'xpack.securitySolution.timeline.body.notes.addNoteTooltip',
{
defaultMessage: 'Add notes for this event',
defaultMessage: 'Add note',
}
);

Expand Down Expand Up @@ -42,23 +42,24 @@ export const INVESTIGATE = i18n.translate(
}
);

export const UNPINNED = i18n.translate(
'xpack.securitySolution.timeline.body.pinning.unpinnedTooltip',
{
defaultMessage: 'Unpinned event',
}
);
export const UNPINNED = (isAlert: boolean) =>
i18n.translate('xpack.securitySolution.timeline.body.pinning.pinTooltip', {
values: { isAlert },
defaultMessage: 'Pin {isAlert, select, true{alert} other{event}}',
});

export const PINNED = i18n.translate('xpack.securitySolution.timeline.body.pinning.pinnedTooltip', {
defaultMessage: 'Pinned event',
});
export const PINNED = (isAlert: boolean) =>
i18n.translate('xpack.securitySolution.timeline.body.pinning.unpinTooltip', {
values: { isAlert },
defaultMessage: 'Unpin {isAlert, select, true{alert} other{event}}',
});

export const PINNED_WITH_NOTES = i18n.translate(
'xpack.securitySolution.timeline.body.pinning.pinnnedWithNotesTooltip',
{
defaultMessage: 'This event cannot be unpinned because it has notes',
}
);
export const PINNED_WITH_NOTES = (isAlert: boolean) =>
i18n.translate('xpack.securitySolution.timeline.body.pinning.pinnnedWithNotesTooltip', {
values: { isAlert },
defaultMessage:
'This {isAlert, select, true{alert} other{event}} cannot be unpinned because it has notes',
});

export const SORTED_ASCENDING = i18n.translate(
'xpack.securitySolution.timeline.body.sort.sortedAscendingTooltip',
Expand All @@ -74,12 +75,12 @@ export const SORTED_DESCENDING = i18n.translate(
}
);

export const DISABLE_PIN = i18n.translate(
'xpack.securitySolution.timeline.body.pinning.disablePinnnedTooltip',
{
defaultMessage: 'This event may not be pinned while editing a template timeline',
}
);
export const DISABLE_PIN = (isAlert: boolean) =>
i18n.translate('xpack.securitySolution.timeline.body.pinning.disablePinnnedTooltip', {
values: { isAlert },
defaultMessage:
'This {isAlert, select, true{alert} other{event}} may not be pinned while editing a template timeline',
});

export const VIEW_DETAILS = i18n.translate(
'xpack.securitySolution.timeline.body.actions.viewDetailsAriaLabel',
Expand Down

0 comments on commit bbf87c5

Please sign in to comment.