-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[Header] Move 'Logo' to Global Theme settings #2083
Conversation
I have a few questions here around how this behaves. Will this new 'Global logo' impact only the password page header logo and the homepage header logo? In what other places, sections and blocks is this logo displayed and for those, would the merchant ever want two colors? (e.g. the logo in color, and the logo in reverse colors) |
@danielvan It should only impact the A downside of this decision, like you mentioned, is that the image now must have enough contrast with the color schemes being applied to both these sections. Our current use of logos is restricted to nearly identical contexts (headers) so we felt confident that this would not result in many issues for merchants. For existing merchants, we will include a note in the release notes that they should review the password-header & header sections before publishing the updated theme, as only the main |
Hey! In the case that the logo lives in theme settings there is a new situation to prevent confusion for.
|
@melissaperreault That's a really good point. We originally considered whether or not it should also apply to this page, but Checkout settings are not part of the theme so they cannot be combined. The idea is to keep all checkout settings separate. I see value in including a help text to say where it's displayed so the merchant doesn't have to search for it, considering there's a "width" setting you might not know where it's applied to (cc @allylane). As for the confusion in having duplicates: checkout settings also contain separate Accent & Button colors, Typography, etc, and we don't include a helper text on those theme settings to say they won't be reflected in checkout. Should we consider it for all these settings in this case, or do you think the Logo is more likely to cause confusion? |
@LucasLacerdaUX Looks good!
|
It would be ideal in general that if we could present the Checkout settings panel in view/in context of what you can edit so you don't have a big learning curve that the logo, typography and colors of Checkout is made in a separate category in the panel. Might not be worth solving or adding extra content for now and monitor if merchants are more confused. It's true that it is already the case with colors and that we would need to look at this from a more global robust angle. |
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 🎉 Added a few comments
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.
Hey Lucas, all look good and work as expected. I left a minor comment and a couple of questions.
7c348b9
to
8652e81
Compare
8652e81
to
528beda
Compare
We have received a translation request but there is a question blocking translation work. Your help is needed. Please answer the following questions.
|
@sofiamatulis @eugenekasimov I requested a new review now that translations have been pushed. All the comments have been addressed :D |
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 🎉 Works as expected
Added one question
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 🎉
* shopify/main: (57 commits) Change sticky header settings (Shopify#2165) Image setting updates (Shopify#2148) Update 1 translation file (Shopify#2183) Update 2 translation files (Shopify#2182) Fix mega-menu vertical shadow issue (Shopify#2147) Update translations: buyer (Shopify#2180) Change product recommendations name to related products (Shopify#2161) Update 2 translation files (Shopify#2178) Add default text for cart-notification link (Shopify#2167) Update 1 translation file (Shopify#2177) Update 20 translation files (Shopify#2176) Update 1 translation file (Shopify#2174) Updates to collage settings and defaults (Shopify#2150) Update 1 translation file (Shopify#2173) Update issue templates Update layout label (Shopify#2164) Search ux improvements (Shopify#2127) Prettier all liquid files [part 2] (Shopify#2126) [Footer] Add 'Show policy links' setting (Shopify#2084) [Header] Move 'Logo' to Global Theme settings (Shopify#2083) ...
PR Summary:
Important: We moved “Logo” and “Logo width” settings from the header and password header sections to the global theme settings under the heading “Logo”. This update may cause a visual change to your logo.
Why are these changes introduced?
Moves logo to so it can be configured in a single place and used in multiple parts of the theme.
Fix #2090
Visual impact on existing themes
The
header
andpassword-header
sections will no longer have different logo pickers. A single logo is now used across both section, with the same width setting. This may require merchants to make some adjustments to either their logo image or to the settings in these sections.Testing steps/scenarios
Demo links
Checklist