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

Jump to bottom

[SIEM] fix tooltip of notes #71342

Merged
merged 7 commits into from
Jul 10, 2020
Merged

[SIEM] fix tooltip of notes #71342

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a011519
fix tooltip of notes
angorayc Jul 9, 2020
0fd7c3d
Merge branch 'master' of github.com:elastic/kibana into notes-tooltip
angorayc Jul 9, 2020
65db38e
fix unit test
angorayc Jul 9, 2020
1a6f0f8
Merge branch 'master' into notes-tooltip
elasticmachine Jul 9, 2020
655d981
update notes tooltip
angorayc Jul 10, 2020
e082f5f
Merge branch 'master' into notes-tooltip
elasticmachine Jul 10, 2020
695d9c5
fix unit test
angorayc Jul 10, 2020
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
69 changes: 69 additions & 0 deletions ...lugins/security_solution/public/timelines/components/timeline/body/actions/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import { useSelector } from 'react-redux';

import { TestProviders, mockTimelineModel } from '../../../../../common/mock';
import { DEFAULT_ACTIONS_COLUMN_WIDTH } from '../constants';
import * as i18n from '../translations';

import { Actions } from '.';
import { TimelineType } from '../../../../../../common/types/timeline';

jest.mock('react-redux', () => {
const origin = jest.requireActual('react-redux');
Expand Down Expand Up @@ -202,6 +204,73 @@ describe('Actions', () => {
expect(toggleShowNotes).toBeCalled();
});

test('it renders correct tooltip for NotesButton - timeline', () => {
const toggleShowNotes = jest.fn();

const wrapper = mount(
<TestProviders>
<Actions
actionsColumnWidth={DEFAULT_ACTIONS_COLUMN_WIDTH}
associateNote={jest.fn()}
checked={false}
expanded={false}
eventId="abc"
eventIsPinned={false}
getNotesByIds={jest.fn()}
loading={false}
loadingEventIds={[]}
noteIds={[]}
onEventToggled={jest.fn()}
onPinClicked={jest.fn()}
onRowSelected={jest.fn()}
showCheckboxes={false}
showNotes={false}
toggleShowNotes={toggleShowNotes}
updateNote={jest.fn()}
/>
</TestProviders>
);

expect(wrapper.find('[data-test-subj="add-note"]').prop('toolTip')).toEqual(i18n.NOTES_TOOLTIP);
});

test('it renders correct tooltip for NotesButton - timeline template', () => {
(useSelector as jest.Mock).mockReturnValue({
...mockTimelineModel,
timelineType: TimelineType.template,
});
const toggleShowNotes = jest.fn();

const wrapper = mount(
<TestProviders>
<Actions
actionsColumnWidth={DEFAULT_ACTIONS_COLUMN_WIDTH}
associateNote={jest.fn()}
checked={false}
expanded={false}
eventId="abc"
eventIsPinned={false}
getNotesByIds={jest.fn()}
loading={false}
loadingEventIds={[]}
noteIds={[]}
onEventToggled={jest.fn()}
onPinClicked={jest.fn()}
onRowSelected={jest.fn()}
showCheckboxes={false}
showNotes={false}
toggleShowNotes={toggleShowNotes}
updateNote={jest.fn()}
/>
</TestProviders>
);

expect(wrapper.find('[data-test-subj="add-note"]').prop('toolTip')).toEqual(
i18n.NOTES_DISABLE_TOOLTIP
);
(useSelector as jest.Mock).mockReturnValue(mockTimelineModel);
});

test('it does NOT render a pin button when isEventViewer is true', () => {
const onPinClicked = jest.fn();

Expand Down
7 changes: 6 additions & 1 deletion x-pack/plugins/security_solution/public/timelines/components/timeline/body/actions/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { EuiButtonIcon, EuiCheckbox, EuiLoadingSpinner, EuiToolTip } from '@elas

import { Note } from '../../../../../common/lib/note';
import { StoreState } from '../../../../../common/store/types';
import { TimelineType } from '../../../../../../common/types/timeline';

import { TimelineModel } from '../../../../store/timeline/model';

Expand Down Expand Up @@ -170,7 +171,11 @@ export const Actions = React.memo<Props>(
status={timeline.status}
timelineType={timeline.timelineType}
toggleShowNotes={toggleShowNotes}
toolTip={timeline.timelineType ? i18n.NOTES_DISABLE_TOOLTIP : i18n.NOTES_TOOLTIP}
toolTip={
timeline.timelineType === TimelineType.template
? i18n.NOTES_DISABLE_TOOLTIP
: i18n.NOTES_TOOLTIP
}
updateNote={updateNote}
/>
</EventsTdContent>
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/security_solution/public/timelines/components/timeline/body/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import { i18n } from '@kbn/i18n';
export const NOTES_TOOLTIP = i18n.translate(
'xpack.securitySolution.timeline.body.notes.addOrViewNotesForThisEventTooltip',
{
defaultMessage: 'Add or view notes for this event',
defaultMessage: 'Add notes for this event',
}
);

export const NOTES_DISABLE_TOOLTIP = i18n.translate(
'xpack.securitySolution.timeline.body.notes.disableEventTooltip',
{
defaultMessage: 'Add notes for event filtered by a timeline template is not allowed',
defaultMessage: 'Notes may not be added here while editing a template timeline',
Copy link
Contributor

Choose a reason for hiding this comment

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

While testing locally in the regular (non-template) mode, I noticed, per the screenshot below, that the tooltip currently reads Add or view notes for this event:

add-or-view-notes

I don't think the or view part of the tooltip is valid, because clicking the button doesn't view notes. Notes are always displayed inline, per the screenshot below:

inline-note

Would you be willing to change NOTES_TOOLTIP in translations.ts from Add or view notes for this event to Add notes for this event?

}
);

Expand Down Expand Up @@ -48,7 +48,7 @@ export const PINNED_WITH_NOTES = i18n.translate(
export const DISABLE_PIN = i18n.translate(
'xpack.securitySolution.timeline.body.pinning.disablePinnnedTooltip',
{
defaultMessage: 'This event cannot be pinned because it is filtered by a timeline template',
defaultMessage: 'This event may not be pinned while editing a template timeline',
}
);

Expand Down