-
-
Notifications
You must be signed in to change notification settings - Fork 696
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: Added simple swipe right/left #471
base: dev
Are you sure you want to change the base?
Conversation
…efactor refacto: remove duplicate
internal/glance/static/js/main.js
Outdated
|
||
const swipeThreshold = 50; | ||
|
||
if (Math.abs(touchendX - touchstartX) > Math.abs(touchendY - touchstartY) && Math.abs(touchendX - touchstartX) > swipeThreshold) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall good code but I think this if statement allows angles near 90º to trigger it, I suggest multiplying Math.abs(touchendY - touchstartY) by 2.
Also refactoring those 2 checks into variables is also a good Idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never played around with touch stuff before so I might be wrong how they work but my thinking was that since the length of x must be greater then y the angle has to be less then 45º which I thought was ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, the thing is that the code will trigger with a long swipe whose start and endpoints are in a cone of 90° from eachother. 60° makes more sense and maybe a time limit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I think I understand what you mean, we just have different ways to look at triangles 😃 But I added the Y*2 so we get a shallower angle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve of this
I like this. But it has issues with the navigation when you have a bunch of them and the horizontal videos, scrolling through them would also trigger the swipe. Edit: const excludedClass = [
'carousel-container',
'mobile-navigation-page-links',
// did I miss something?
];
document.addEventListener('touchend', function (event) {
let targetElement = event.target;
while (targetElement && targetElement.tagName !== 'IFRAME' && !excludedClass.some(c => targetElement.classList.contains(c))) {
targetElement = targetElement.parentElement;
}
if (targetElement) return;
// continue
}, false); |
Ok I added some more checks for when to trigger the left and right swipes, hope that helps. |
#128
Quick and easy solution to add swipe functionality for smartphones to change column.
The swipeThreshold maybe should be changeable from some setting but seems to work quite well overall as is.