[Slideshow] Add fixed position background #2346

kjellr · 2023-02-28T20:43:33Z

PR Summary:

Adds a fixed position background option to the Slideshow section.

fixed-position.mp4

Why are these changes introduced?

Closes #2345.

What approach did you take?

This implements a new dropdown menu as shown in the ticket. When "Fixed background position" is selected, a CSS class is applied to the media elements within the Slideshow slides. This class sets the position of the media element to fixed, and uses clip-path(0) on the parent to ensure the image doesn't bleed outside of its intended container.

The CSS for this is defined in base.css so that we can theoretically reuse it for the Image with Text (#2340) and Image Banner (#2341) sections.

Other considerations

⚠ We may want to re-evaluate this approach, since it's causing some issues in Chrome. Sometimes, the pseudo element and/or slideshow content doesn't appear correctly when you scroll into it or advance onto the next slide. Scrolling a little or moving the mouse near the slideshow fixes this. From what I can infer, this is related to the use of clip-path. These issues are not present in Safari or Firefox. 😞

If for some reason this approach falls through, we could also explore setting the background-image of the media container, and using background-attachment: fixed. This should avoid the issues, but is far more verbose and likely less performant, as it eliminates the ability for us to use srcset. 😕

📹 Example

28-33-mid8y-50ej0.mp4

Visual impact on existing themes

None. This is an optional feature.

Testing steps/scenarios

Visit the demo store.
Add and select a Slideshow section.
Look for the new "Image behavior" dropdown underneath the new "Animations" header in the sidebar.
Select "Fixed position background" from that dropdown.
Add various images, verify that they appear as expected.
Try various breakpoints, verify that they appear as expected.

Checklist

Added PR summary for release notes
Requested review from UX (Only for changes that are affecting the experience or perceivable visual details)
Created a ticket for the help.shopify.com documentation team about updates to theme settings. (Internal-only task)
Followed theme code principles
Linted with Theme Check
Tested on mobile
Tested on multiple browsers
Tested for accessibility

assets/base.css

stufreen

Not working for me unfortunately. I would be OK to use a background-image in this case - we can make it responsive using either several divs or the newish image-set CSS function. We will need to add some stuff for a11y if we go that route.

assets/base.css

kimberlyoleiro · 2023-03-03T17:29:30Z

Not working for me either. Both Chrome & Safari. No fixed background effect appearing

kjellr · 2023-03-03T21:17:23Z

Not working for me unfortunately. I would be OK to use a background-image in this case - we can make it responsive using either several divs or the newish image-set CSS function. We will need to add some stuff for a11y if we go that route.

I pushed a new commit that uses background-image to get this working again. This doesn't implement any responsive behavior yet — it just loads in a huge image. We can try implementing with image-set() next week. The main caveat is that Safari still doesn't support w-based values, just the x-based density ones. 😕

In regards to accessibility, I retained the original image and just hid it using one of the WebAIM-recommended techniques. This is one approach for exposing the image to screen readers. The other I know of would be to remove the <img> tag (I think we may be loading it twice here?) and add role="img" alt-text="..." on the banner__media div. I'm open to suggestions, but I think the latter is probably preferable.

(The PR description above still outlines about the initial version — I can update next week.)

DavidSheerin · 2023-03-07T11:45:10Z

Hey 👋 Just seen this issue and thought I'd toss in my two cents from a support perspective 🪙 🪙

For the css property background-attachment: fixed; that is being proposed, there are limitations with it's usage in Safari on iOS. When this property is used, it often creates frustration and/or confusion for merchants which we tend to see in tickets escalated to Theme Support. A current example within Dawn is where the fixed gradient doesn't get applied to the header or footer on mobile and iPad. In a previous review, we found that this particular issue made up ~5% of issue tickets that had been escalated to our team. I suspect the fixed image effect would be more popular than the gradient and as a result, create more support debt - just something to consider if implementing.

A similar effect was previously used in the Narrative theme that caused a lot of frustration for merchants at the time and to overcome the Safari issue on iOS, a function was added to the js to check the device and a class as a fallback. I don't believe it solved the problem as there were still further issues reported and it's implementation goes against the zero js principle but thought I'd share for some added context.

metamoni · 2023-03-08T10:43:52Z

sections/slideshow.liquid

@@ -117,6 +117,23 @@
        #Slide-{{ section.id }}-{{ forloop.index }} .banner__media::after {
          opacity: {{ block.settings.image_overlay_opacity | divided_by: 100.0 }};
        }
+        {%- if section.settings.image_behavior == 'fixed' -%}
+          #Slide-{{ section.id }}-{{ forloop.index }} .animate--fixed {
+            background: url("{{ block.settings.image | image_url: width: 3840 }}") center no-repeat;


Are we sure we always want this size for the image?

metamoni · 2023-03-08T10:47:38Z

