Fix double-taping on iPad to open popover in Quick Order List #2934

eugenekasimov · 2023-08-02T00:58:52Z

PR Summary:

This PR fixes the issue when you need to tap twice on info-icon on iPad(tablets) in order to open the popover.

Why are these changes introduced?

What approach did you take?

There was a conflict between two event listeners mouseenter and click that fired the same function same time. On iPad when you tap an element both events are fired, first mouseenter and then click.

We still need to keep them both because we have to cover cases for mouse and keyboard users and iPad and Desktop may have the same screen size.

I added a variable eventMouseEnterHappened to track a state of the mouseenter event. Now, if the mouseenter happens the click event won't be fired until eventMouseEnterHappened is reset.

Other considerations

Decision log

Visual impact on existing themes

Testing steps/scenarios

Add the quick order list section to the product page.
Login as a b2b customer and go to a product that has volume-pricing or qty rule.
Hovering mouse over info-icon test that popover element appears as expected.
Test the same on a tablet and make sure the popover element opens after first tap on the info-icon.

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

edmund-teh

This works beautifully on my iPad @eugenekasimov— great work!
Also, tested on my phone and on Desktop for any regressions... everything's looking solid. Thanks!

andrewetchen

This works great! Left some small feedback in case you decide to change the variable name.

andrewetchen · 2023-08-03T14:34:27Z

assets/quantity-popover.js

@@ -11,6 +11,7 @@ if (!customElements.get('quantity-popover')) {
        this.popoverInfo = this.querySelector('.quantity-popover__info');
        this.closeButton = this.querySelector('.button-close');
        this.variantInfo = this.querySelector('.quantity-popover-container');
+        this.eventMouseEnterHappened = false;


Small nitpick but maybe we can simplify this to just this.mouseEnterCompleted as it conveys that the variable is related to the completion of the mouseenter event.

sofiamatulis · 2023-08-03T18:30:51Z

assets/quantity-popover.js

@@ -37,6 +39,11 @@ if (!customElements.get('quantity-popover')) {

      togglePopover(event) {
        event.preventDefault();
+        if (event.type === 'mouseenter') {


Small suggestion: Maybe we can convert these to variables

this.actions = { mouseEnter: 'mouseenter', click: 'click' }

And refer to it as this.actions.click and this.actions.mouseEnter throughout the file

I don't see we use it a lot in the file, we use it only in one function. Am I getting you right that you suggest we do it this way? I'm not sure if it's worth it but I'm okay if everyone agrees.

Suggested change

if (event.type === 'mouseenter') {

this.actions = {

mouseEnter: 'mouseenter',

click: 'click'

}

if (event.type === this.actions.mouseEnter) {

this.eventMouseEnterHappened = true;

}

if (event.type === this.actions.click && this.eventMouseEnterHappened) return;

const button = this.infoButtonDesktop && this.mql.matches ? this.infoButtonDesktop : this.infoButtonMobile;

const isExpanded = button.getAttribute('aria-expanded') === 'true';

button.setAttribute('aria-expanded', !isExpanded);

this.popoverInfo.toggleAttribute('hidden');

const isOpen = button.getAttribute('aria-expanded') === 'true';

button.classList.toggle('quantity-popover__info-button--open');

if (isOpen && event.type !== this.actions.mouseEnter) {

button.focus();

}

That is fair! We can keep it as is, but if we repeat it more often throughout the file we can change that

sofiamatulis

Looks good! Added a small suggestion

* upstream/main: (205 commits) Fix for small screens with large fonts don't fit all content (Shopify#2946) Adjust quantity rules margin (Shopify#2948) Update Social media settings defaults to remove Shopify links (Shopify#2830) added json to barcode to pass gtin as a json string (Shopify#2804) Fixed extra margin spacing in collage section when header is empty (Shopify#2770) Track state of mouseenter event (Shopify#2934) Fix misalignment of total items in quick order list (Shopify#2923) Hide vol pricing and qty rules when variant is unavailable (Shopify#2889) Fix font family for quick order list (Shopify#2888) v11.0.0 version bump and release notes (Shopify#2916) Revert "v11.0.0 version bump and release notes (Shopify#2906)" (Shopify#2915) Update quantity-popover.css v11.0.0 version bump and release notes (Shopify#2906) Fix social list styles loading (Shopify#2900) Fix an error (Shopify#2903) Fix hardcoded info color (Shopify#2893) Fix error misalignment for Quick order list (Shopify#2887) Replace generic section name with section ID. (Shopify#2884) Fix cart drawer for variant list and tablet spacing (Shopify#2880) Add missing shadow styles to inputs in Quick Order List (Shopify#2879) ...

Track state of mouseenter event

7ddfd50

sofiamatulis requested review from sofiamatulis, edmund-teh and andrewetchen August 2, 2023 13:45

edmund-teh approved these changes Aug 2, 2023

View reviewed changes

andrewetchen approved these changes Aug 3, 2023

View reviewed changes

sofiamatulis reviewed Aug 3, 2023

View reviewed changes

sofiamatulis approved these changes Aug 3, 2023

View reviewed changes

eugenekasimov merged commit 7d02769 into main Aug 4, 2023

eugenekasimov deleted the fix-double-tap-quick-order-list branch August 4, 2023 14:57

phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024

Track state of mouseenter event (Shopify#2934)

1bbeedf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix double-taping on iPad to open popover in Quick Order List #2934

Fix double-taping on iPad to open popover in Quick Order List #2934

eugenekasimov commented Aug 2, 2023 •

edited

Loading

edmund-teh left a comment

andrewetchen left a comment

andrewetchen Aug 3, 2023

sofiamatulis Aug 3, 2023 •

edited

Loading

eugenekasimov Aug 3, 2023

sofiamatulis Aug 4, 2023

sofiamatulis left a comment

-        if (event.type === 'mouseenter') {
+       this.actions = {
+          mouseEnter: 'mouseenter',
+          click: 'click'
+        }
+        if (event.type === this.actions.mouseEnter) {
+          this.eventMouseEnterHappened = true;
+        }
+        if (event.type === this.actions.click && this.eventMouseEnterHappened) return;
+        const button = this.infoButtonDesktop && this.mql.matches ? this.infoButtonDesktop : this.infoButtonMobile;
+        const isExpanded = button.getAttribute('aria-expanded') === 'true';
+        button.setAttribute('aria-expanded', !isExpanded);
+        this.popoverInfo.toggleAttribute('hidden');
+        const isOpen = button.getAttribute('aria-expanded') === 'true';
+        button.classList.toggle('quantity-popover__info-button--open');
+        if (isOpen && event.type !== this.actions.mouseEnter) {
+          button.focus();
+        }

Fix double-taping on iPad to open popover in Quick Order List #2934

Fix double-taping on iPad to open popover in Quick Order List #2934

Conversation

eugenekasimov commented Aug 2, 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

edmund-teh left a comment

Choose a reason for hiding this comment

andrewetchen left a comment

Choose a reason for hiding this comment

andrewetchen Aug 3, 2023

Choose a reason for hiding this comment

sofiamatulis Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

eugenekasimov Aug 3, 2023

Choose a reason for hiding this comment

sofiamatulis Aug 4, 2023

Choose a reason for hiding this comment

sofiamatulis left a comment

Choose a reason for hiding this comment

eugenekasimov commented Aug 2, 2023 •

edited

Loading

sofiamatulis Aug 3, 2023 •

edited

Loading