-
Notifications
You must be signed in to change notification settings - Fork 8.4k
[Security Solution] bump Timeline, document and timeline flyout components' z-index by 1 to be on top of side nav #236655
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] bump Timeline, document and timeline flyout components' z-index by 1 to be on top of side nav #236655
Conversation
786c9d1
to
cf33fdd
Compare
…nents' z-index by 1 to be on top of side nav
cf33fdd
to
2f0bed4
Compare
Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations) |
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.
Code-only review, Data Discovery changes LGTM
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.
Detection Engine changes LGTM. I had one question about an added prop that wasn't there before, and one suggestion for future improvements here 👍
x-pack/solutions/security/plugins/security_solution/public/flyout/index.tsx
Show resolved
Hide resolved
...y_solution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.tsx
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.
cases here, looking good. desk tested, new case flyout rendered correctly.
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.
Looks great! Thank you for this!
💚 Build Succeeded
Metrics [docs]Async chunks
History
|
Summary
This PR an issue that has been present for a few months: the side nav in Security Solution is rendered on top of the expandable flyout.
This issue comes from this change introduced by this PR a few months ago in EUI.
The changes suggested here are:
z-index
from1000
to1001
z-index
from1001
to1002
z-index
from1002
to1003
z-index
from1003
to1004
Thankfully when we had made a similar change over a year ago we had marked all the related flyouts with the following comment
// EUI TODO: This z-index override of EuiOverlayMask is a workaround, and ideally should be resolved with a cleaner UI/UX flow long-term
So it was easy to find them.
This is not an ideal fix (obviously it was already not an ideal fix over a year ago) but this will quickly get us back to a better UX, as currently if the flyout opens behind the side nav, users can't resize them at all because the handle to resize is hidden...
What to test
make sure that the security solution flyout is always on top of the side nav
make sure that Timeline is always on top of the security solution flyout
make sure that the timeline flyout is always on top of Timeline
make sure that the flyout on the ESQL tab in Timeline behaves correctly
make sure these work after a page refresh
make sure that things work both with overlay and push modes
make sure that all secondary flyouts - accessible via the Take action button in the flyout's footer - are displayed on top.
The PR description includes the appropriate Release Notes section, and the correct
release_note:*
label is applied per the guidelinesReview the backport guidelines and apply applicable
backport:*
labels.