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.

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

Draggable with margins lands in the wrong place, sometimes #785

Closed
abettadapur opened this issue Sep 5, 2018 · 9 comments
Closed

Draggable with margins lands in the wrong place, sometimes #785

abettadapur opened this issue Sep 5, 2018 · 9 comments

Comments

@abettadapur
Copy link

abettadapur commented Sep 5, 2018

First, fantastic library, so far a real joy to use. Thanks for contributing this!

Bug or feature request?

Bug

Description

I have margins on my Draggable elements to space them apart.

image

With these margins set, when I drop a card, I sometimes see cards move to the wrong place. It is only for specific Droppable components. See the gif below. Notice that the second cell always places it in the correct place, but the others do not

beautiful-dnd

I thought this might be margin collapsing, but I was further confused by the fact that the card seems to ignore the horizontal margins as well

I was able to fix this behavior by making the Draggable have no margins and instead have an inner div with the margins.

To me this seems like unexpected behavior, or at least something that could be documented. What are your thoughts?

Expected behavior

The Draggable should be aware of its margins and move appropriately

What version of React are you using?

16.4

What version of react-beautiful-dnd are you running?

Latest (9.0.2)

@alexreardon
Copy link
Collaborator

alexreardon commented Sep 6, 2018

Your example looks great! It looks like our drop position is not correctly considering margins when moving into the last position of a foreign list. I can try to look a bit further into this one

@abettadapur
Copy link
Author

Thanks @alexreardon, let me know if you need more from my end

@alexreardon
Copy link
Collaborator

An independent example would be very helpful. Here is our boilerplate:

https://codesandbox.io/s/k260nyxq9v

@alexreardon
Copy link
Collaborator

I think I have the right line, but it would be good to get an example to verify

@abettadapur
Copy link
Author

Sure, I'll try to put one together for you tomorrow

@abettadapur
Copy link
Author

abettadapur commented Sep 6, 2018

@alexreardon, reliable repro. Your suspicion was right, it only occurs when the item is dropped at the end of the list
https://codesandbox.io/s/7jmqpljpq1

dnd

@alexreardon
Copy link
Collaborator

This led me down quite the rabbit hole 🐇

I was already wrestling with this area of the code for #511. As a result of that work, and this bug - I have completely changed how we do positioning with keyboard. My work fixes the issue you are seeing.

It will be shipping as a part of #719 in version 10.

Thanks so much for raising this.

Here is the fix in action:

margin-fix

@abettadapur
Copy link
Author

Fantastic @alexreardon, thanks so much for the quick turnaround. Glad I could help :)

@alexreardon
Copy link
Collaborator

@abettadapur please give version 10 a go and see if your issues are resolved #838

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants