fix(theme): fix dark theme bugs #5535
Conversation
|
Preview: https://patternfly-pr-5535.surge.sh A11y report: https://patternfly-pr-5535-a11y.surge.sh |
mcoker
requested review from
mcarrano,
srambach,
andrew-ronaldson,
mceledonia and
thatblindgeye
There was a problem hiding this comment.
I thought that a good way to check this out would be to look at some of our full screen demos. But now I'm seeing that when demos open in a new window, they no longer inherit dark theme even it that's set in the workspace. Is this expected? I expect that if dark theme is set at the workspace level, demos as well as embedded examples will inherit that theme.
There was a problem hiding this comment.
Regarding the full screen demos, I realize that is unrelated to this PR. However I feel like it's something we should try to fix with the website/workspace as it will be surprising if we cannot preview these demos in light or dark.
For this PR, I will defer to @mceledonia and @andrew-ronaldson for design review, although in taking a quick look, seems like some things are still unresolved. I'm especially seeing this for Brand and components that use the brand like cards. Is there a fix there?
|
Rebased with the latest from v5. Note that the dark theme toggle on full page examples isn't working, I put up a fix for that here and will rebase this PR once that fix is published to the docs framework package - patternfly/patternfly-org#3532 For now you can manually enable dark theme on those pages by by adding |
|
@mcoker should be able to rebase and get the fixed theme switcher |
There was a problem hiding this comment.
Went through the demo and the changes look good. Thanks @mcoker
There was a problem hiding this comment.
@mcoker This looks good, but I just have a few questions about things that I'm seeing.
-
Is it possible to swap the brand image in examples where we are looking at the black on white PatternFly brand placeholder? I'm guessing that occurs because we have black text on a transparent background. See Brand and Card examples specifically.
-
I'm noticing that outlined label borders have a weird pixelation in dark theme.
Gray borders seem OK. Anything that can be done about that? (actually looks better in this screen shot than live on my monitor) -
I see that the background of the User menu changes when I switch to dark theme. Is that what we expect or want?
@andrew-ronaldson @mceledonia interested in your thoughts
|
I added a margin to the green labels to check these borders and I think helps with the pixelation @mcarrano saw. |
|
There's a contrast failure on the read-only code editor. |
|
File upload drag over states fail contrast - should the background be a darker gray? https://patternfly-pr-5535.surge.sh/components/file-upload/multiple-file-upload |
|
🎉 This PR is included in version 5.0.0-alpha.58 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fixes #5523
@srambach @thatblindgeye also
updated docs for the input group(oops, just saw #5518), and the use of input group in the slider.@mceledonia @andrew-ronaldson the updates we discussed today should be in place. I also added the floating background color to the "raised" panel variation - is that OK? I believe the intention of that variation is to be used for stuff you may want to attach to a menu toggle. In the future, I believe our plan is to always have the raised panel variation wrap the menu when toggling a menu with the menu toggle - then we'll remove the header/filter/search and the footer from the menu component, and build that into the panel's header/footer instead. Here is a use of the panel in this PR - it holds the form below this text input group: https://patternfly-pr-5535.surge.sh/components/text-input-group/#search-input-group-advanced-search-expanded