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]Cell Action modal colliding with timeline viewing #118844

Closed
ghost opened this issue Nov 17, 2021 · 10 comments
Closed

[Security Solution]Cell Action modal colliding with timeline viewing #118844

ghost opened this issue Nov 17, 2021 · 10 comments
Assignees
@michaelolo24
Labels
bug Fixes for quality problems that affect the customer experience fixed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. QA:Validated Issue has been validated by QA Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.6.0

Comments

@ghost
Copy link

ghost commented Nov 17, 2021 •
edited by MadameSheema
Loading

Describe the bug
Cell Action modal colliding with timeline viewing

Build Details
Version:7.16.0 BC4
Commit:e50bc2eded568ff3ceaebdbe616f84b3987be975
Build:45952

Steps

  • Create 7.16.0 BC4
  • Generate some alerts on it
  • go to Alert list
  • click on hit enter and modal will open up
  • now click on investigate in timeline from cell hover action not from open hit enter modal
  • observed that hit enter modal stay show up in timeline view

Screen-Cast

Issue.mp4

logs

  • N/A
@ghost ghost added bug Fixes for quality problems that affect the customer experience triage_needed Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Nov 17, 2021
@elasticmachine
Copy link
Contributor

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

@ghost ghost added Team:Threat Hunting:Explore impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Nov 17, 2021
@manishgupta-qasource
Copy link

Reviewed & assigned to @MadameSheema

@MadameSheema MadameSheema removed their assignment Nov 17, 2021
@michaelolo24
Copy link
Contributor

@cchaos or @chandlerprall - I don't think there's a programmatic way for us to click a hover action and trigger the close of the popover. Correct me if I'm wrong, but it looks like isExpanded just defines whether that action or field is shown in the details popover? Would it make sense to, by default, close the details popover whenever another cell action is clicked?

@michaelolo24 michaelolo24 self-assigned this Nov 17, 2021
@cchaos
Copy link
Contributor

cchaos commented Nov 17, 2021

Yep, that should get fixed when we address elastic/eui#5310

@michaelolo24
Copy link
Contributor

Awesome, thank you!

@YulNaumenko
Copy link
Contributor

YulNaumenko commented Jan 24, 2022 •
edited
Loading

Seems to be a similar (or duplication) of #115339

@michaelolo24
Copy link
Contributor

@YulNaumenko , they are slightly different as this one deals with the EuiDataGrid popover while the other issue has to deal with the topN modal. They are all tied to z-index management in general, but unrelated components.

@michaelolo24
Copy link
Contributor

Changes made for this fix may also help with this as well: #115341

@MindyRS MindyRS added the Team:Threat Hunting Security Solution Threat Hunting Team label Feb 23, 2022
@elasticmachine
Copy link
Contributor

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

@christineweng christineweng mentioned this issue Nov 11, 2022
Merged
christineweng added a commit that referenced this issue Nov 16, 2022
@christineweng
[Security Solution][Bug] fix popover in t-grid (#145025)
05d1ff8 
This PR contains fixes for the following issues:

#### # 1 Popover overlaps flyout
- #139280
- #128235
#### # 2 Popover persists after clicking filter out
- #115341
#### # 3 Popover persists after clicking a button outside of popover
- #118844

## Background
Previously, a cell's popover remains open after clicking an action. In
many cases we want the popover to close upon clicking on a cell action.
EUI team addressed this by adding a `closeCellPopover` to a `ref` API.
- elastic/eui#5590

In T-grid, there are 2 types of cell actions: 
- Default cell actions such as filter in, filter out, add to timeline
and copy. `closeCellPopover` is not used.
- Formatted fields that have more information in the form of flyouts
(host name, user name, ip, etc.)
`closeCellPopover` prop is passed but currently not working as expected.

This PR contains fixes for: 
- Fixing `closeCellPopover` in T-grid body for formatted fields - fixes
# 1
- Adding `closeCellPopover` props in default cell actions - fixes # 2
and # 3

## # 1 - `closeCellPopover` in T-grid 

`dataGridRef.current?.closeCellPopover` was added and intended to close
any open popovers when a cell action is clicked. However, because it is
a mutable object, it is not being monitored in `columnsWithCellActions`.
When the page is initially loaded, `dataGridRef.current` remain as null
and it does not update until the page re-renders and `dataGridRef`
becomes non-null.
- After: popover closes properly


https://user-images.githubusercontent.com/18648970/201202326-ec657f78-c425-46a6-9356-f6e9ef1ab798.mov


## # 2 & # 3 Add `closeCellPopover` to default cell actions

- After: upon opening the expansion popover, clicking any options and
the popover will disappear


https://user-images.githubusercontent.com/18648970/201417542-063c514b-5474-4676-a747-a9401627c5e8.mov

- After: upon opening the expansion popover, clicking any options
outside and the popover will disappear


https://user-images.githubusercontent.com/18648970/201417678-7cf0fefa-f4a7-4a70-9a10-76b248323639.mov

Note for UX: although QA only flagged `filter out` and `add to
timeline`, for consistency's sake, the expansion popover will disappear
after clicking any of the cell actions, which includes `filter in` and
`copy`.
@ghost
Copy link
Author

ghost commented Nov 17, 2022

Hi @michaelolo24

we have validated this issue on 8.6.0 BC1 and found the issue to be fixed ✔️ .

Build Details:

Version: 8.6.0 BC1
Commit:50a7feb0a5eb068d3acccc49c83b9ccb6db6734f
Build:58392

Screen-Cast:

karanbir.-.ec2-3-88-91-221.compute-1.amazonaws.com.-.Remote.Desktop.Connection.2022-11-17.13-50-06.mp4

Hence we are closing this issue and adding QA:Validated tag to it.

thanks !!

@ghost ghost closed this as completed Nov 17, 2022
@ghost ghost added the QA:Validated Issue has been validated by QA label Nov 17, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michaelolo24 michaelolo24

Labels
bug Fixes for quality problems that affect the customer experience fixed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. QA:Validated Issue has been validated by QA Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.6.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@cchaos @MindyRS @elasticmachine @michaelolo24 @MadameSheema @christineweng @YulNaumenko @manishgupta-qasource