clauderic / react-sortable-hoc

A set of higher-order components to turn any list into an animated, accessible and touch-friendly sortable list✌️
https://clauderic.github.io/react-sortable-hoc/
MIT License
10.8k stars 980 forks source link

Unable to set CSS cursor on sortable helper #212

Open rosston opened 7 years ago

rosston commented 7 years ago

Any CSS cursor property is ignored on the sortable helper, and the cursor on the screen is determined by the background content that the helper is currently being dragged over (this part of the bug is actually the weirdest to me). Example here: https://jsfiddle.net/pufcv77k/3/

The problem appears to have been introduced here: https://github.com/clauderic/react-sortable-hoc/pull/160. It looks like that change fixes scrolling-while-dragging only when the sortable items are in a container with overflow: auto;, but perhaps I'm missing another scenario?

Not sure what the fix is here, if anything. I'd prefer not to have to set pointer-events: auto !important; to allow a custom cursor (and quash the weird background cursor behavior), but I'm not sure which behavior is preferred as default.

konutis commented 7 years ago

Your helper doesn't even get className - SortableHelperWithOverride. You have set helper className - (overridePointerEvents) ? 'SortableHelperWithOverride' : 'SortableHelper'. Seems like overridePointerEvents is false. If You set className to helper just - 'SortableHelperWithOverride', it works.

rosston commented 7 years ago

@konutis There are two sortable lists. The top one does not have the override; the bottom one does (line 47 in the JS). The sortable helper in the bottom one is in fact getting the SortableHelperWithOverride class, and does work, as you suggest it would.

The point of this issue is to ask: should pointer-events: auto !important; really be necessary on every helper where I want 1) non-leaking cursor behavior and 2) the ability to set a custom cursor?

MtDalPizzol commented 7 years ago

Plus one on this. Can't get cursor: grabbing; to work...

rosston commented 7 years ago

@MtDalPizzol Just in case you didn't see it, take a look at my example code (https://jsfiddle.net/pufcv77k/3/) for a workaround. Set a helperClass (line 33 in my JS) to whatever you want, and then set pointer-events: auto !important; on that class (lines 15-17 in my CSS). It's not pretty, but it works. I'm using this workaround to set cursor: grabbing; in my day job.

MtDalPizzol commented 7 years ago

Hey, @rosston ! Thanks! It worked... I haven't opened the fiddle because when I read your post I had the impression you were only reproducing the problem, not presenting a possible solution... Anyway.... thanks again! This solution is pretty enough for me... nothing to crazy...

bmueller-sykes commented 7 years ago

@rosston A note and a question: first, it appears your trick does not work in Safari, FWIW. Maybe Safari doesn't support pointer events yet? Also, more importantly, I don't understand how to set cursor:grabbing in this context. I would have thought, using your JSFiddle example, that putting cursor:grabbing on .SortableHelperWithOverride in the CSS would do the trick, but that didn't work. I'm trying this:

.dragging {
  pointer-events:auto !important;
  cursor:grabbing !important;
}

...and I get very spotty behavior in Chrome. Sometimes, I'll see the grabbing cursor for a split second, and then it'll revert back to the cursor I get when hovering over the draggable item. Other times (though less often), I'll see the grabbing cursor for the duration of the drag and drop operation.

rosston commented 7 years ago

@bmueller-sykes You're right that it (mostly) doesn't work in Safari (at least 11). I can't remember if it worked fully in Safari 10.1, but I don't have it handy now. What it does do in Safari is prevent a text selection from happening on the background elements. And FWIW, Safari does support pointer-events, just apparently not in the same way as other browsers. 🙂

As for Chrome, is it possible you just need to use -webkit-grabbing instead? MDN shows it as only supporting the prefix. Other than that, I would think that your code would make it work correctly.

bmueller-sykes commented 7 years ago

Thanks for the reply! This does appear to make things more reliable:

.grabbing {
  pointer-events: auto !important;
  cursor: grabbing !important;
  cursor: -moz-grabbing !important;
  cursor: -webkit-grabbing !important;
}

...but it's not perfect. It looks like that css command is competing with some other css cursor commands I've got within the dragging item. I don't know if that's a fundamental problem, or just something unique to my code.

mcroba commented 6 years ago

@rosston I have the same issue and I have found the same workaround with pointer-events: auto !important but it doesn't work with Firefox whereas the example of the documentation does.

here is my fiddle but I have the same with yours.

Do you or @clauderic have any idea?

Thanks in advance for your response EDIT

Actually I have found a way to make it work with Chrome and Firefox without changing the pointer-events but by setting the cursor on the container and the (sortable) items when dragging.

I'm adding a className on the container in onSortStart and remove it in onSortEnd

you can see it in my fiddle. It still doesn't work in Safari unfortunately

I hope it can help

dagadbm commented 6 years ago

so does anyone know of a way to make this work with safari?

All of the solutions here fail in Safari. I don't even know why :(

dagadbm commented 6 years ago

also how does the react-beautiful-dnd from atlassian manage to make this work on safari I wonder. I tried looking for the styling but I'm not sure what the main difference is honesty.

dagadbm commented 6 years ago

https://github.com/clauderic/react-sortable-hoc/issues/253

this helps on this issue as well (but Safari still doesn't work properly)

zalevsk1y commented 5 years ago

I will describe how I solved this problem. I have created div wrapper and used the helperClass prop to specify an additional class for the dragging container. For example .drag-container . In css properties:

.dragging-container{
pointer-events: auto !important;
cursor:move;
}

and for inner

tag set pointer-events to none, to prevent that element become target of pointer events:

.dragging-container div{
pointer-events: none !important;
}