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

Show target when dragging cards #30

Merged
merged 2 commits into from
May 25, 2016

Conversation

chrislaskey
Copy link
Contributor

Adds a target area when dragging cards between lists. Implemented using CSS styles to display the card's target location while dragging based on the monitor.isOver() functionality in React DnD.

screen shot 2016-05-24 at 8 43 42 pm

A CSS based solution like this can't handle all the edge cases, like dragging cards to an empty list.

But it does handle the primary one of dragging cards around within a list and between lists very well. It works well with the CSS transition and creates a nice overall effect.

This revealed two existing drag and drop behaviors which are unrelated to these changes:

  1. Picking up a card and dropping back in the same spot will cause the card to swap places with the one below it. In all other cases the behavior is for the current card to go above the card its hovering over.
  2. There's some consistent failures that occur when trying to update the card order, which you can see in the console when dragging around cards.

This PR doesn't attempt to tackle either of these issues.

Hope this code helps, this is a great learning project and really shows off the power of both Phoenix + Elixir and React + Redux!

Adds a target area when dragging cards between lists.

Implemented using CSS styles to display the card's target location while dragging.
@bigardone
Copy link
Owner

bigardone commented May 25, 2016

Hi @chrislaskey! Thank you very much for this PR, this was on top of my to-do list since the beginning, but I never had time to solve it. I love the result, great job!
Just a minor comment for the sake of consistency, do you mind renaming the isOver class name to is-over? I'm following the convention of naming classes using-dashes and id's using_underscores... I hope you don't mind!
Thank you very much again :)

@bigardone bigardone merged commit ecc9986 into bigardone:master May 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants