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

Fix issue no touch event on mobile #37234

Closed
wants to merge 14 commits into from
Closed

Fix issue no touch event on mobile #37234

Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0123048
Fix issue no touch event on mobile
mrtuvn Mar 17, 2023
5523e0a
Update logic check passive
mrtuvn Mar 24, 2023
e9b25b3
Update based on code reviews
mrtuvn Mar 24, 2023
904e1aa
Merge branch '2.4-develop' into patch-fotorama-event-touch
mrtuvn Apr 1, 2023
33b65c2
Merge branch '2.4-develop' into patch-fotorama-event-touch
engcom-Lima Apr 11, 2023
a3b75a2
Update third param pass as boolean just in case
mrtuvn Apr 15, 2023
0ed1b9d
Merge branch '2.4-develop' into patch-fotorama-event-touch
mrtuvn Apr 15, 2023
92d05b9
rework add test modernizr passive event listerners
mrtuvn Apr 18, 2023
e340108
Merge branch '2.4-develop' into patch-fotorama-event-touch
mrtuvn Apr 18, 2023
862a089
Update fotorama.js
mrtuvn Apr 19, 2023
fa7a9a4
leave passive with option only
mrtuvn Apr 22, 2023
0c73617
Update code
mrtuvn May 10, 2023
9f2e517
Update fotorama.js
mrtuvn May 10, 2023
d997cfc
Merge branch '2.4-develop' into patch-fotorama-event-touch
mrtuvn May 18, 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
49 changes: 34 additions & 15 deletions lib/web/fotorama/fotorama.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,19 @@ fotoramaVersion = '4.6.4';
return ret;
};

tests['passiveeventlisteners'] = function() {
var supportsPassiveOption = false;
try {
var opts = Object.defineProperty({}, 'passive', {
get: function() {
supportsPassiveOption = true;
}
});
window.addEventListener('test', null, opts);
} catch (e) {}
return supportsPassiveOption;
};


tests['csstransitions'] = function () {
return testPropsAll('transition');
Expand Down Expand Up @@ -659,8 +672,8 @@ fotoramaVersion = '4.6.4';

function readTransform(css, dir) {
return css.match(/ma/) && css.match(/-?\d+(?!d)/g)[css.match(/3d/) ?
(dir === 'vertical' ? 13 : 12) : (dir === 'vertical' ? 5 : 4)
]
(dir === 'vertical' ? 13 : 12) : (dir === 'vertical' ? 5 : 4)
]
}

function readPosition($el, dir) {
Expand Down Expand Up @@ -1140,7 +1153,13 @@ fotoramaVersion = '4.6.4';

function addEvent(el, e, fn, bool) {
if (!e) return;
el.addEventListener ? el.addEventListener(e, fn, !!bool) : el.attachEvent('on' + e, fn);

const options = (Modernizr.passiveeventlisteners) ? {
Copy link
Contributor Author

@mrtuvn mrtuvn May 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

just curiously do we actual need add passive for every dom element (or just add to body, window object) ? CC: @fredden
Can we add logic for check specific. Since browsers have updated a lot of changes and magento js system seem fall behind quite far

Copy link
Contributor Author

Choose a reason for hiding this comment

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

element window.document or window.document.body or window should be set passive false

Copy link
Member

Choose a reason for hiding this comment

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

Looking at this page on developer.mozilla.org, it seems that omitting 'passive' seems to be the best option as modern browsers* default the 'passive' option to a sensible value depending on the event.

* but not Safari. Safari seems to often lag when implementing features. For example, the 'loading=lazy' attribute for '<img>'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeap i think default passive enabled on safari and it's supported well
https://caniuse.com/passive-event-listener
only not support for retired IE browser

passive: e === 'wheel',
capture: !!bool,
} : !!bool;

el.addEventListener(e, fn, options);
}

/**
Expand Down Expand Up @@ -1519,7 +1538,7 @@ fotoramaVersion = '4.6.4';
addEvent(el, 'touchmove', onMove);
addEvent(el, 'touchend', onEnd);

addEvent(document, 'touchstart', onOtherStart, true);
addEvent(document, 'touchstart', onOtherStart);
addEvent(document, 'touchend', onOtherEnd);
addEvent(document, 'touchcancel', onOtherEnd);

Expand Down Expand Up @@ -2338,10 +2357,10 @@ fotoramaVersion = '4.6.4';

function loaded() {
$.Fotorama.measures[src] = imgData.measures = $.Fotorama.measures[src] || {
width: img.width,
height: img.height,
ratio: img.width / img.height
};
width: img.width,
height: img.height,
ratio: img.width / img.height
};

setMeasures(imgData.measures.width, imgData.measures.height, imgData.measures.ratio, index);

Expand All @@ -2356,7 +2375,7 @@ fotoramaVersion = '4.6.4';
}

fit($img, (
$.isFunction(specialMeasures) ? specialMeasures() : specialMeasures) || measures);
$.isFunction(specialMeasures) ? specialMeasures() : specialMeasures) || measures);

$.Fotorama.cache[src] = frameData.state = 'loaded';

Expand Down Expand Up @@ -2463,11 +2482,11 @@ fotoramaVersion = '4.6.4';
if (dataFrame.html) {
$('<div class="' + htmlClass + '"></div>')
.append(
dataFrame._html ? $(dataFrame.html)
.removeAttr('id')
.html(dataFrame._html) // Because of IE
: dataFrame.html
)
dataFrame._html ? $(dataFrame.html)
.removeAttr('id')
.html(dataFrame._html) // Because of IE
: dataFrame.html
)
.appendTo($frame);
}

Expand Down Expand Up @@ -2566,7 +2585,7 @@ fotoramaVersion = '4.6.4';
},
specialMeasures = getSpecialMeasures(),
exceedLimit = opts.navdir === 'vertical' ?
thisData.t > rightLimit : thisData.l > rightLimit;
thisData.t > rightLimit : thisData.l > rightLimit;
specialMeasures.w = thisData.w;

if ((opts.navdir !== 'vertical' && thisData.l + thisData.w < leftLimit)
Expand Down