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

[Security Solution][Detections] fixes various timeline components issues #135907

Merged
merged 9 commits into from
Jul 13, 2022
Merged

[Security Solution][Detections] fixes various timeline components issues #135907

merged 9 commits into from
Jul 13, 2022

Conversation

vitaliidm
Copy link
Contributor

@vitaliidm vitaliidm commented Jul 7, 2022 •
edited
Loading

Summary

Addresses: #128740

1. Responsiveness

Before

It doesn't look good on a smaller screen:

After

Screenshot 2022-07-08 at 16 57 13

2. Focus

Before

Its focus state seems off and doesn't match the combo box focus state (it's especially noticeable in Dark Mode).

After

Screen.Recording.2022-07-08.at.16.57.48.mov

3. Clicking on "favorite star"

Before

If you click the "favorite star" when selecting the first component it can't be reselected until focus is lost:

After

Screen.Recording.2022-07-08.at.16.57.48.mov

Within Edit Rules (also notice how it re-open after selection....weird):

Haven't found way to mitigate this, but anyway it doesn't impact heavily user experience or causes significant impact. So, I left it as it is.

4.Search bar placeholder text

Before

It's not fully visible, and we should either make it shorter or truncate with ellipsis like we do in other parts of the UI:

After

More details on trimming of placeholder here
Screenshot 2022-07-11 at 18 29 01

Checklist

Delete any items that are not applicable to this PR.

For maintainers

Release note

fixes multiple small issues with timeline template selector used on rule edit page and bulk edit

@vitaliidm vitaliidm self-assigned this Jul 7, 2022
@vitaliidm vitaliidm added bug Fixes for quality problems that affect the customer experience release_note:fix impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:Rule Management Security Solution Detection Rule Management Team:Detection Rule Management Security Detection Rule Management Team v8.4.0 backport:skip This commit does not require backporting ci:deploy-cloud labels Jul 7, 2022
@vitaliidm vitaliidm marked this pull request as ready for review July 12, 2022 12:39
@vitaliidm vitaliidm requested review from a team as code owners July 12, 2022 12:39
@vitaliidm vitaliidm requested a review from xcrzx July 12, 2022 12:39
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@vitaliidm vitaliidm mentioned this pull request Jul 12, 2022
Closed
michaelolo24
michaelolo24 reviewed Jul 12, 2022
View reviewed changes
...plugins/security_solution/public/timelines/components/timeline/search_super_select/index.tsx
@@ -27,11 +27,11 @@ const StyledEuiFieldText = styled(EuiFieldText)`

// To match EuiFieldText focus state
&:focus {
background-color: #fff;
background-color: ${({ theme }) => theme.eui.euiFormBackgroundColor};
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏾

michaelolo24
michaelolo24 approved these changes Jul 12, 2022
View reviewed changes
Copy link
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes!

spong
spong approved these changes Jul 12, 2022
View reviewed changes
Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Checked out, tested fixes locally, and code LGTM! 👍 Thank you so much for all these fixes @vitaliidm -- this component definitely feels quite a bit better now 🙂

In testing I did end up finding one more bug (see comment), but I think I there's a fix for it too if you want to include it here. 🎉

@vitaliidm @spong
Update x-pack/plugins/security_solution/public/timelines/components/t…
30061ca 
…imeline/selectable_timeline/index.tsx

Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
@vitaliidm
Copy link
Contributor Author

Thanks for the suggestion @spong. Will include it here, it feels much nicer as in terms of user experience

@vitaliidm vitaliidm enabled auto-merge (squash) July 13, 2022 07:52
@vitaliidm vitaliidm linked an issue Jul 13, 2022 that may be closed by this pull request
[Security Solution] Timeline template selector rendering issues #128740
Closed
@kibana-ci
Copy link
Collaborator

💚 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
securitySolution 5.2MB 5.2MB +479.0B

History

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

cc @vitaliidm

@vitaliidm vitaliidm merged commit 44d2ecf into elastic:main Jul 13, 2022
@tylersmalley tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022
@vitaliidm vitaliidm deleted the fix/timeline-components branch March 4, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xcrzx xcrzx xcrzx approved these changes

@spong spong spong approved these changes

@michaelolo24 michaelolo24 michaelolo24 approved these changes

Assignees

@vitaliidm vitaliidm

Labels
backport:skip This commit does not require backporting bug Fixes for quality problems that affect the customer experience ci:cloud-deploy Create or update a Cloud deployment Feature:Rule Management Security Solution Detection Rule Management impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. release_note:fix Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.4.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Security Solution] Timeline template selector rendering issues
7 participants
@vitaliidm @elasticmachine @kibana-ci @xcrzx @spong @michaelolo24 @tylersmalley