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.

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

feat(storefront): BCTHEME-425 Incorrect focus order for product carousels #2034

Merged
merged 1 commit into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## Draft
- Incorrect focus order for product carousels. [#2034](https://github.com/bigcommerce/cornerstone/pull/2034)
- Removed duplicates of main tag.[#2032](https://github.com/bigcommerce/cornerstone/pull/2032)
- Hamburger Menu Icon missing on Google AMP Pages. [#2022](https://github.com/bigcommerce/cornerstone/pull/2022)
- Wish List drop down is truncated on product page. [#2001](https://github.com/bigcommerce/cornerstone/pull/2001)
Expand Down
1 change: 1 addition & 0 deletions assets/js/theme/common/carousel/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const FOCUSABLE_ELEMENTS_SELECTOR = '[href], button, input, textarea, select, details, [contenteditable="true"], [tabindex]';
51 changes: 25 additions & 26 deletions assets/js/theme/common/carousel/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,70 +2,69 @@ import 'slick-carousel';

import {
activatePlayPauseButton,
analizeSlides,
arrowAriaLabling,
dotsSetup,
getActiveSlideIdxAndSlidesQuantity,
handleImageAspectRatio,
handleImageLoad,
setTabindexes,
tooltipSetup,
refreshFocus,
updateTextWithLiveData,
} from './utils';

/**
* returns activeSlideIdx and slidesQuantity
* based on provided carousel settings
* @param {Object} $slickSettings
* @returns {Object}
*/
const extractSlidesDetails = ({
slideCount, $slides, options: { slidesToShow, slidesToScroll },
}) => getActiveSlideIdxAndSlidesQuantity(
slideCount,
slidesToShow,
slidesToScroll,
$slides,
);
export const setCarouselState = ({ delegateTarget }, carouselObj) => {
const carouselObjCurrent = carouselObj || delegateTarget.slick;
const { $slider } = carouselObjCurrent;

$slider.data('state', getActiveSlideIdxAndSlidesQuantity(carouselObjCurrent));
};

export const onUserCarouselChange = ({ data }, context, $slider) => {
const $activeSlider = $slider || data;
const $parentContainer = $activeSlider.hasClass('productView-thumbnails') ? $('.productView-images') : $activeSlider;
const { activeSlideIdx, slidesQuantity } = extractSlidesDetails($activeSlider[0].slick);
const $parentContainer = $activeSlider.hasClass('productView-thumbnails') ? $activeSlider.parent('.productView-images') : $activeSlider;
const { activeSlideIdx, slidesQuantity } = $activeSlider.data('state');
const $carouselContentElement = $('[data-carousel-content-change-message]', $parentContainer);
const carouselContentAnnounceMessage = updateTextWithLiveData(context.carouselContentAnnounceMessage, (activeSlideIdx + 1), slidesQuantity);

$carouselContentElement.text(carouselContentAnnounceMessage);
};

export const onSlickCarouselChange = (e, carousel, context) => {
export const onSlickCarouselChange = (e, carouselObj, context) => {
const {
$dots,
$slider,
$prevArrow,
$nextArrow,
} = carousel;
options: { infinite },
} = carouselObj;

const { activeSlideIdx, slidesQuantity } = extractSlidesDetails(carousel);
const { activeSlideIdx, slidesQuantity } = $slider.data('state') || getActiveSlideIdxAndSlidesQuantity(carouselObj);
yurytut1993 marked this conversation as resolved.
Show resolved Hide resolved

dotsSetup($dots, activeSlideIdx, slidesQuantity, context);
arrowAriaLabling($prevArrow, $nextArrow, activeSlideIdx, slidesQuantity, context.carouselArrowAndDotAriaLabel);
setTabindexes($slider.find('.slick-slide'));
tooltipSetup($prevArrow, $nextArrow, $dots);
activatePlayPauseButton(carousel, slidesQuantity, context);
arrowAriaLabling($prevArrow, $nextArrow, activeSlideIdx, slidesQuantity, infinite, context.carouselArrowAndDotAriaLabel);
analizeSlides($slider.find('.slick-slide'));
refreshFocus($prevArrow, $nextArrow, $dots, $slider, activeSlideIdx, slidesQuantity, infinite);

$slider.data('state', null);
yurytut1993 marked this conversation as resolved.
Show resolved Hide resolved
};

export default function (context) {
$('[data-slick]').each((idx, carousel) => {
// getting element using find to pass jest test
const $carousel = $(document).find(carousel);

$carousel.on('init breakpoint swipe', setCarouselState);
$carousel.on('click', '.slick-arrow, .slick-dots', setCarouselState);

$carousel.on('init breakpoint', (e, carouselObj) => activatePlayPauseButton(e, carouselObj, context));
yurytut1993 marked this conversation as resolved.
Show resolved Hide resolved
$carousel.on('init afterChange', (e, carouselObj) => onSlickCarouselChange(e, carouselObj, context));
$carousel.on('click', '.slick-arrow, .slick-dots', $carousel, e => onUserCarouselChange(e, context));
$carousel.on('swipe', (e, carouselObj) => onUserCarouselChange(e, context, carouselObj.$slider));

if ($carousel.hasClass('heroCarousel')) {
$carousel.on('init afterChange', handleImageLoad);
$carousel.on('swipe', handleImageAspectRatio);
$carousel.on('click', '.slick-arrow, .slick-dots', $carousel, handleImageAspectRatio);
$carousel.on('click', '.slick-arrow, .slick-dots', handleImageAspectRatio);

// Alternative image styling for IE, which doesn't support objectfit
if (typeof document.documentElement.style.objectFit === 'undefined') {
Expand Down
48 changes: 25 additions & 23 deletions assets/js/theme/common/carousel/utils/activatePlayPauseButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,51 @@ import { throttle } from 'lodash';

const PLAY_ACTION = 'slickPlay';
const PAUSE_ACTION = 'slickPause';
const IS_ACTIVATED_DATA_ATTR = 'is-activated';

export default (carousel, slidesQuantity, context) => {
const { $slider, $dots, speed } = carousel;
const $playPauseButton = $slider.find('[data-play-pause-button]');

if ($playPauseButton.length === 0) return;

$playPauseButton.css('display', slidesQuantity < 2 ? 'none' : 'block');

if ($playPauseButton.data(IS_ACTIVATED_DATA_ATTR)) return;

const updateButtonLabels = (context) => {
const {
carouselPlayPauseButtonPlay,
carouselPlayPauseButtonPause,
carouselPlayPauseButtonAriaPlay,
carouselPlayPauseButtonAriaPause,
} = context;

const updateLabels = action => {
$playPauseButton
return ($button, action) => {
$button
.text(action === PLAY_ACTION
? carouselPlayPauseButtonPause : carouselPlayPauseButtonPlay)
.attr('aria-label', action === PLAY_ACTION
? carouselPlayPauseButtonAriaPause : carouselPlayPauseButtonAriaPlay);
};
};
let updateButtonLabelsWithContext;

const onPlayPauseClick = () => {
const action = carousel.paused ? PLAY_ACTION : PAUSE_ACTION;
export default (e, carouselObj, context) => {
const { $slider, $dots, options: { speed } } = carouselObj;
const $playPauseButton = $slider.find('[data-play-pause-button]');

$slider.slick(action);
updateLabels(action);
};
if ($playPauseButton.length === 0) return;

// for correct carousel controls focus order
if ($dots) {
$playPauseButton.insertBefore($dots);
} else $slider.append($playPauseButton);

$playPauseButton.on('click', throttle(onPlayPauseClick, speed, { trailing: false }));
$playPauseButton.data(IS_ACTIVATED_DATA_ATTR, true);
const { slidesQuantity } = $slider.data('state');
$playPauseButton.css('display', slidesQuantity > 1 ? 'block' : 'none');

if (carousel.breakpoints.length) {
$slider.on('breakpoint', () => updateLabels(PLAY_ACTION));
if (e.type === 'init') updateButtonLabelsWithContext = updateButtonLabels(context);

if (e.type === 'breakpoint') {
BC-tymurbiedukhin marked this conversation as resolved.
Show resolved Hide resolved
updateButtonLabelsWithContext($playPauseButton, PLAY_ACTION);
return;
}

const onPlayPauseClick = () => {
const action = carouselObj.paused ? PLAY_ACTION : PAUSE_ACTION;

$slider.slick(action);
updateButtonLabelsWithContext($playPauseButton, action);
};

$playPauseButton.on('click', throttle(onPlayPauseClick, speed, { trailing: false }));
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const FOCUSABLE_ELEMENTS_SELECTOR = '[href], button, input, textarea, select, details, [contenteditable="true"], [tabindex]';
import { FOCUSABLE_ELEMENTS_SELECTOR } from '../constants';

export default ($slides) => {
$slides.each((idx, slide) => {
Expand Down
15 changes: 12 additions & 3 deletions assets/js/theme/common/carousel/utils/arrowAriaLabling.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
import updateTextWithLiveData from './updateTextWithLiveData';
import tooltipSetup from './tooltipSetup';

export default ($prevArrow, $nextArrow, activeSlideIdx, slidesQuantity, ariaLabel) => {
export default ($prevArrow, $nextArrow, activeSlideIdx, slidesQuantity, isInfinite, ariaLabel) => {
if (slidesQuantity < 2 || !$prevArrow || !$nextArrow) return;

const activeSlideNumber = activeSlideIdx + 1;

const prevSlideNumber = activeSlideIdx === 0 ? slidesQuantity : activeSlideNumber - 1;
const arrowLeftText = updateTextWithLiveData(ariaLabel, prevSlideNumber, slidesQuantity);

$prevArrow.attr('aria-label', arrowLeftText);
$prevArrow.attr({
'aria-label': arrowLeftText,
tabindex: !isInfinite && activeSlideIdx === 0 ? -1 : 0,
});
tooltipSetup($prevArrow);

const nextSlideNumber = activeSlideIdx === slidesQuantity - 1 ? 1 : activeSlideNumber + 1;
const arrowRightText = updateTextWithLiveData(ariaLabel, nextSlideNumber, slidesQuantity);

$nextArrow.attr('aria-label', arrowRightText);
$nextArrow.attr({
'aria-label': arrowRightText,
tabindex: !isInfinite && activeSlideIdx === slidesQuantity - 1 ? -1 : 0,
});
tooltipSetup($nextArrow);
};
4 changes: 3 additions & 1 deletion assets/js/theme/common/carousel/utils/dotsSetup.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import updateTextWithLiveData from './updateTextWithLiveData';
import tooltipSetup from './tooltipSetup';

export default ($dots, activeSlideIdx, slidesQuantity, { carouselArrowAndDotAriaLabel, carouselActiveDotAriaLabel }) => {
if (!$dots) return;
Expand All @@ -14,7 +15,8 @@ export default ($dots, activeSlideIdx, slidesQuantity, { carouselArrowAndDotAria
const dotLabelText = updateTextWithLiveData(carouselArrowAndDotAriaLabel, idx + 1, slidesQuantity);
const dotSlideStatusText = idx === activeSlideIdx ? `, ${carouselActiveDotAriaLabel}` : '';
const dotAriaLabel = `${dotLabelText}${dotSlideStatusText}`;
const $dotButton = $(dot).find('[data-carousel-dot]');

$(dot).find('[data-carousel-dot]').attr('aria-label', dotAriaLabel);
tooltipSetup($dotButton.attr('aria-label', dotAriaLabel));
});
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export default (slideCount, slidesToShow, slidesToScroll, $slides) => {
export default ({ slideCount, $slides, options: { slidesToShow, slidesToScroll } }) => {
const lastVisibleIdx = $slides.get().reduce((acc, curr, idx) => {
if ($(curr).hasClass('slick-active')) return idx;
return acc;
Expand Down
4 changes: 2 additions & 2 deletions assets/js/theme/common/carousel/utils/index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
export { default as activatePlayPauseButton } from './activatePlayPauseButton';
export { default as analizeSlides } from './analizeSlides';
export { default as arrowAriaLabling } from './arrowAriaLabling';
export { default as dotsSetup } from './dotsSetup';
export { default as getActiveSlideIdxAndSlidesQuantity } from './getActiveSlideIdxAndSlidesQuantity';
export { default as handleImageAspectRatio } from './handleImageAspectRatio';
export { default as handleImageLoad } from './handleImageLoad';
export { default as setTabindexes } from './setTabindexes';
export { default as tooltipSetup } from './tooltipSetup';
export { default as refreshFocus } from './refreshFocus';
export { default as updateTextWithLiveData } from './updateTextWithLiveData';
20 changes: 20 additions & 0 deletions assets/js/theme/common/carousel/utils/refreshFocus.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { FOCUSABLE_ELEMENTS_SELECTOR } from '../constants';

export default ($prevArrow, $nextArrow, $dots, $slider, activeSlideIdx, slidesQuantity, isInfinite) => {
if (isInfinite || !$prevArrow || !$nextArrow) return;

if (activeSlideIdx === 0 && $prevArrow.is(':focus')) {
$nextArrow.focus();
} else if (activeSlideIdx === slidesQuantity - 1 && $nextArrow.is(':focus')) {
if ($dots) {
$dots.children().first().find('[data-carousel-dot]').focus();
return;
}

const $firstActiveSlide = $slider.find('.slick-active').first();

if ($firstActiveSlide.is(FOCUSABLE_ELEMENTS_SELECTOR)) {
$firstActiveSlide.focus();
} else $firstActiveSlide.find(FOCUSABLE_ELEMENTS_SELECTOR).first().focus();
}
};
20 changes: 1 addition & 19 deletions assets/js/theme/common/carousel/utils/tooltipSetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const TOOLTIP_DATA_SELECTOR = 'data-carousel-tooltip';
const TOOLTIP_CLASS = 'carousel-tooltip';
const TOOLTIP_NODE = `<span ${TOOLTIP_DATA_SELECTOR} class="${TOOLTIP_CLASS}"></span>`;

const setupTooltipAriaLabel = ($node) => {
export default ($node) => {
const $existedTooltip = $node.find(`[${TOOLTIP_DATA_SELECTOR}]`);
if ($existedTooltip.length) {
$existedTooltip.attr('aria-label', $node.attr('aria-label'));
Expand All @@ -11,21 +11,3 @@ const setupTooltipAriaLabel = ($node) => {
$node.append($tooltip);
}
};

const setupArrowTooltips = (...arrowNodes) => {
arrowNodes.forEach($arrow => setupTooltipAriaLabel($arrow));
};

const setupDotTooltips = ($dots) => {
$dots.children().each((idx, dot) => setupTooltipAriaLabel($('[data-carousel-dot]', dot)));
};

export default ($prevArrow, $nextArrow, $dots) => {
if ($prevArrow && $nextArrow) {
setupArrowTooltips($prevArrow, $nextArrow);
}

if ($dots) {
setupDotTooltips($dots);
}
};
7 changes: 5 additions & 2 deletions assets/js/theme/global/quick-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import utils from '@bigcommerce/stencil-utils';
import ProductDetails from '../common/product-details';
import { defaultModal } from './modal';
import 'slick-carousel';
import { onSlickCarouselChange, onUserCarouselChange } from '../common/carousel';
import { setCarouselState, onSlickCarouselChange, onUserCarouselChange } from '../common/carousel';

export default function (context) {
const modal = defaultModal();
Expand All @@ -24,7 +24,10 @@ export default function (context) {
const $carousel = modal.$content.find('[data-slick]');

if ($carousel.length) {
$carousel.on('init afterChange', (e, carousel) => onSlickCarouselChange(e, carousel, context));
$carousel.on('init breakpoint swipe', setCarouselState);
$carousel.on('click', '.slick-arrow, .slick-dots', setCarouselState);

$carousel.on('init afterChange', (e, carouselObj) => onSlickCarouselChange(e, carouselObj, context));
$carousel.on('click', '.slick-arrow, .slick-dots', $carousel, e => onUserCarouselChange(e, context));
$carousel.on('swipe', (e, carouselObj) => onUserCarouselChange(e, context, carouselObj.$slider));

Expand Down