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.

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

[Security Solution] [Timeline] Remove code added to workaround an eui issue, causing focus issues #158392

Conversation

kqualters-elastic
Copy link
Contributor

Summary

This pr removes some code added 2 years ago to work around an eui issue with the global search bar and timeline focus, was causing issues in some cases, like in #152344 none of this code is needed, as the original bug no longer exists.

Checklist

@kqualters-elastic kqualters-elastic added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.9.0 labels May 24, 2023
@kqualters-elastic kqualters-elastic requested a review from maximpn May 24, 2023 15:35
@kqualters-elastic kqualters-elastic requested a review from a team as a code owner May 24, 2023 15:35
Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

@kqualters-elastic thank you for addressing #152344 and removing unnecessary anymore code 🎉

I've tested it locally and tried to run Cypress tests without force: true option in type() command and it works as expected.

@michaelolo24
Copy link
Contributor

@elasticmachine merge upstream

@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 9.2MB 9.2MB -669.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 398 402 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 478 482 +4
total +6

History

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

@kqualters-elastic kqualters-elastic merged commit 1c2bc02 into elastic:main May 25, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 25, 2023
@kqualters-elastic kqualters-elastic deleted the remove-timeline-focus-eui-workaround branch May 25, 2023 03:26
maximpn added a commit that referenced this pull request May 30, 2023
…s tests (#158560)

**Relates to:** #152344, #152470
**Depends on:** #158392

## Summary

Removes `force` option from Cypress actions like `type()`, `click()` and etc. It helps to catch UI quirks instead of silently ignoring them.

## Details

After merging back of #158392 it's possible to remove `force: true` flag from the Cypress actions used in Detection rules tests. Based on the discussion in #152470 it's bette to avoid forced actions and leave a comment if there is no other way than forcing an action.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants