Skip to content
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.

Sign up for GitHub

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

[Slideshow] Add fixed position background #2346

Closed
wants to merge 4 commits into from
Closed

[Slideshow] Add fixed position background #2346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d35253b
[Slideshow] Add fixed position background
kjellr Feb 28, 2023
a64ea39
Try a fix for rendering issues in Chrome.
kjellr Mar 1, 2023
5fd57ea
Add a new fixed background approach.
kjellr Mar 3, 2023
ea64e39
Merge branch 'main' into fixed-position-image-slideshow
kjellr Mar 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions locales/en.default.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2923,6 +2923,18 @@
"change_slides_speed": {
"label": "Change slides every"
},
"animations": {
"content": "Animations"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

},
"image_behavior": {
"options__1": {
"label": "None"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

},
"options__2": {
"label": "Fixed background position"
},
"label": "Image behavior"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

},
"mobile": {
"content": "Mobile layout"
},
Expand Down
39 changes: 38 additions & 1 deletion sections/slideshow.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

clip: rect(1px, 1px, 1px, 1px);
clip-path: inset(50%);
height: 1px;
width: 1px;
margin: -1px;
overflow: hidden;
padding: 0;
position: absolute;
}
{%- endif -%}
</style>
<div
class="slideshow__slide grid__item grid--1-col slider__slide"
Expand All @@ -127,7 +144,7 @@
aria-label="{{ forloop.index }} {{ 'general.slider.of' | t }} {{ forloop.length }}"
tabindex="-1"
>
<div class="slideshow__media banner__media media{% if block.settings.image == blank %} placeholder{% endif %}">
<div class="slideshow__media banner__media media{% if block.settings.image == blank %} placeholder{% endif %}{% if section.settings.image_behavior != 'none' %} animate--{{ section.settings.image_behavior }}{% endif %}">
{%- if block.settings.image -%}
{%- assign height = block.settings.image.width | divided_by: block.settings.image.aspect_ratio | round -%}
{{
Expand Down Expand Up @@ -338,6 +355,26 @@
"label": "t:sections.slideshow.settings.change_slides_speed.label",
"default": 5
},
{
"type": "header",
"content": "t:sections.slideshow.settings.animations.content"
},
{
"type": "select",
"id": "image_behavior",
"options": [
{
"value": "none",
"label": "t:sections.slideshow.settings.image_behavior.options__1.label"
},
{
"value": "fixed",
"label": "t:sections.slideshow.settings.image_behavior.options__2.label"
}
],
"default": "none",
"label": "t:sections.slideshow.settings.image_behavior.label"
},
{
"type": "header",
"content": "t:sections.slideshow.settings.mobile.content"
Expand Down