-
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
Improves dark theme support in the filter editor #13522
Conversation
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!
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.
In ui_framework/doc_site/src/views/modal/modal_example.js
could you add this to line 36:
<GuideDemo isDarkTheme>
<StaticConfirmModal />
</GuideDemo>
Then if you look at the example in the UI Framework, you can see the color of the text is off:
We can fix this by making the following changes to modal/_modal.scss
:
.kuiModalHeader__title {
font-size: $globalTitleFontSize;
@include darkTheme {
color: $globalTextColor--darkTheme;
}
}
.kuiModalBodyText {
font-size: 14px;
@include darkTheme {
color: $globalTextColor--darkTheme;
}
}
@localNavDepth: 3; /* 1 */ | ||
@dashboardGridDepth: 1; /* 2 */ |
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 ❤️ these comments.
@@ -4,5 +4,9 @@ $modalBackgroundColor: #FFF; | |||
$modalOverlayBackground: rgba(#000000, 0.5); | |||
$globalModalDepth: 1000; | |||
|
|||
// Dark theme colors | |||
$modalBorderColor--darkTheme: #000; | |||
$modalBackgroundColor--darkTheme: #272727; |
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.
This looks awesome overall btw |
Does this also fix #12988? |
Thanks for the review @cjcenizal, I've made those updates. @trevan unfortunately no, that modal lies outside of the application container, so the dark theme styles aren't getting applied to it. The changes in this PR might make it a little easier for someone to fix though. |
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!
Fixes elastic#12955 * Added dark theme support to the ui framework modal component * Also fixed z-index issue where dashboard panels would appear above the filter editor modal when the user moved or resized the panel (see gif below)
Fixes elastic#12955 * Added dark theme support to the ui framework modal component * Also fixed z-index issue where dashboard panels would appear above the filter editor modal when the user moved or resized the panel (see gif below)
Fixes elastic#12955 * Added dark theme support to the ui framework modal component * Also fixed z-index issue where dashboard panels would appear above the filter editor modal when the user moved or resized the panel (see gif below)
Fixes #12955 * Added dark theme support to the ui framework modal component * Also fixed z-index issue where dashboard panels would appear above the filter editor modal when the user moved or resized the panel (see gif below)
Fixes #12955 * Added dark theme support to the ui framework modal component * Also fixed z-index issue where dashboard panels would appear above the filter editor modal when the user moved or resized the panel (see gif below)
Fixes #12955
Note: I didn't change the colors of the uiSelect dropdowns. I took a swing at it but it turned into a big mess. There's tons of bootstrap and uiSelect styles that need to be overridden to do this properly. The white dropdowns don't look too bad to me, so I think if we want to add dark theme support to uiSelect we should do that in a separate PR.
z-index issue