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] Fixes the position of popover when presenting dynamic content #139079

Merged
merged 3 commits into from
Aug 25, 2022

Conversation

logeekal
Copy link
Contributor

@logeekal logeekal commented Aug 18, 2022

Summary

Issue #133186 ( as shown in below video) demonstrates couple of issues and this PR tries to fix those.

show.top.mp4

ISSUE - 1

When in host details flyout, clicking on show Top N, the datagrid will flicker scrolling from right to left and then back to its original position.

Solution : This issue was not reproducible so I am assuming this has been already resolved. ( See below video for more details)

ISSUE - 2

When in host details flyout, clicking on show Top N, the popup hides behind the bottom screen edge. This is because initially the popup was small but when Show TopN is clicked, the content of Popup is changed and popup becomes bigger in size forcing it to exceed out of the screen's real estate.

Solution : This PR makes sure that, if hover actions Popup's content changes, Popup is re-positioned according to its new dimensions. Checkout below demonstration.

Screen.Recording.2022-08-18.at.13.02.40.mov

For maintainers

@logeekal logeekal changed the title Fix/popover position dynamic content [Security Solution] Fixes the position of popover when presenting dynamic content Aug 18, 2022
@logeekal logeekal requested a review from a team August 18, 2022 11:09
@logeekal logeekal added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Threat Hunting:Investigations Security Solution Investigations Team v8.5.0 labels Aug 18, 2022
@logeekal logeekal marked this pull request as ready for review August 18, 2022 11:09
@logeekal logeekal requested a review from a team as a code owner August 18, 2022 11:09
@logeekal logeekal added the release_note:skip Skip the PR/issue when compiling release notes label Aug 18, 2022
Copy link
Contributor

@janmonschke janmonschke left a comment

Choose a reason for hiding this comment

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

👍

Regarding the first issue: Could it be that the issue only occurs on certain layout breakpoints? Might make sense to ask the QA which browser size they tested this with.

@logeekal
Copy link
Contributor Author

Regarding the first issue: Could it be that the issue only occurs on certain layout breakpoints? Might make sense to ask the QA which browser size they tested this with.

That is a good point @janmonschke. I will followup with QA team on the same.

@stephmilovic
Copy link
Contributor

stephmilovic commented Aug 18, 2022

Screen Shot 2022-08-18 at 7 40 39 AM

why does the initial popover stay open? i feel like when you click View Host Summary, the popover should close

@logeekal
Copy link
Contributor Author

Screen Shot 2022-08-18 at 7 40 39 AM why does the initial popover stay open? i feel like when you click View Host Summary, the popover should close

@stephmilovic I 100% agree. But I noticed this on https://kibana.siem.estc.dev/ as well and this seems to be a consistent behaviour so I assumed this is not broken and did not touch it.

But if we agree that expected behaviour is that it should close ( which makes sense to me), I can fix that.

@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.4MB 5.4MB +145.0B

History

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

Copy link
Contributor

@stephmilovic stephmilovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for opening that new issue

@logeekal logeekal merged commit acf675c into elastic:main Aug 25, 2022
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 25, 2022
…amic content (elastic#139079)

## Summary

Issue elastic#133186 ( as shown in below video) demonstrates couple of issues and this PR tries to fix those.

https://user-images.githubusercontent.com/61860752/171141631-f633a223-af76-457d-bed2-6bd8ea418a0e.mp4

**ISSUE - 1**

When in host details flyout, clicking on `show Top N`, the datagrid will flicker scrolling from right to left and then back to its original position.

**Solution** : This issue was not reproducible so I am assuming this has been already resolved. ( See below video for more details)

**ISSUE - 2**

When in host details flyout, clicking on `show Top N`, the popup hides behind the bottom screen edge. This is because initially the popup was small but when `Show TopN` is clicked, the content of Popup is changed and popup becomes bigger in size forcing it to exceed out of the screen's real estate.

**Solution** : This PR makes sure that, if hover actions Popup's content changes, Popup is re-positioned according to its new dimensions. Checkout below demonstration.

https://user-images.githubusercontent.com/7485038/185380538-d8fb159a-d666-4be2-b3b0-161a21d1b9d6.mov
(cherry picked from commit acf675c)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.4

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 Aug 25, 2022
…amic content (#139079) (#139454)

## Summary

Issue #133186 ( as shown in below video) demonstrates couple of issues and this PR tries to fix those.

https://user-images.githubusercontent.com/61860752/171141631-f633a223-af76-457d-bed2-6bd8ea418a0e.mp4

**ISSUE - 1**

When in host details flyout, clicking on `show Top N`, the datagrid will flicker scrolling from right to left and then back to its original position.

**Solution** : This issue was not reproducible so I am assuming this has been already resolved. ( See below video for more details)

**ISSUE - 2**

When in host details flyout, clicking on `show Top N`, the popup hides behind the bottom screen edge. This is because initially the popup was small but when `Show TopN` is clicked, the content of Popup is changed and popup becomes bigger in size forcing it to exceed out of the screen's real estate.

**Solution** : This PR makes sure that, if hover actions Popup's content changes, Popup is re-positioned according to its new dimensions. Checkout below demonstration.

https://user-images.githubusercontent.com/7485038/185380538-d8fb159a-d666-4be2-b3b0-161a21d1b9d6.mov
(cherry picked from commit acf675c)

Co-authored-by: Jatin Kathuria <jatin.kathuria@elastic.co>
Mpdreamz pushed a commit to Mpdreamz/kibana that referenced this pull request Sep 6, 2022
…amic content (elastic#139079)

## Summary

Issue elastic#133186 ( as shown in below video) demonstrates couple of issues and this PR tries to fix those.

https://user-images.githubusercontent.com/61860752/171141631-f633a223-af76-457d-bed2-6bd8ea418a0e.mp4

**ISSUE - 1**

When in host details flyout, clicking on `show Top N`, the datagrid will flicker scrolling from right to left and then back to its original position.

**Solution** : This issue was not reproducible so I am assuming this has been already resolved. ( See below video for more details)

 
**ISSUE - 2**

When in host details flyout, clicking on `show Top N`, the popup hides behind the bottom screen edge. This is because initially the popup was small but when `Show TopN` is clicked, the content of Popup is changed and popup becomes bigger in size forcing it to exceed out of the screen's real estate.

**Solution** : This PR makes sure that, if hover actions Popup's content changes, Popup is re-positioned according to its new dimensions. Checkout below demonstration.


https://user-images.githubusercontent.com/7485038/185380538-d8fb159a-d666-4be2-b3b0-161a21d1b9d6.mov
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.4.1 v8.5.0
Projects
None yet
5 participants