-
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
Remove forced white text color in Image Banner & Slideshow #2663
Conversation
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.
Just jumping on some quick feedback. Approach seems reasonable enough to me.
max-width: 89rem; | ||
border: none; | ||
border-radius: 0; | ||
box-shadow: none; | ||
} | ||
|
||
.banner--desktop-transparent .button--secondary { | ||
--color-button: 255, 255, 255; | ||
--color-button-text: 255, 255, 255; | ||
--alpha-button-background: 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.
Although I obviously know why this is happening, this is a weird result. Or at least it's different than the desktop equivalent. I'd expect the secondary button to stay transparent unless the mobile container was enabled https://screenshot.click/19-41-z63lp-yb7ol.mp4
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 pushed 1c9af8e, which makes this work again. I think it's fine to have the background be transparent in all cases like that, but let me know if you notice anything unexpected.
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.
hm, it worked for me locally, but it's not working here 😅🤔
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.
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 reconnected the repo and updated the link in the description to the PR. Now it looks everything working good.
"color_scheme": { | ||
"info": "Visible when container displayed." | ||
}, |
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.
Ideally we go thru and remove these from all the translated files, though it technically won't hurt too much.
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.
Should be all set in 44a0c2e. 🎉
The gap will disappear once the theme is saved, but this is not a behavior we have in the current version of Dawn.
|
Closing in favor of #2668 for now. |
After some offline discussions, we're reopening this one. Should be ready for more dev reviews, and it needs a fix for this issue. |
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 can see that two things left to fix:
- Remove translations from all locales files.
- Address the issue when outline styled buttons on mobile inherit background colors from active container on desktop.
The rest 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.
It looks good to me. The only issue that I can see now is that gap between text and an image on mobile in the editor that despairs after you save. It seems that the issue came after we added animation, so it's not related to this pr. I approve 👍
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.
Beside what was already raised for the space, LGTM! 🎉
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.
Looking good. Just found one remaining discrepancy.
78dacd0
to
041a69c
Compare
041a69c
to
35ddeb6
Compare
I cannot replicate this issue anymore. Can some one else confirm that it's gone :) ? |
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.
* Remove forced white color. * Change color scheme for Dawn's default homepage Image Banner.. * Remove warning about color scheme usage. * Fix bug with transparent buttons on mobile. * Remove unused info key from translated files. * Update display of button when it's in a mobile container.
* Fix color for image in password (#2608) * [Image behavior] fixed background jitter fix on mobile (#2611) * [Animation in editor] Remove transform translate when re rendering and re ordering blocks (#2614) * Remove transform translate when re rendering and re ordering blocks * target only what's needed * [bug] Horizontal scrolling on mobile. (#2617) * Add overflow-x:hidden to prevent horizontal scrolling on mobile. * Change margin for slider for mobile and tablet. * Remove redundant preconnect to cdn.shopify.com (#2621) * Update reverse scheme (#2626) * [Footer] Remove Global Media settings inheritance from images (#2631) * Send timezone offset as string instead of integer (#2636) * Fix slider scrolling issue (#2635) * Fix background gradient for Related Products. (#2641) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Quick add remove animation from image and content (#2657) * Add default values for color scheme group (#2660) * Revert unwanted changes (#2669) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Revert "Update from Shopify for theme dawn/release/10.0.0" This reverts commit 89d927e. * Remove forced white text color in Image Banner & Slideshow (#2663) * Remove forced white color. * Change color scheme for Dawn's default homepage Image Banner.. * Remove warning about color scheme usage. * Fix bug with transparent buttons on mobile. * Remove unused info key from translated files. * Update display of button when it's in a mobile container. * Update existing placeholder images (#2610) * Update existing placeholder images * Featured collection and product card * Featured product section * Collection list section * Featured blog and multirow sections * Slideshow: change order of placeholder images * Collage adjustments for product and collection cards * Image with text adjustments * Featured product: re-add `.product--no-media` selectors * Update featured product section * Update `card-collection` * Cleanup `card-collection` --------- Co-authored-by: Ludo <[email protected]> * Gradient fix for transparent background medias and cards (#2651) * Fix media, product, and collection cards placeholder (#2682) * Fix missing else for collection cards (#2692) --------- Co-authored-by: Sofia Matulis <[email protected]> Co-authored-by: Eugene Kasimov <[email protected]> Co-authored-by: Mateusz Krzeszowiak <[email protected]> Co-authored-by: Lucas Lacerda <[email protected]> Co-authored-by: Kjell Reigstad <[email protected]> Co-authored-by: shopify[bot] <79544226+shopify[bot]@users.noreply.github.com> Co-authored-by: Ken Meleta <[email protected]> Co-authored-by: Andrew Etchen <[email protected]>
* Fix color for image in password (#2608) * [Image behavior] fixed background jitter fix on mobile (#2611) * [Animation in editor] Remove transform translate when re rendering and re ordering blocks (#2614) * Remove transform translate when re rendering and re ordering blocks * target only what's needed * [bug] Horizontal scrolling on mobile. (#2617) * Add overflow-x:hidden to prevent horizontal scrolling on mobile. * Change margin for slider for mobile and tablet. * Remove redundant preconnect to cdn.shopify.com (#2621) * Update reverse scheme (#2626) * [Footer] Remove Global Media settings inheritance from images (#2631) * Send timezone offset as string instead of integer (#2636) * Fix slider scrolling issue (#2635) * Fix background gradient for Related Products. (#2641) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Quick add remove animation from image and content (#2657) * Add default values for color scheme group (#2660) * Revert unwanted changes (#2669) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Revert "Update from Shopify for theme dawn/release/10.0.0" This reverts commit 89d927e. * Remove forced white text color in Image Banner & Slideshow (#2663) * Remove forced white color. * Change color scheme for Dawn's default homepage Image Banner.. * Remove warning about color scheme usage. * Fix bug with transparent buttons on mobile. * Remove unused info key from translated files. * Update display of button when it's in a mobile container. * Update existing placeholder images (#2610) * Update existing placeholder images * Featured collection and product card * Featured product section * Collection list section * Featured blog and multirow sections * Slideshow: change order of placeholder images * Collage adjustments for product and collection cards * Image with text adjustments * Featured product: re-add `.product--no-media` selectors * Update featured product section * Update `card-collection` * Cleanup `card-collection` --------- Co-authored-by: Ludo <[email protected]> * Gradient fix for transparent background medias and cards (#2651) * Fix media, product, and collection cards placeholder (#2682) * Fix missing else for collection cards (#2692) --------- Co-authored-by: Sofia Matulis <[email protected]> Co-authored-by: Eugene Kasimov <[email protected]> Co-authored-by: Mateusz Krzeszowiak <[email protected]> Co-authored-by: Lucas Lacerda <[email protected]> Co-authored-by: Kjell Reigstad <[email protected]> Co-authored-by: shopify[bot] <79544226+shopify[bot]@users.noreply.github.com> Co-authored-by: Ken Meleta <[email protected]> Co-authored-by: Andrew Etchen <[email protected]>
* made the test/default blog post 3 by default * blog default posts are not mobile responsive and on a slider. created snippet for blog-placeholder that renders into featured-blog with changable arguments * removed commented out code * default blog posts now are modifiable by post-limit and column number presets * reformated code, reverted header back to h1, removed snippet, and made changes as suggested * noticed the blog section needs a margin bottom when color scheme is applied * removed padding from blog and removed date and author presets * Fix link formatting in Related Products heading (#2680) * fix default UI for dropdown and mega menu (#2644) * added vertical padding and made l2 bold to mega menu * added vertical padding to drop down * fixed default UI for dropdown and mega menu * added hover effect to all l2 links and fixed overlapping or mega menu * adding padding to l3 links and fixed hover effect for dropdown * reverted the mega menu back to the current setting * changed horizontal grid gap on mega menu and vertical paddings on dropdown menu * made vertical grid gap at 1.8 rem to match the 3 rem vertical paddings on mega-menu container * removed the unnecessary lines of code * removed the text-thickness that was making l2/l3 links a different size from l1 links * added a hover effect over active link * removed uneccessary link size and white space * Add release/v10.0.0 branch fixes to main (#2693) * Fix color for image in password (#2608) * [Image behavior] fixed background jitter fix on mobile (#2611) * [Animation in editor] Remove transform translate when re rendering and re ordering blocks (#2614) * Remove transform translate when re rendering and re ordering blocks * target only what's needed * [bug] Horizontal scrolling on mobile. (#2617) * Add overflow-x:hidden to prevent horizontal scrolling on mobile. * Change margin for slider for mobile and tablet. * Remove redundant preconnect to cdn.shopify.com (#2621) * Update reverse scheme (#2626) * [Footer] Remove Global Media settings inheritance from images (#2631) * Send timezone offset as string instead of integer (#2636) * Fix slider scrolling issue (#2635) * Fix background gradient for Related Products. (#2641) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Quick add remove animation from image and content (#2657) * Add default values for color scheme group (#2660) * Revert unwanted changes (#2669) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Revert "Update from Shopify for theme dawn/release/10.0.0" This reverts commit 89d927e. * Remove forced white text color in Image Banner & Slideshow (#2663) * Remove forced white color. * Change color scheme for Dawn's default homepage Image Banner.. * Remove warning about color scheme usage. * Fix bug with transparent buttons on mobile. * Remove unused info key from translated files. * Update display of button when it's in a mobile container. * Update existing placeholder images (#2610) * Update existing placeholder images * Featured collection and product card * Featured product section * Collection list section * Featured blog and multirow sections * Slideshow: change order of placeholder images * Collage adjustments for product and collection cards * Image with text adjustments * Featured product: re-add `.product--no-media` selectors * Update featured product section * Update `card-collection` * Cleanup `card-collection` --------- Co-authored-by: Ludo <[email protected]> * Gradient fix for transparent background medias and cards (#2651) * Fix media, product, and collection cards placeholder (#2682) * Fix missing else for collection cards (#2692) --------- Co-authored-by: Sofia Matulis <[email protected]> Co-authored-by: Eugene Kasimov <[email protected]> Co-authored-by: Mateusz Krzeszowiak <[email protected]> Co-authored-by: Lucas Lacerda <[email protected]> Co-authored-by: Kjell Reigstad <[email protected]> Co-authored-by: shopify[bot] <79544226+shopify[bot]@users.noreply.github.com> Co-authored-by: Ken Meleta <[email protected]> Co-authored-by: Andrew Etchen <[email protected]> * made the test/default blog post 3 by default * added updated placeholder image * added the different placeholder images, while maintaining ability to edit with presets * blog card theme settings work on default blog posts * removed css that wasn't being used * reverted to original code without theme settings * fixed prettier error by removing extra quotation mark and made test blog default instead of news --------- Co-authored-by: Jon Neill <[email protected]> Co-authored-by: Ludo <[email protected]> Co-authored-by: Sofia Matulis <[email protected]> Co-authored-by: Eugene Kasimov <[email protected]> Co-authored-by: Mateusz Krzeszowiak <[email protected]> Co-authored-by: Lucas Lacerda <[email protected]> Co-authored-by: Kjell Reigstad <[email protected]> Co-authored-by: shopify[bot] <79544226+shopify[bot]@users.noreply.github.com> Co-authored-by: Ken Meleta <[email protected]> Co-authored-by: Andrew Etchen <[email protected]>
* Fix color for image in password (Shopify#2608) * [Image behavior] fixed background jitter fix on mobile (Shopify#2611) * [Animation in editor] Remove transform translate when re rendering and re ordering blocks (Shopify#2614) * Remove transform translate when re rendering and re ordering blocks * target only what's needed * [bug] Horizontal scrolling on mobile. (Shopify#2617) * Add overflow-x:hidden to prevent horizontal scrolling on mobile. * Change margin for slider for mobile and tablet. * Remove redundant preconnect to cdn.shopify.com (Shopify#2621) * Update reverse scheme (Shopify#2626) * [Footer] Remove Global Media settings inheritance from images (Shopify#2631) * Send timezone offset as string instead of integer (Shopify#2636) * Fix slider scrolling issue (Shopify#2635) * Fix background gradient for Related Products. (Shopify#2641) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Quick add remove animation from image and content (Shopify#2657) * Add default values for color scheme group (Shopify#2660) * Revert unwanted changes (Shopify#2669) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Revert "Update from Shopify for theme dawn/release/10.0.0" This reverts commit 89d927e. * Remove forced white text color in Image Banner & Slideshow (Shopify#2663) * Remove forced white color. * Change color scheme for Dawn's default homepage Image Banner.. * Remove warning about color scheme usage. * Fix bug with transparent buttons on mobile. * Remove unused info key from translated files. * Update display of button when it's in a mobile container. * Update existing placeholder images (Shopify#2610) * Update existing placeholder images * Featured collection and product card * Featured product section * Collection list section * Featured blog and multirow sections * Slideshow: change order of placeholder images * Collage adjustments for product and collection cards * Image with text adjustments * Featured product: re-add `.product--no-media` selectors * Update featured product section * Update `card-collection` * Cleanup `card-collection` --------- Co-authored-by: Ludo <[email protected]> * Gradient fix for transparent background medias and cards (Shopify#2651) * Fix media, product, and collection cards placeholder (Shopify#2682) * Fix missing else for collection cards (Shopify#2692) --------- Co-authored-by: Sofia Matulis <[email protected]> Co-authored-by: Eugene Kasimov <[email protected]> Co-authored-by: Mateusz Krzeszowiak <[email protected]> Co-authored-by: Lucas Lacerda <[email protected]> Co-authored-by: Kjell Reigstad <[email protected]> Co-authored-by: shopify[bot] <79544226+shopify[bot]@users.noreply.github.com> Co-authored-by: Ken Meleta <[email protected]> Co-authored-by: Andrew Etchen <[email protected]>
* made the test/default blog post 3 by default * blog default posts are not mobile responsive and on a slider. created snippet for blog-placeholder that renders into featured-blog with changable arguments * removed commented out code * default blog posts now are modifiable by post-limit and column number presets * reformated code, reverted header back to h1, removed snippet, and made changes as suggested * noticed the blog section needs a margin bottom when color scheme is applied * removed padding from blog and removed date and author presets * Fix link formatting in Related Products heading (Shopify#2680) * fix default UI for dropdown and mega menu (Shopify#2644) * added vertical padding and made l2 bold to mega menu * added vertical padding to drop down * fixed default UI for dropdown and mega menu * added hover effect to all l2 links and fixed overlapping or mega menu * adding padding to l3 links and fixed hover effect for dropdown * reverted the mega menu back to the current setting * changed horizontal grid gap on mega menu and vertical paddings on dropdown menu * made vertical grid gap at 1.8 rem to match the 3 rem vertical paddings on mega-menu container * removed the unnecessary lines of code * removed the text-thickness that was making l2/l3 links a different size from l1 links * added a hover effect over active link * removed uneccessary link size and white space * Add release/v10.0.0 branch fixes to main (Shopify#2693) * Fix color for image in password (Shopify#2608) * [Image behavior] fixed background jitter fix on mobile (Shopify#2611) * [Animation in editor] Remove transform translate when re rendering and re ordering blocks (Shopify#2614) * Remove transform translate when re rendering and re ordering blocks * target only what's needed * [bug] Horizontal scrolling on mobile. (Shopify#2617) * Add overflow-x:hidden to prevent horizontal scrolling on mobile. * Change margin for slider for mobile and tablet. * Remove redundant preconnect to cdn.shopify.com (Shopify#2621) * Update reverse scheme (Shopify#2626) * [Footer] Remove Global Media settings inheritance from images (Shopify#2631) * Send timezone offset as string instead of integer (Shopify#2636) * Fix slider scrolling issue (Shopify#2635) * Fix background gradient for Related Products. (Shopify#2641) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Quick add remove animation from image and content (Shopify#2657) * Add default values for color scheme group (Shopify#2660) * Revert unwanted changes (Shopify#2669) * Update from Shopify for theme dawn/release/10.0.0 Committed from shop: Skeleton Theme * Revert "Update from Shopify for theme dawn/release/10.0.0" This reverts commit 89d927e. * Remove forced white text color in Image Banner & Slideshow (Shopify#2663) * Remove forced white color. * Change color scheme for Dawn's default homepage Image Banner.. * Remove warning about color scheme usage. * Fix bug with transparent buttons on mobile. * Remove unused info key from translated files. * Update display of button when it's in a mobile container. * Update existing placeholder images (Shopify#2610) * Update existing placeholder images * Featured collection and product card * Featured product section * Collection list section * Featured blog and multirow sections * Slideshow: change order of placeholder images * Collage adjustments for product and collection cards * Image with text adjustments * Featured product: re-add `.product--no-media` selectors * Update featured product section * Update `card-collection` * Cleanup `card-collection` --------- Co-authored-by: Ludo <[email protected]> * Gradient fix for transparent background medias and cards (Shopify#2651) * Fix media, product, and collection cards placeholder (Shopify#2682) * Fix missing else for collection cards (Shopify#2692) --------- Co-authored-by: Sofia Matulis <[email protected]> Co-authored-by: Eugene Kasimov <[email protected]> Co-authored-by: Mateusz Krzeszowiak <[email protected]> Co-authored-by: Lucas Lacerda <[email protected]> Co-authored-by: Kjell Reigstad <[email protected]> Co-authored-by: shopify[bot] <79544226+shopify[bot]@users.noreply.github.com> Co-authored-by: Ken Meleta <[email protected]> Co-authored-by: Andrew Etchen <[email protected]> * made the test/default blog post 3 by default * added updated placeholder image * added the different placeholder images, while maintaining ability to edit with presets * blog card theme settings work on default blog posts * removed css that wasn't being used * reverted to original code without theme settings * fixed prettier error by removing extra quotation mark and made test blog default instead of news --------- Co-authored-by: Jon Neill <[email protected]> Co-authored-by: Ludo <[email protected]> Co-authored-by: Sofia Matulis <[email protected]> Co-authored-by: Eugene Kasimov <[email protected]> Co-authored-by: Mateusz Krzeszowiak <[email protected]> Co-authored-by: Lucas Lacerda <[email protected]> Co-authored-by: Kjell Reigstad <[email protected]> Co-authored-by: shopify[bot] <79544226+shopify[bot]@users.noreply.github.com> Co-authored-by: Ken Meleta <[email protected]> Co-authored-by: Andrew Etchen <[email protected]>
PR Summary:
Always allow text and button color customization within the Image banner and Slideshow sections.
Important: Color scheme may need to be manually changed to prevent visual change and ensure adequate contrast.
Why are these changes introduced?
Fixes #1391
What approach did you take?
This PR does a few things:
background-1
(which uses black text) toinverse
(which uses white text).For new merchants, this approach maintains the acceptable contrast ratio that existed by default on the top of the homepage template for Dawn (the text and buttons are still white, so there's no perceptable change.)
In other areas, we cannot guarantee an acceptable ratio today anyway: If the background image is light, and/or there's no overlay applied, white text is unreadable. In the past, the merchant was stuck with that being broken. With this new approach, the merchant can fix it by applying a new color scheme.
🎥 Video
19-54-lyzp0-8gej1.mp4
Other considerations
Visual changes
If selected color scheme doesn't use white text, image banners and slideshows without content containers may have a visual change. Shops will need to manually update their color scheme to account for this.
Testing steps/scenarios
Demo links
Checklist