-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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][RAC] - Hide hover actions overflow #109693
[Security Solution][RAC] - Hide hover actions overflow #109693
Conversation
Pinging @elastic/security-solution (Team: SecuritySolution) |
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
@@ -595,10 +595,17 @@ export const BodyComponent = React.memo<StatefulBodyProps>( | |||
const rowData = rowIndex < data.length ? data[rowIndex].data : null; | |||
const header = columnHeaders.find((h) => h.id === columnId); | |||
const eventId = rowIndex < data.length ? data[rowIndex]._id : null; | |||
const defaultStyles = useMemo( | |||
() => ({ | |||
overflow: 'hidden', |
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.
😎
@stephmilovic I'm trying to reproduce what you saw, but it's working on my end: EDIT: I was able to reproduce it when I switched branches without rebuilding. |
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.
LGTM!!
* add overflow * fix types error
* add overflow * fix types error
…9819) * add overflow * fix types error Co-authored-by: Michael Olorunnisola <[email protected]>
…9818) * add overflow * fix types error Co-authored-by: Michael Olorunnisola <[email protected]>
I'm reopening the issue this PR was against. This change doesn't fix the core issue, which is that you have too many actions for small columns. Before the buttons were unclickable, now they can no longer be seen (but are still no longer clickable). If we want to have 5 actions per cell, we need to set a minwidth to the column to account for the width of the actions OR we need to remove the amount of actions we allow and surface them into the popover (I'd prefer the later) since 5 actions per cell seems like way too much going on in such a small space. |
@snide We spoke about this in our meeting and @andrew-goldstein is going to create an EUI issue specifically for focusing on the number of fields to show |
There's a full write up here: https://github.com/elastic/stack-design-team/issues/85 for what the end goal for this should be. I'm happy to discuss this further and figure out how to iterate to this. I think whether the popover layout is correct or not is something we can continue working on, but to Dave's point, the primary goal here is to remove the 5 icons from the cell hover. |
Summary
Fixes #108891
Shown with the building block styling below to show that the styles aren't overriden:
overflow.mp4