sections/slideshow.liquid

+            background-size: cover;
+            background-attachment: fixed;
+          }
+          #Slide-{{ section.id }}-{{ forloop.index }} .animate--fixed img {


Are these sr-only styles? We should create a class in base.css, maybe right where we have our visually-hidden class. It would make it obvious what this is, as sr-only is a widely used and familiar class name and we could make use of it in the future.

metamoni

Tested this on Safari, Firefox, Edge, Chrome, with different breakpoints. It seems to work fine for me!

melissaperreault

Thank you! It behaves as expected from a UX perspective.

Small finds

The focal point is not preserved (Video reference)
The Image slide placeholder illustration background changes as you select from None to the new setting. (Video reference)

translation-platform · 2024-05-15T14:43:03Z

locales/en.default.schema.json

@@ -2923,6 +2923,18 @@
        "change_slides_speed": {
          "label": "Change slides every"
        },
+        "animations": {
+          "content": "Animations"


Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

The value Animations for key sections.slideshow.settings.animations.content is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

translation-platform · 2024-05-15T14:43:05Z

locales/en.default.schema.json

+        },
+        "image_behavior": {
+          "options__1": {
+            "label": "None"


Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

The value None for key sections.slideshow.settings.image_behavior.options__1.label is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

translation-platform · 2024-05-15T14:43:07Z

locales/en.default.schema.json

+          "options__2": {
+            "label": "Fixed background position"
+          },
+          "label": "Image behavior"


Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

The value Image behavior for key sections.slideshow.settings.image_behavior.label is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

LucasLacerdaUX · 2024-05-15T14:45:42Z

Closing this as it's inactive/old.

[Slideshow] Add fixed position background

d35253b

kjellr added Category: Enhancement New feature or request motion labels Feb 28, 2023

kjellr requested review from stufreen, metamoni, melissaperreault and LucasLacerdaUX February 28, 2023 20:43

kjellr self-assigned this Feb 28, 2023

metamoni reviewed Mar 1, 2023

View reviewed changes

assets/base.css Outdated Show resolved Hide resolved

Try a fix for rendering issues in Chrome.

a64ea39

stufreen reviewed Mar 1, 2023

View reviewed changes

assets/base.css Outdated Show resolved Hide resolved

assets/base.css Outdated Show resolved Hide resolved

Add a new fixed background approach.

5fd57ea

Merge branch 'main' into fixed-position-image-slideshow

ea64e39

stufreen unassigned kjellr Mar 6, 2023

metamoni reviewed Mar 8, 2023

View reviewed changes

melissaperreault reviewed Mar 8, 2023

View reviewed changes

LucasLacerdaUX self-assigned this Mar 9, 2023

kimberlyoleiro added 🎨 Design: Animations Bugs, features or enhancements related to animations and removed motion labels May 18, 2023

LucasLacerdaUX removed their request for review May 31, 2023 14:00

translation-platform bot reviewed May 15, 2024

View reviewed changes

LucasLacerdaUX closed this May 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Slideshow] Add fixed position background #2346

[Slideshow] Add fixed position background #2346

kjellr commented Feb 28, 2023

stufreen left a comment •

edited

Loading

kimberlyoleiro commented Mar 3, 2023

kjellr commented Mar 3, 2023 •

edited

Loading

DavidSheerin commented Mar 7, 2023

metamoni Mar 8, 2023

metamoni Mar 8, 2023

metamoni left a comment

melissaperreault left a comment •

edited

Loading

translation-platform bot May 15, 2024

translation-platform bot May 15, 2024

translation-platform bot May 15, 2024

LucasLacerdaUX commented May 15, 2024

[Slideshow] Add fixed position background #2346

[Slideshow] Add fixed position background #2346

Conversation

kjellr commented Feb 28, 2023

PR Summary:

Why are these changes introduced?

What approach did you take?

Other considerations

Visual impact on existing themes

Testing steps/scenarios

Checklist

stufreen left a comment • edited Loading

Choose a reason for hiding this comment

kimberlyoleiro commented Mar 3, 2023

kjellr commented Mar 3, 2023 • edited Loading

DavidSheerin commented Mar 7, 2023

metamoni Mar 8, 2023

Choose a reason for hiding this comment

metamoni Mar 8, 2023

Choose a reason for hiding this comment

metamoni left a comment

Choose a reason for hiding this comment

melissaperreault left a comment • edited Loading

Choose a reason for hiding this comment

translation-platform bot May 15, 2024

Choose a reason for hiding this comment

translation-platform bot May 15, 2024

Choose a reason for hiding this comment

translation-platform bot May 15, 2024

Choose a reason for hiding this comment

LucasLacerdaUX commented May 15, 2024

stufreen left a comment •

edited

Loading

kjellr commented Mar 3, 2023 •

edited

Loading

melissaperreault left a comment •

edited

Loading