Skip to content

[Security Solution][Timeline] Toast wrap word when new filter is added to timeline#142803

Merged
christineweng merged 3 commits intoelastic:mainfrom
christineweng:BUG-add-to-timeline-toast-wrapping
Oct 11, 2022
Merged

[Security Solution][Timeline] Toast wrap word when new filter is added to timeline#142803
christineweng merged 3 commits intoelastic:mainfrom
christineweng:BUG-add-to-timeline-toast-wrapping

Conversation

@christineweng
Copy link
Copy Markdown
Contributor

@christineweng christineweng commented Oct 5, 2022

Ref: #142804

This PR updates the message in toast when a new filter is added to timeline. Previously if the text is too long and has no break word characters the text would overflow the toast box. A break-word wrapper is added to properly display the message.

image

@christineweng christineweng added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Feature:Timeline Security Solution Timeline feature Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team v8.5.0 v8.6.0 labels Oct 5, 2022
@christineweng christineweng requested a review from a team as a code owner October 5, 2022 21:29
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@christineweng christineweng changed the title [Security Solution][Timeline][Fix] Toast wrap word when new filter is added to timeline [Security Solution][Timeline] Toast wrap word when new filter is added to timeline Oct 5, 2022
return handleStartDragToTimeline;
};

const ToastAddSuccess: React.FC<{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider renaming this to something like AddSuccessMessage as this component is not handling anything related to a toast.

You can also consider passing children instead of props so that any message can be passed as below

<ToastAddSuccess>
{i18n.ADDED_TO_TIMELINE_OR_TEMPLATE_MESSAGE(
    addSuccess,
    timelineType === 'default'
  )}
</ToastAddSuccess>

nitpick: Another suggestion is to create a generic Component BreakWordText which can receive message as children and can be used anywhere where we need to handle long text.

addSuccess,
timelineType === 'default'
);
return <span className="eui-textBreakWord">{message}</span>;
Copy link
Copy Markdown
Contributor

@logeekal logeekal Oct 6, 2022

Choose a reason for hiding this comment

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

TIL about class eui-textBreakWord. Thanks.

Copy link
Copy Markdown
Contributor

@logeekal logeekal left a comment

Choose a reason for hiding this comment

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

Overall logically, changes look good 👌🏽. But I left some feedback about naming. You can check those and see if they make sense.

@christineweng
Copy link
Copy Markdown
Contributor Author

christineweng commented Oct 6, 2022

Updated naming convention to better describe purpose and test cases that look for component with the test string.

Kept AddSuccessMessage in add_to_timeline as the bug only appears when a user add a long field without break words (e.g. dll.hash.sha256) to timeline.

@christineweng christineweng enabled auto-merge (squash) October 6, 2022 21:16
Copy link
Copy Markdown
Contributor

@logeekal logeekal left a comment

Choose a reason for hiding this comment

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

This is looking great. Thanks for reviewing the feedback and making changes.

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
timelines 26.2KB 26.4KB +239.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@christineweng christineweng merged commit 4ec846c into elastic:main Oct 11, 2022
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 11, 2022
…d to timeline (elastic#142803)

* added break-word wrapper around toast that fires off when a filter is added to timeline

* renamed ToastAddSuccess to AddSuccessMessage and update it to accept children

(cherry picked from commit 4ec846c)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 11, 2022
…d to timeline (#142803) (#143142)

* added break-word wrapper around toast that fires off when a filter is added to timeline

* renamed ToastAddSuccess to AddSuccessMessage and update it to accept children

(cherry picked from commit 4ec846c)

Co-authored-by: christineweng <18648970+christineweng@users.noreply.github.com>
@christineweng christineweng self-assigned this Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Fixes for quality problems that affect the customer experience Feature:Timeline Security Solution Timeline feature release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.5.0 v8.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants