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

Hide variant images setting #158

Merged
merged 18 commits into from
Aug 19, 2021
Merged

Hide variant images setting #158

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d007e4a
Show currently selected variant image only
ludoboludo Jul 8, 2021
6cfb828
rebase part1
ludoboludo Aug 5, 2021
702c71b
edit copy
ludoboludo Jul 13, 2021
a422119
change if statement order
ludoboludo Jul 15, 2021
d60a827
rebase part6
ludoboludo Aug 5, 2021
a02eb3b
rebase part8
ludoboludo Aug 5, 2021
2163ff5
re add code tweaks to variant and slider in global.js
ludoboludo Aug 5, 2021
69fcff7
add scrollLeft into setTimeout
ludoboludo Aug 5, 2021
5d02205
address reviewer's feedback
ludoboludo Aug 9, 2021
224dced
add timer into the onScroll function
ludoboludo Aug 9, 2021
0b1a452
update modalContent selector
ludoboludo Aug 12, 2021
409b004
address reviewer's comment and focus on adding functionality for the …
ludoboludo Aug 12, 2021
87e4326
Update locales/en.default.schema.json
ludoboludo Aug 17, 2021
b3291eb
address reviewer's feedback
ludoboludo Aug 17, 2021
c09d46d
update the dispatchEvent
ludoboludo Aug 18, 2021
6163377
remove code duplicate
ludoboludo Aug 19, 2021
dfefb32
fix modal selector
ludoboludo Aug 19, 2021
f7a7397
Update 10 translation files
translation-platform[bot] Aug 19, 2021
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
27 changes: 21 additions & 6 deletions assets/global.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,15 +456,17 @@ class SliderComponent extends HTMLElement {
}

initPages() {
if (!this.sliderItems.length === 0) return;
this.slidesPerPage = Math.floor(this.slider.clientWidth / this.sliderItems[0].clientWidth);
this.totalPages = this.sliderItems.length - this.slidesPerPage + 1;
this.sliderItemsToShow = Array.from(this.sliderItems).filter(element => element.clientWidth > 0);
this.sliderLastItem = this.sliderItemsToShow[this.sliderItemsToShow.length - 1];
if (this.sliderItemsToShow.length === 0) return;
this.slidesPerPage = Math.floor(this.slider.clientWidth / this.sliderItemsToShow[0].clientWidth);
this.totalPages = this.sliderItemsToShow.length - this.slidesPerPage + 1;
this.update();
}

