-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
fix 'Save modal dark theme issues in Discover, Dashboard, and Maps' #149147
Conversation
f7651f4
to
08a0c0b
Compare
…-ref HEAD~1..HEAD --fix'
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.
Presentation team changes LGTM! It's great to see the plugin system being actually used as a DI mechanism instead of making the consumers pass in common dependencies!
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
Pinging @elastic/kibana-presentation (Team:Presentation) |
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations) |
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, tested in Discover and the theme is applying correctly now.
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 good from our end. Code review
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.
maps changes lgtm!
code review and tested dark mode in chrome
Fixes #149130 and #142636
PR updates
showSaveModal
to wrap modal inKibanaThemeProvider
to resolve dark theme issues. Rather than passing in theme$ as a new parameter, theme$ is imported from within the plugin itself.