-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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][Bug] fix popover in t-grid #145025
Conversation
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
x-pack/plugins/security_solution/public/common/lib/cell_actions/add_to_timeline.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review for Explore team. LGTM 🔥
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these fixes @christineweng! 🙏
✅ desk tested locally
LGTM 🚀
This PR contains fixes for the following issues:
# 1 Popover overlaps flyout
# 2 Popover persists after clicking filter out
# 3 Popover persists after clicking a button outside of popover
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 aref
API.ref
eui#5590In T-grid, there are 2 types of cell actions:
closeCellPopover
is not used.closeCellPopover
prop is passed but currently not working as expected.This PR contains fixes for:
closeCellPopover
in T-grid body for formatted fields - fixes # 1closeCellPopover
props in default cell actions - fixes # 2 and # 3# 1 -
closeCellPopover
in T-griddataGridRef.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 incolumnsWithCellActions
. When the page is initially loaded,dataGridRef.current
remain as null and it does not update until the page re-renders anddataGridRef
becomes non-null.Screen.Recording.2022-11-10.at.2.44.53.PM.mov
# 2 & # 3 Add
closeCellPopover
to default cell actionsScreen.Recording.2022-11-11.at.1.31.01.PM.mov
Screen.Recording.2022-11-11.at.1.41.25.PM.mov
Note for UX: although QA only flagged
filter out
andadd to timeline
, for consistency's sake, the expansion popover will disappear after clicking any of the cell actions, which includesfilter in
andcopy
.