Theme switch in examples #5022
Theme switch in examples #5022
Conversation
|
Demo starting at https://vanilla-framework-5022.demos.haus |
|
@bartaz I added the toggles as button rather than radio buttons, let me know what you think. Also I'm not so sure how to handle some dark examples like the ones in navigation, the |
scss/_patterns_rule.scss
Outdated
| &.is-accent { | ||
| @include vf-highlight-bar($color-accent); | ||
| } | ||
| @include vf-highlight-bar($colors--theme--text-default); |
There was a problem hiding this comment.
You didn't have to do this, thanks :)
I think by mistake you removed the .is-accent version. I don't think we have separate themed accent colour, so for now just keep it as is, regardless of the theme.
| <button class="p-button--negative is-dark">Negative button</button> | ||
| <button class="p-button--brand is-dark">Brand button</button> | ||
| <div class="p-strip is-shallow"> | ||
| <button class="p-button">Default button</button> |
There was a problem hiding this comment.
I'm not sure if that works, because buttons have not been migrated to new theming yet, so they need to have is-dark class on them (they don't inherit the theme).
You can keep the is_dark = True part to have class on body, but the is-dark class on buttons needs to stay for now. I know that your toggle will not work properly here, but it's fine.
(I commented in MM about the conditional, so that's another option as well).
Overall, the is-dark classes on buttons need to stay until buttons are properly themed.
| @@ -5,7 +5,7 @@ | |||
|
|
|||
| {% set is_paper = true %} | |||
| {% block content %} | |||
| <div class="p-strip is-dark"> | |||
| <div class="p-strip"> | |||
There was a problem hiding this comment.
This example is used in the docs to show how to make strip dark, so in this case we want the is-dark class on strip, not on body (to show how to change single strip to dark).
So for now it's ok that theme switch will not work (or will not be available) on this example.
| <div class="u-fixed-width p-section--shallow"> | ||
| <hr class="p-rule--highlight is-dark"> | ||
| <hr class="p-rule--highlight"> |
There was a problem hiding this comment.
This will also not work, the rule component does not support new theming yet, so it will not inherit the theme. It needs to have is-dark class on itself.
So the switch will not work here yet, please revert this example.
| </style> | ||
| {% endblock %} | ||
|
|
||
| {%set is_dark = True%} |
There was a problem hiding this comment.
Just a bit of missing whitespace for consistency
| {%set is_dark = True%} | |
| {% set is_dark = True %} |
| </style> | ||
| {% endblock %} | ||
|
|
||
| {%set is_dark = True%} |
There was a problem hiding this comment.
As you mentioned in the comment, this is tricky one because if images. We can't change images based on the theme, we don't have a solution for it (outside of built in icons). We can't change src based on theme or anything, at least not easily.
So I guess it's ok to just keep this one dark, with switch not working.
| {% block content %} | ||
| <div class="p-strip is-dark"> | ||
| <div class="p-strip"> |
There was a problem hiding this comment.
You can remove the strip completely, it was only there to make backround dark.
templates/_layouts/examples.html
Outdated
| @@ -4,6 +4,7 @@ | |||
| <head> | |||
| <meta charset="utf-8"/> | |||
| <meta name="viewport" content="width=device-width, initial-scale=1"> | |||
| <meta id="is-not-themed" data-is_not_themed="{{is_not_themed}}"> | |||
There was a problem hiding this comment.
This is an interesting approach to inject the value into the template, but because we only need to read it in JS, maybe it would be simpler (and more reliable) to pass a JS variable instead?
Something like:
<script>
const SHOW_THEME_SWITCH = {% if is_not_themed %}False{% else %}True{% endif %};
</script>
And then you can just check the value of this global var?
Co-authored-by: Bartek Szopka <83575+bartaz@users.noreply.github.com>
…nilla-framework into theme-switch-in-examples
bartaz
added
Documentation 📝
Done
[List of work items including drive-bys]
Fixes [list issues/bugs if needed]
QA
Go to docs/examples, click on any example and ensure that there are buttons at the bottom to change between themes
Click the themes and ensure it changes accordingly
Click on dark examples and ensure it's dark by default, then click on light or paper theme and ensure it changes accordingly.
Open demo
[Add QA steps]
Review updated documentation:
Check if PR is ready for release
If this PR contains Vanilla SCSS code changes, it should contain the following changes to make sure it's ready for the release:
Feature 🎁,Breaking Change 💣,Bug 🐛,Documentation 📝,Maintenance 🔨.package.jsonshould be updated relative to the most recent release, following semver convention:Screenshots
[if relevant, include a screenshot or screen capture]