[Security Solution] Updates Timeline tooltips to use action words / fewer words #132756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrew-goldstein
added
bug
janmonschke
reviewed
May 24, 2022
Comment on lines
+42
to
+50
| 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); | ||
| } |
There was a problem hiding this comment.
Thanks for cleaning this up, it's much more readable now 👍
janmonschke
reviewed
May 24, 2022
| @@ -221,6 +224,7 @@ const ActionsComponent: React.FC<ActionProps> = ({ | |||
| /> | |||
| <PinEventAction | |||
| ariaLabel={i18n.PIN_EVENT_FOR_ROW({ ariaRowindex, columnValues, isEventPinned })} | |||
| isAlert={isAlert(ecsData)} | |||
There was a problem hiding this comment.
Since eventType is already extracted above (line 108), I wonder if it might be better to change isAlert(ecs: Ecs) to isAlert(eventType: string). That way you'd save one getEventType call per render.
…/ fewer words - This PR updates the row-level action button tooltips in Timeline, per issue <elastic#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  #### After: Unpinned alert  #### Before: Pinned alert  #### After: Pinned alert  #### Before: Add a note to an alert  #### After: Add a note to an alert  #### Before: A pinned alert with notes  #### After: A pinned alert with notes  #### Before: A timeline template with alerts  #### After: A timeline template with alerts  ### Event tooltips This section contains before / after screenshots for events. #### Before: Unpinned event  #### After: Unpinned event  #### Before: Pinned event  #### After: Pinned event  #### Before: Add a note to event  #### After: Add a note to an event  #### Before: A pinned event with notes  #### After: A pinned event with notes  #### Before: A timeline template with events  #### After: A timeline template with events 
andrew-goldstein
force-pushed
the
update-timeline-tooltips
branch
from
bbf87c5
to
ce0c17d
Compare
💛 Build succeeded, but was flakyFailed CI StepsTest Failures
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
⚪ Backport skippedThe pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually. Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[Security Solution] Updates Timeline tooltips to use action words / fewer words
Alert tooltips
This section contains before / after screenshots for alerts.
Before: Unpinned alert
After: Unpinned alert
Before: Pinned alert
After: Pinned alert
Before: Add a note to an alert
After: Add a note to an alert
Before: A pinned alert with notes
After: A pinned alert with notes
Before: A timeline template with alerts
After: A timeline template with alerts
Event tooltips
This section contains before / after screenshots for events.
Before: Unpinned event
After: Unpinned event
Before: Pinned event
After: Pinned event
Before: Add a note to event
After: Add a note to an event
Before: A pinned event with notes
After: A pinned event with notes
Before: A timeline template with events
After: A timeline template with events
CC: @dimadavid