fix(datalist): use drag-over modifier instead of css style #5119
Conversation
|
PF4 preview: https://patternfly-react-pr-5119.surge.sh |
| @@ -326,12 +326,10 @@ export class DataList extends React.Component<DataListProps, DataListState> { | |||
| isCompact && styles.modifiers.compact, | |||
| gridBreakpointClasses[gridBreakpoint], | |||
| wrapModifier && styles.modifiers[wrapModifier], | |||
| dragging && styles.modifiers.dragOver, | |||
There was a problem hiding this comment.
This looks good to replace the existing behavior, but I think we need to change it so that styles.modifiers.dragOver is only applied while dragging, meaning it's removed once the item is dropped.
There was a problem hiding this comment.
It looks like the dragging state needs to be updated in the drag end handler, it is removed properly on a drag cancel. It should work with that updated.
There was a problem hiding this comment.
As @kmcfaul mentioned. dragging is a part of the DataList state which is being updated to true in dragStart0 and false in onDragCancel. These two are being called accordingly to the events when the user clicks and holds the element or releases the element.
There was a problem hiding this comment.
It should also be updated in dragEnd0 so the style gets removed when the drag is successfully completed.
|
@boaz It looks like there are outstanding comments here. |
|
@boaz0 this one still has outstanding comments. |
|
@tlabaj sorry, I thought I fixed it. I will work on it. |
Codecov Report
@@ Coverage Diff @@
## master #5119 +/- ##
==========================================
- Coverage 51.91% 51.90% -0.02%
==========================================
Files 579 579
Lines 11272 11275 +3
Branches 4184 4184
==========================================
Hits 5852 5852
- Misses 4683 4686 +3
Partials 737 737
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Looking at this draggable example, I'm still seeing |
|
Um, maybe I am missing something. But this is what I have on Firefox. movie.mp4 |
|
@boaz0 ah ok, I was testing in chrome - this is what I'm seeing Jan-21-2021.09-17-01.mp4 |
|
Thanks @mcoker I will look at this more. 😆 |
Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
|
@mcoker do you mind review this again. I hope this fixes the problem. |
What: Closes #5041
//cc @mcoker