-
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
[Logs UI] Limit the height of the "view in context" container #83178
[Logs UI] Limit the height of the "view in context" container #83178
Conversation
The `EuiModal` has a max-height setting. In chrome, content with a bigger height got clipped instead of forced to this max-height. To ensure this never happens, we also limit the height of the content to the same `max-height` of the modal.
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
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.
That seems to solve it, but I left a question about the implementation below.
@@ -84,6 +84,7 @@ const LogInContextWrapper = euiStyled.div<{ width: number | string; height: numb | |||
padding: 16px; | |||
width: ${(props) => (typeof props.width === 'number' ? `${props.width}px` : props.width)}; | |||
height: ${(props) => (typeof props.height === 'number' ? `${props.height}px` : props.height)}; | |||
max-height: 75vh; // Same as EuiModal |
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.
Do we want to hard-code it like this even though there's a parameterized height
prop? According to the CSS spec "max-height
overrides height
", so I wonder if we could somehow unify the treatment of both.
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.
According to the CSS spec "
max-height
overridesheight
"
I'm relying on this :) I want the content of the modal to "take as much height available (the height
) without making it bigger than its container (the max-height
, which replicates the value in the EuiModal
).
The problem is that we have no control on the height of the modal itself. The content can be as tall as we want, but the modal will clip it no matter what *.
*: in Chrome at least. In Firefox it worked correctly, but I'm not sure what browser is breaking the standards here.
@elasticmachine merge upstream |
@elasticmachine merge upstream |
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.
I don't immediately see a better way either since the modal height is not exposed as a variable by EUI, so LGTM if CI agrees 👍
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
* master: [Security Solution] Exceptions Cypress tests (elastic#81759) [ML] Fix spaces job ID check (elastic#84404) [Security Solution][Detections] Handle dupes when processing threshold rules (elastic#83062) skip flaky suite (elastic#84440) skip flaky suite (elastic#84445) [APM] Fix missing `service.node.name` (elastic#84269) Upgrade fp-ts to 2.8.6 (elastic#83866) Added data streams privileges to better control delete actions in UI (elastic#83573) Improve short-url redirect validation (elastic#84366) TSVB offsets (elastic#83051) [Discover] Fix navigating back when changing index pattern (elastic#84061) [Logs UI] Polish the UI for the log entry examples in the anomaly table (elastic#82139) [Logs UI] Limit the height of the "view in context" container (elastic#83178) [Application Usage] Update `schema` with new `fleet` rename (elastic#84327) fix identation in list (elastic#84301)
Closes #83063
The
EuiModal
has a max-height setting. In chrome, content with a bigger height got clipped instead of forced to this max-height.To ensure this never happens, we also limit the height of the content to the same
max-height
of the modal.