-
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] Fix index checkbox overlap of the analysis setup dat… #49489
[Logs UI] Fix index checkbox overlap of the analysis setup dat… #49489
Conversation
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
</EuiFormControlLayout> | ||
</EuiFlexGroup> | ||
</EuiFormRow> | ||
</EuiDescribedFormGroup> | ||
); | ||
}; | ||
|
||
const WithFixedDatepickerZIndex = euiStyled( |
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 created this function-as-a-child component to work around the issue that the popover class must be specified as popperClassName
instead of className
.
💚 Build Succeeded |
</EuiFormControlLayout> | ||
</EuiFlexGroup> | ||
</EuiFormRow> | ||
</EuiDescribedFormGroup> | ||
); | ||
}; | ||
|
||
const WithFixedDatepickerZIndex = euiStyled( |
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.
Maybe save some complexity by doing:
const WithFixedDatepickerZIndex = euiStyled( | |
const StyledDatePicker = euiStyled( | |
({ | |
className, | |
...rest, | |
}: { | |
className?: string; | |
rest: Object; // Not sure off the top of my head how to refer to EuiDatePicker's propTypes | |
}) => <EuiDatePicker {...rest} popperClassName={className}/> | |
)` | |
z-index: 3 !important; | |
`; |
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.
Ok, I can do something to that effect. The reason why I implemented it as a FAAC component was to avoid that coupling to the datepicker props and retain the ability to supply a className
to the datepicker.
b450005
to
34cb645
Compare
💚 Build Succeeded |
@Zacqary I followed your suggestion - this is ready for another review pass. |
…tic#49489) This slightly increases the datepicker popover `z-index` to avoid the checkbox labels (which for some reason have an increased `z-index`) from underneath showing through. fixes elastic#49245
…tic#49489) This slightly increases the datepicker popover `z-index` to avoid the checkbox labels (which for some reason have an increased `z-index`) from underneath showing through. fixes elastic#49245
Backports the following commits to 7.x: - [Logs UI] Fix index checkbox overlap of the analysis setup dat… (#49489)
Backports the following commits to 7.5: - [Logs UI] Fix index checkbox overlap of the analysis setup dat… (#49489)
Summary
This slightly increases the datepicker popover
z-index
to avoid the checkbox labels (which for some reason have an increasedz-index
) from underneath showing through.fixes #49245
Preview
Checklist
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsUnit or functional tests were updated or added to match the most common scenariosThis was checked for keyboard-only and screenreader accessibilityFor maintainers
This was checked for breaking API changes and was labeled appropriatelyThis includes a feature addition or change that requires a release note and was labeled appropriately