-
Notifications
You must be signed in to change notification settings - Fork 404
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
Dragging and link issue #71
Comments
Is there another solution to this? |
@pawelgrzybek The solution is to split the space of the slider item wrapper into two parts. One should react to swiping the other to click. In practice:
|
Hi all. There are 2 possible solutions:
I'll look at the possible solutions and decide about it later. Thanks for surrestions @WhereJuly and reporting @armantaherian |
@pawelgrzybek thank you for the Siema 👍 Shortly I will add the pen to show how to get the both (or even more) behaviours at the same time. Meanwhile I spent good three hours trying to separate the However I tried to set my own event listener above or at the same element as Siema item wrapper. However I tried to catch the click on bubbling or captturing phase I could not make to separate it from Siema's |
Here is the pen combining 3 behaviors on each Siema slide:
One can combine as many behavors as required not sacrificing the main slider's functionality. |
Hi all. @armantaherian, @yidemir, @WhereJuly I have made a simple workaround. |
@pawelgrzybek please respond immediately sir. i have to push an update into prod by saturday night or its my a$$... i bet the farm on siema(great project btw, good job man) and now i've ran into this bug. will you accept a pr????? like today???? |
version of @matass workaround that should work with multiple sliders and slides appended at anytime ('assuming they all are named like "siema-") var dragging, href, el;
dragging = false;
href = '';
$(document).on('mousedown','[class*="siema-"] a',function() {
el = $(this);
dragging = false;
href = $(this).attr('href');
})
$(document).on('mousemove','[class*="siema-"] a',function() {
dragging = true;
})
$(document).on('mouseup','[class*="siema-"] a',function() {
var wasDragging;
wasDragging = dragging;
dragging = false;
if (wasDragging) {
$(this).attr('href', null);
setTimeout((function() {
el.attr('href', href);
}), 10);
}
}); |
I won't accept any PR today for sure. This is going to be fixed in 1.5 and I am planing to release it very soon (but not today or tomorrow). Sorry. |
Hi! Thanks for Siema, has become part of my favourite lightweight libraries. I'm interested in this too, if you try to implement it into a product slider shop, it's important to handle the click event in order to work only when the slider is not moving. Awesome work pawelgrzybek ;) |
+1 for this feature. Looking forward to 1.5! |
Hi guys. You were waiting long enough, sorry about it :) And this is a CodePen link with a presented solution. Hopefully it is what you expected. Have a lovely day you all 🥑 |
Awesome! Thank you man ;) |
I'm testing it in this widget: Why is it behaving like in the previous version? Siema is updated and is taking users to the link even if they are dragging. |
I see what is going on. You are not dragging a link. You have tons of markup inside a link ( https://github.com/pawelgrzybek/siema/blob/master/src/siema.js#L425-L427 You can ignore pointer events on https://codepen.io/pawelgrzybek/pen/aEqpMO Hopefully it makes sense for you :) |
Ok! Let me try and I tell you. Thank you so much, I didn't think about that fact. |
I tried a codepen from #71 (comment) and it still redirects user to the link, even when you drag it and slide gets changed. |
@aczekajski, have you tried this pen – https://codepen.io/anon/pen/NwPjPa ? |
I've updated the link to siema library in the codepen to current minified file in master branch and now it cannot be clicked at all: You can drag it but you cannot navigate to the link. It's the same with 1.4.10. (tested in Chrome 63) |
Hi @aczekajski. Just tested your CodePen on 2 machines. On iOS and Windows. Everything is working as expected for me. Just for the sake of it I tested it on IE11 — works like a charm. What OS are you on, Do you use some custom input devices that may trick the order of event invocations? |
It sounds strange but it started working after restarting Chrome... However, I've found a scenario where first click doesn't work. If you drag the slide but the drag ends outside the Siema main wrapper. After that, first click doesn't navigate, subsequent clicks do. |
I see what is going on. There isa prop Thanks for involvement, and I'll address next fix to this scenario. |
By the way this logic behind preventing click event sounds really fishy for me. Why only preventing navigate when the target is a link? There are tons of scenarios where someone might want to have something clickable on the slide, but not neccesarily a link. Now I'm only fiddling with the code but I believe it might be cool if there was an option to prevent events when the slider was dragged. For example if I did if (this.drag.preventClick) {
e.stopPropagation();
e.preventDefault();
} inside this.selector.addEventListener('click', this.clickHandler, true); // useCapture parameter is changed to "true" then I can prevent the event handler provided by the user from firing. But it of course needs some more testing since changing You can see the work-in-progress version with the effect here: |
Hi. It is not a true that click is prevented only for links. It checks for links, textareas, options, inputs and select boxes. Your solution with conditional event firing and events capturing will break when you work with nested carousels (wich is fully tested and well supported back to IE10). By the way, your issue with prevented click when dragging left the selector has been fixed: 634ab1c Thanks 🥑 |
Quick comment to your CodePen: a.addEventListener('click', () => console.log('klik a'));
b.addEventListener('click', () => console.log('klik b'));
c.addEventListener('click', () => console.log('klik c'));
const mySiema = new Siema();
// when you drag, it won't trigger event listeners for a, b, c elements This is absolutely expected and intentional. On |
Please comapre behaviour: Which behaviour is more expected? For me, it is the latter, where |
According to #71 (comment) - is there a codepen with example of nested carousels so I can test it? |
@pawelgrzybek ping ;) |
I'm having this issue when wrapping images inside of anchors, any solution to this? EDIT: Seems like it happened when the slider was wrapped in a floated div. |
Up! |
@pawelgrzybek this still seems to break as soon as there is another tag inside the anchor tag. |
Hi @rafaelderolez. I am aware of it and this issue is going to be addressed in next release. Thanks a lot for reporting and using Siema :) |
I still had the same problem with v1.5.1. I had to bind a link on the click event...
And at the beginning of the binding:
|
I also had some problems with this ended up writing this code (might not work in all cases though) and added it to the onInit function: Siema.prototype.preventLinkClick = function (selector) {
const CONTAINER = this.selector.parentNode;
const LINKS = CONTAINER.querySelectorAll(selector);
LINKS.forEach(el => {
let firstMouseX = 0;
el.addEventListener('mousedown', (e) => firstMouseX = e.clientX);
el.addEventListener('click', (e) => {
let lastMouseX = e.clientX;
let diffMouseX = firstMouseX - lastMouseX;
if (diffMouseX > 1 || diffMouseX < -1) {
e.preventDefault();
}
});
});
} new Siema({
onInit() {
this.preventLinkClick('.mtt-post-slider__slide > a');
}
}); |
I made this removing HREF from A if the mousediff is bigger than 3 Did some changes to work on IE11 too:
Finnally! :D |
PS: i believe your functions isnt really working for you |
It's working fine in all cases I've tested it, you shouldn't need to anything else than prevent the default behavior of the link. |
Is this solved? |
@vielhuber I don't maintain this lib anymore. I consider it an anti-pattern. Feel free to resolve it yourself and I am more than happy to accept you PR. Thanks |
I resolved this issue with:
On my links inside carousel panels. |
Worked for me |
Just drag here:
https://s.codepen.io/armantaherian/debug/eEgRGM/nqMwvedyKpXk
You will see the issue, I’m dragging, but the page will go to Google.
Codepen: https://codepen.io/armantaherian/pen/eEgRGM
The text was updated successfully, but these errors were encountered: