Follow ups for accessibility of announcements slider #2580

eugenekasimov · 2023-04-27T02:16:05Z

PR Summary:

This PR:

changes settings of auto-rotation for announcements. Min speed is 5 sec, max is 10 sec, off for default.
prevents auto-rotation for mobile.
stops auto-rotation after first interaction with an arrow button.

Why are these changes introduced?

Fixes #2494

What approach did you take?

Using JS I prevent auto-rotation for mobile and also stop it if a user interacts with an arrow button.

Other considerations

Decision log

Visual impact on existing themes

Testing steps/scenarios

Add a few announcements to the announcement bar.
In the settings set Auto-rotate slides.
Change the screen size to mobile and make sure that the slider stops rotation, then resize the screen back and make sure the slider continues rotating. (! while testing this don't interact with arrow buttons since this will stop the slider).
Interact with an arrow button and make sure that the slider stops rotating. Resize the screen to mobile and back and make sure it doesn't resume rotating.

Demo links

Editor

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

Prevent auto-rotation for mobile. Change settings for auto-rotation.

stufreen

Are we sure we want to apply the changes to every Slideshow, or is it just for Announcement bar?

If we only want the changes on Announcement bar then it may be time to create another custom component. With the animation changes we will probably need a different component anyway.

assets/global.js

eugenekasimov · 2023-04-27T17:44:31Z

Are we sure we want to apply the changes to every Slideshow, or is it just for Announcement bar?

If we only want the changes on Announcement bar then it may be time to create another custom component. With the animation changes we will probably need a different component anyway.

@stufreen I agree that we want to create a new component since the announcement bar slider is quite different now from the slide-show component. However, I'd do it as a different task. For now I just apply the current changes to the announcement bar.

@melissaperreault @YoannJailin, Do we want to apply below changes to the slideshow section as well? So far they are applied to the announcement bar only.

prevents auto-rotation for mobile.
stops auto-rotation after first interaction with an arrow button.

!update:
@melissaperreault confirmed that we would want to apply these changes to the slideshow section as well. We'll create a separate issue for that and it's going to be a different PR.

melissaperreault

LGTM! 🎉

melissaperreault · 2023-04-27T20:43:03Z

This issue this PR says it will fix was meant to be the reference for the Slideshow section though #2494

This PR is based of this feedback made on the other PR #2394 (comment)

Do you want me to create an issue to track?

ludoboludo

I agree with Stu that adding a feature specific logic in a general component class seem a bit off.
But for now we can go with that. We can create an issue and look into a refactor later.

assets/global.js

ludoboludo · 2023-04-28T14:19:27Z

assets/global.js

+    this.sliderArrowButtons.forEach((button) => {
+      button.addEventListener('click', () => {
+        this.arrowButtonWasInteracted = true;
+      }, {once: true});
+    });


Here I don't think you need to add/create a new event listener. We already have an existing listener on the buttons from the SliderComponent.
So you could just add the functionality that you need in the onButtonClick function specifically when it's the announcement bar buttons.

Video explanation.

I know you use the once: true on the listener you declared but you should be able to run your specific logic just once in onButtonClick as well by have a check.

Though because it's something that isn't general to slideshows but only in one specific use case (announcement bars), it might be better to keep it separate 🤔

@stufreen do you have a preference yourself ?

assets/global.js

stufreen

LGTM with a couple minor changes

assets/global.js

ludoboludo

One small suggestion otherwise looks good 👍

assets/global.js

* Stop auto-rotation after first interaction with an arrow button. Prevent auto-rotation for mobile. Change settings for auto-rotation. * Change 'resize' to 'change' and max-width to min-width. * Apply the current changes to the announcement bar only. * Renaming variables in JS. * Add a condition for the announcement bar for JS. * Refactoring JS code. * Refactor JS. * Combine mediaQuery into array.

…#2580)" This reverts commit af83afd.

* shopify/main: (59 commits) [Announcement bar] Add social icons (Shopify#2497) Update theme version to match the pubic release (Shopify#2698) Add release/v10.0.0 branch fixes to main (Shopify#2693) fix default UI for dropdown and mega menu (Shopify#2644) Fix link formatting in Related Products heading (Shopify#2680) Update 2 translation files (Shopify#2637) Enable gift card recipient form by default on featured product section (Shopify#2666) Gift cards/enable recipient form by default (Shopify#2618) Add a Color Scheme setting for Menus-Header (Shopify#2622) Made mobile drawer full width by default-header (Shopify#2625) Allow multiple announcement bars in Header group (Shopify#2619) [Feat Product] Add rating styling sheet (Shopify#2620) Fix password page variables (Shopify#2607) Fix transform applied when it should not for sliders (Shopify#2606) Modify info string for gift card recipient checkbox (Shopify#2588) [3D lift animation] Raise hovered card above adjacent cards (Shopify#2589) Remove fallback color scheme info text (Shopify#2602) Fix CSS specifity issue to cancel animation for theme editor events (Shopify#2605) Remove settings daya for icon color (Shopify#2601) Follow ups for accessibility of announcements slider (Shopify#2580) ...

* Stop auto-rotation after first interaction with an arrow button. Prevent auto-rotation for mobile. Change settings for auto-rotation. * Change 'resize' to 'change' and max-width to min-width. * Apply the current changes to the announcement bar only. * Renaming variables in JS. * Add a condition for the announcement bar for JS. * Refactoring JS code. * Refactor JS. * Combine mediaQuery into array.

Stop auto-rotation after first interaction with an arrow button.

1727d86

Prevent auto-rotation for mobile. Change settings for auto-rotation.

eugenekasimov self-assigned this Apr 27, 2023

stufreen reviewed Apr 27, 2023

View reviewed changes

assets/global.js Outdated Show resolved Hide resolved

assets/global.js Outdated Show resolved Hide resolved

eugenekasimov added 2 commits April 27, 2023 09:20

Change 'resize' to 'change' and max-width to min-width.

4dcf680

Apply the current changes to the announcement bar only.

a89de56

eugenekasimov requested review from stufreen, YoannJailin and melissaperreault April 27, 2023 17:44

melissaperreault approved these changes Apr 27, 2023

View reviewed changes

ludoboludo self-requested a review April 28, 2023 13:42

ludoboludo reviewed Apr 28, 2023

View reviewed changes

eugenekasimov added 2 commits April 28, 2023 09:38

Renaming variables in JS.

9396596

Add a condition for the announcement bar for JS.

00d23e0

eugenekasimov requested a review from ludoboludo April 28, 2023 17:18

Refactoring JS code.

441ab20

ludoboludo reviewed May 1, 2023

View reviewed changes

assets/global.js Outdated Show resolved Hide resolved

assets/global.js Outdated Show resolved Hide resolved

Refactor JS.

adefa39

stufreen approved these changes May 1, 2023

View reviewed changes

assets/global.js Outdated Show resolved Hide resolved

assets/global.js Outdated Show resolved Hide resolved

ludoboludo approved these changes May 2, 2023

View reviewed changes

assets/global.js Show resolved Hide resolved

Combine mediaQuery into array.

a92da33

eugenekasimov requested a review from ludoboludo May 2, 2023 17:54

ludoboludo approved these changes May 2, 2023

View reviewed changes

eugenekasimov merged commit 39e020b into main May 2, 2023

eugenekasimov deleted the improve-accessibility-announcement-bar branch May 2, 2023 18:32

jabrms pushed a commit to fp-trading/fp-trading-theme that referenced this pull request Jun 12, 2023

Revert "Follow ups for accessibility of announcements slider (Shopify…

7d1da5f

…#2580)" This reverts commit af83afd.

kjellr mentioned this pull request Jul 10, 2023

[Announcement Bar] Auto-rotate fixes #2810

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Follow ups for accessibility of announcements slider #2580

Follow ups for accessibility of announcements slider #2580

eugenekasimov commented Apr 27, 2023 •

edited

Loading

stufreen left a comment

eugenekasimov commented Apr 27, 2023 •

edited

Loading

melissaperreault left a comment

melissaperreault commented Apr 27, 2023

ludoboludo left a comment

ludoboludo Apr 28, 2023

ludoboludo Apr 28, 2023

stufreen left a comment

ludoboludo left a comment

Follow ups for accessibility of announcements slider #2580

Follow ups for accessibility of announcements slider #2580

Conversation

eugenekasimov commented Apr 27, 2023 • edited Loading

PR Summary:

Why are these changes introduced?

What approach did you take?

Other considerations

Decision log

Visual impact on existing themes

Testing steps/scenarios

Demo links

Checklist

stufreen left a comment

Choose a reason for hiding this comment

eugenekasimov commented Apr 27, 2023 • edited Loading

melissaperreault left a comment

Choose a reason for hiding this comment

melissaperreault commented Apr 27, 2023

ludoboludo left a comment

Choose a reason for hiding this comment

ludoboludo Apr 28, 2023

Choose a reason for hiding this comment

ludoboludo Apr 28, 2023

Choose a reason for hiding this comment

stufreen left a comment

Choose a reason for hiding this comment

ludoboludo left a comment

Choose a reason for hiding this comment

eugenekasimov commented Apr 27, 2023 •

edited

Loading

eugenekasimov commented Apr 27, 2023 •

edited

Loading