-
Notifications
You must be signed in to change notification settings - Fork 3.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
Update Preset default #410
Conversation
Colors look good based on the preset values Wik provided. I'd like us to touch base with @Guillaumegranger1 and/or @wiktoriaswiecicka about the announcement bar, header and footer preset settings. Because these are static sections, it means that their settings are going to be stored in the For example: looking at the Default preset mockups, it looks like we should be using the |
The default preset/demo actually has the
|
Thanks Guillaume. Is that the same for the Craft preset? Based on the mockups, the announcement bar and footer look like they use |
@martinamarien The 2nd preset uses the |
No problem at all. We just need to know this information so that we can set the footer and announcement bar color scheme as part of the preset. For example, right now, the footer's color scheme defaults to |
config/settings_data.json
Outdated
"announcement-bar": { | ||
"type": "announcement-bar", | ||
"settings": { | ||
"color_scheme": "accent-1" | ||
} | ||
}, |
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.
accent-1
is actually already the default value for the announcement bar color scheme. So we don't need to include this here because it will default to that color anyways.
Instead, we will need to include this for the Default preset (unless the default value of the color scheme changes for the announcement bar).
The code you have used here would work for a section that contains only one announcement bar - as in the section itself is an individual announcement bar. In this case, our announcement bar section contains blocks of type: announcement
that each have their own settings. So we would need to provide overrides for each block instead of for the section as a whole. So it might looks something like this:
"Default": {
"sections": {
"announcement-bar": {
"type": "announcement-bar",
"blocks": {
"announcement-bar-0": {
"type": "announcement",
"settings": {
...
}
}
},
"block_order": [
"announcement-bar-0"
]
}
}
},
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.
accent-1 is actually already the default value for the announcement bar color scheme. So we don't need to include this here because it will default to that color anyways.
I had changed the default value in the section itself since it for the default style but I do it the way you mentioned instead.
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 to me.
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 have a minor comment, but I'm approving this as I'm not 100% sure on this one (never implemented presets myself before). Could be a bug on my store.
Please try reproducing the issue on your own store.
config/settings_data.json
Outdated
"Default": { | ||
"sections": { |
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 think you need to explicitly repeat the preset values here, otherwise you can't go back to the default preset once you change it.
"Default": { | |
"sections": { | |
"Default": { | |
"colors_solid_button_labels": "#FFFFFF", | |
"colors_accent_1": "#121212", | |
"colors_accent_2": "#334FB4", | |
"colors_text": "#121212", | |
"colors_outline_button_labels": "#121212", | |
"colors_background_1": "#FFFFFF", | |
"colors_background_2": "#F3F3F3", | |
"type_header_font": "lato_n3", | |
"type_body_font": "lato_n3", | |
"sections": { |
To reproduce the issue:
- Go to
Theme Settings
->Theme Style
- Change from
Default
toCraft
. Save
and reload the page. You can't go back toDefault
preset.- Add the default values like I provided above. Repeat steps 1-3. It should now be possible to go back to default preset.
Maybe I'm doing something wrong or it's a bug with my store, though, so please validate before accepting this suggestion.
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.
Ludo and I have both been able to reproduce this and have shared it with the appropriate team.
That said, if you change the current theme and upload it, it does show the preset correctly with the correct overrides. So I think we can be confident that the issue is not with the presets in the settings_data
file.
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.
Ah but I see what Lucas means. If you set the default to Craft
in settings_data.json
which I think would replicate what happens when a merchant uploads a specific style from the theme store or free theme banner in the admin. Then change the style to default... it has nothing to go back to.
Video explanation (ugh my mic didn't work but basically I set the default as craft and switch back to default and nothing happens)
b1ce1df
@LucasLacerdaUX and @martinamarien just need a re review, I added the colors and font in the preset for the Default style. |
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.
third time's the charm, I guess
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 great! ✨
commit 399626d Author: tairli <tairli@yahoo.com> Date: Mon Aug 23 04:47:25 2021 +0930 Update product-form.js commit 330e6b4 Merge: 79467e4 7a2f34f Author: tairli <tairli@yahoo.com> Date: Mon Aug 23 04:45:54 2021 +0930 Merge remote-tracking branch 'upstream/main' into main commit 7a2f34f Author: Lucas Lacerda <37168033+LucasLacerdaUX@users.noreply.github.com> Date: Thu Aug 19 15:55:39 2021 -0400 Add featured product section (Shopify#261) commit 9789ae1 Author: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> Date: Thu Aug 19 15:52:08 2021 -0400 Update 2 translation files (Shopify#449) Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> commit d83f16f Author: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> Date: Thu Aug 19 15:51:51 2021 -0400 Update 7 translation files (Shopify#448) Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> commit ce0e5ec Author: Ludo <ludo.segura@shopify.com> Date: Thu Aug 19 14:46:05 2021 -0400 Hide variant images setting (Shopify#158) commit d3b44e7 Author: Ludo <ludo.segura@shopify.com> Date: Thu Aug 19 10:29:50 2021 -0400 Update font style for Default preset (Shopify#435) * Update font style for Default preset * updated default in settings_schema commit 2643bab Author: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> Date: Thu Aug 19 10:29:34 2021 -0400 Update 15 translation files (Shopify#439) Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> commit e9a2fd4 Author: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> Date: Thu Aug 19 10:02:16 2021 -0400 Update 4 translation files (Shopify#440) Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> commit 9ea0392 Author: Ludo <ludo.segura@shopify.com> Date: Wed Aug 18 17:19:16 2021 -0400 404 continue shopping style (Shopify#419) commit 6c2128a Author: Ken Meleta <30790058+kmeleta@users.noreply.github.com> Date: Wed Aug 18 15:14:33 2021 -0600 Update price sale badge spacing (Shopify#430) commit a6d25cf Author: Kai <KaichenWang@users.noreply.github.com> Date: Wed Aug 18 11:13:22 2021 -0400 Adjust global focus styles for inputs, selects and collection filter UI elements (Shopify#389) * Adjust global focus styles for inputs, selects and collection filter UI elements * Fix focus for filter drawer summary elements * Simplify focus for elements that are always "focus-visible" when in focus * Adjust focus style of search submit button commit 002b6b0 Author: Ken Meleta <30790058+kmeleta@users.noreply.github.com> Date: Wed Aug 18 09:01:47 2021 -0600 Product card spacing and price polish (Shopify#407) * Adjust product card spacing and price styles * Fix unit price/sale badge spacing * Revert flex gap approach commit 6fa46b7 Author: Ludo <ludo.segura@shopify.com> Date: Wed Aug 18 10:54:58 2021 -0400 Update Preset default (Shopify#410) commit 341e051 Author: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> Date: Wed Aug 18 10:43:37 2021 -0400 Update 10 translation files (Shopify#427) Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com> commit 79467e4 Merge: 42cad96 789c3d9 Author: tairli <tairli@yahoo.com> Date: Wed Aug 18 19:26:00 2021 +0930 Merge branch 'Shopify:main' into main commit 42cad96 Merge: b5ddb38 2ba2725 Author: tairli <tairli@yahoo.com> Date: Tue Aug 17 22:06:29 2021 +0930 Merge branch 'Shopify:main' into main
Why are these changes introduced?
Fixes #58 .
What approach did you take?
Updated the default values based on the linked issue.
Other considerations
Demo links
Checklist