update() {
if (!this.pageCount || !this.pageTotal) return;
this.currentPage = Math.round(this.slider.scrollLeft / this.sliderItems[0].clientWidth) + 1;
this.currentPage = Math.round(this.slider.scrollLeft / this.sliderLastItem.clientWidth) + 1;

if (this.currentPage === 1) {
this.prevButton.setAttribute('disabled', true);
Expand All @@ -484,7 +486,11 @@ class SliderComponent extends HTMLElement {

onButtonClick(event) {
event.preventDefault();
const slideScrollPosition = event.currentTarget.name === 'next' ? this.slider.scrollLeft + this.sliderItems[0].clientWidth : this.slider.scrollLeft - this.sliderItems[0].clientWidth;
const slideScrollPosition = event.currentTarget.name === 'next' ? this.slider.scrollLeft + this.sliderLastItem.clientWidth : this.slider.scrollLeft - this.sliderLastItem.clientWidth;
this.slider.scrollTo({
left: slideScrollPosition
});

this.slider.scrollTo({
left: slideScrollPosition
});
Expand All @@ -496,7 +502,9 @@ customElements.define('slider-component', SliderComponent);
class VariantSelects extends HTMLElement {
constructor() {
super();
this.header = document.querySelector('sticky-header');
this.addEventListener('change', this.onVariantChange);
this.mediaChangeEvent = new Event('mediaChange', {"bubbles": true});
}

onVariantChange() {
Expand Down Expand Up @@ -533,10 +541,17 @@ class VariantSelects extends HTMLElement {
const newMedia = document.querySelector(
`[data-media-id="${this.dataset.section}-${this.currentVariant.featured_media.id}"]`
);
const modalContent = document.querySelector('.product-media-modal__content');
const newMediaModal = modalContent.querySelector( `[data-media-id="${this.currentVariant.featured_media.id}"]`);

if (!newMedia) return;
const parent = newMedia.parentElement;
if (parent.firstChild == newMedia) return;
modalContent.prepend(newMediaModal);
parent.prepend(newMedia);
window.setTimeout(() => { parent.scroll(0, 0) });
parent.dispatchEvent(this.mediaChangeEvent);
window.setTimeout(() => { parent.scrollLeft = 0; parent.scrollIntoView({behavior: "smooth", inline: "start"}); });
window.setTimeout(() => { this.header.productMediaIsChanging = false;}, 800);
}

updateURL() {
Expand Down
10 changes: 10 additions & 0 deletions assets/section-main-product.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,16 @@ a.product__text {
}
}

.product__media-item--variant,
.product-media-modal__content > .product__media-item--variant.product__media-item--variant {
display: none;
}

.product__media-item--variant:first-child,
.product-media-modal__content > .product__media-item--variant:first-child {
display: block;
}

.product__media-icon .icon {
width: 1.2rem;
height: 1.4rem;
Expand Down
3 changes: 3 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 @@ -1284,6 +1284,9 @@
"enable_sticky_info": {
"label": "Enable sticky product information on large screens"
},
"hide_variants": {
"label": "Hide other variants’ media when a variant is selected"
Copy link
Contributor

Choose a reason for hiding this comment

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

If the expectation is to have all variant media visible inside the modal, does this setting naming still makes sense? My expectation is that turning this on will hide other variants' media EVERYWHERE.

Copy link
Contributor

Choose a reason for hiding this comment

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

The expectation is to hide everywhere when the setting is true, I believe it was the case when I tested if I recall. Otherwise it was overlooked, when this setting is enabled, it applies to all devices! 🙌

Copy link
Contributor

@melissaperreault melissaperreault Aug 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
"label": "Hide other variants’ media when a variant is selected"
"label": "Hide other variants’ media after selecting a variant"

Hey @gretchen-willenberg the reason why we didn't see this line of text is because the PR was not merged yet. I suggested the change based on your audit! Confirming this is a setting label, so no period at the end of the sentence.

},
"enable_video_looping": {
"label": "Enable video looping"
}
Expand Down
29 changes: 22 additions & 7 deletions sections/header.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,15 +437,21 @@
this.header = document.getElementById('shopify-section-header');
this.headerBounds = {};
this.currentScrollTop = 0;
this.productMediaIsChanging = false;

this.productSection = document.querySelector('.product-section');

this.onScrollHandler = this.onScroll.bind(this);
this.productMediaScroll = this.productMediaChanged.bind(this);

this.productSection?.addEventListener('mediaChange', this.productMediaScroll)
window.addEventListener('scroll', this.onScrollHandler, false);

this.createObserver();
}

disconnectedCallback() {
this.productSection?.removeEventListener('mediaChange', this.productMediaScroll)
window.removeEventListener('scroll', this.onScrollHandler);
}

Expand All @@ -458,18 +464,27 @@
observer.observe(this.header);
}

productMediaChanged() {
this.productMediaIsChanging = true;
}

onScroll() {
const scrollTop = window.pageYOffset || document.documentElement.scrollTop;

if (scrollTop > this.currentScrollTop && scrollTop > this.headerBounds.bottom) {
if (this.productMediaIsChanging) {
requestAnimationFrame(this.hide.bind(this));
} else if (scrollTop < this.currentScrollTop && scrollTop > this.headerBounds.bottom) {
requestAnimationFrame(this.reveal.bind(this));
} else if (scrollTop <= this.headerBounds.top) {
requestAnimationFrame(this.reset.bind(this));
}
this.currentScrollTop = scrollTop;
} else {
if (scrollTop > this.currentScrollTop && scrollTop > this.headerBounds.bottom) {
requestAnimationFrame(this.hide.bind(this));
} else if (scrollTop < this.currentScrollTop && scrollTop > this.headerBounds.bottom) {
requestAnimationFrame(this.reveal.bind(this));
} else if (scrollTop <= this.headerBounds.top) {
requestAnimationFrame(this.reset.bind(this));
}

this.currentScrollTop = scrollTop;
this.currentScrollTop = scrollTop;
}
}

hide() {
Expand Down
14 changes: 11 additions & 3 deletions sections/main-product.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,16 @@
{{ "accessibility.skip_to_product_info" | t }}
</a>
<ul class="product__media-list grid grid--peek list-unstyled slider slider--mobile" role="list">
{%- assign variant_images = product.images | where: 'attached_to_variant?', true | map: 'src' -%}
{%- if product.selected_or_first_available_variant.featured_media != null -%}
{%- assign media = product.selected_or_first_available_variant.featured_media -%}
<li class="product__media-item grid__item slider__slide{% if media.media_type != 'image' %} product__media-item--full{% endif %}" data-media-id="{{ section.id }}-{{ media.id }}">
<li class="product__media-item grid__item slider__slide{% if media.media_type != 'image' %} product__media-item--full{% endif %}{% if section.settings.hide_variants and variant_images contains media.src %} product__media-item--variant{% endif %}" data-media-id="{{ section.id }}-{{ media.id }}">
{% render 'product-thumbnail', media: media, position: 'featured', loop: section.settings.enable_video_looping, modal_id: section.id %}
</li>
{%- endif -%}
{%- for media in product.media -%}
{%- unless media.id == product.selected_or_first_available_variant.featured_media.id -%}
<li class="product__media-item grid__item slider__slide{% if media.media_type != 'image' %} product__media-item--full{% endif %}" data-media-id="{{ section.id }}-{{ media.id }}">
<li class="product__media-item grid__item slider__slide{% if media.media_type != 'image' %} product__media-item--full{% endif %}{% if section.settings.hide_variants and variant_images contains media.src %} product__media-item--variant{% endif %}" data-media-id="{{ section.id }}-{{ media.id }}">
{% render 'product-thumbnail', media: media, position: forloop.index, loop: section.settings.enable_video_looping, modal_id: section.id %}
</li>
{%- endunless -%}
Expand All @@ -44,7 +45,7 @@
<span class="slider-counter--current">1</span>
<span aria-hidden="true"> / </span>
<span class="visually-hidden">{{ 'accessibility.of' | t }}</span>
<span class="slider-counter--total">{{ product.media.size }}</span>
<span class="slider-counter--total">{% if section.settings.hide_variants %}{{ product.media.size | minus: variant_images.size | plus: 1 }}{% else %}{{ product.media.size }}{% endif %}</span>
</div>
<button type="button" class="slider-button slider-button--prev" name="previous" aria-label="{{ 'accessibility.previous_slide' | t }}">{% render 'icon-caret' %}</button>
<button type="button" class="slider-button slider-button--next" name="next" aria-label="{{ 'accessibility.next_slide' | t }}">{% render 'icon-caret' %}</button>
Expand Down Expand Up @@ -332,6 +333,7 @@
width="1100"
height="{{ 1100 | divided_by: media.preview_image.aspect_ratio | ceil }}"
data-media-id="{{ media.id }}"
{% if section.settings.hide_variants and variant_images contains media.src %}class="product__media-item--variant"{% endif %}
>
{%- else -%}
{%- if media.media_type == 'model' -%}
Expand Down Expand Up @@ -739,6 +741,12 @@
"default": true,
"label": "t:sections.main-product.settings.enable_sticky_info.label"
},
{
"type": "checkbox",
"id": "hide_variants",
"default": false,
"label": "t:sections.main-product.settings.hide_variants.label"
},
{
"type": "checkbox",
"id": "enable_video_looping",
Expand Down