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

Adds drag and drop capability to 2d token #316

Merged
merged 13 commits into from
Jan 21, 2019
Merged

Adds drag and drop capability to 2d token #316

merged 13 commits into from
Jan 21, 2019

Conversation

vdfdev
Copy link
Contributor

@vdfdev vdfdev commented Dec 15, 2018

Checklist

  • Use a separate branch in your local repo (not master).
  • Test coverage is 100% (or you have a story for why it's ok).

@vdfdev
Copy link
Contributor Author

vdfdev commented Dec 15, 2018

FYI This decrease in coverage does not come from the files I changed:
image

@jasonharrison
Copy link
Contributor

@flamecoals is in-flight right now and his internet pass expired. He asked me to post this GIF demonstrating drag/drop:

GIF of chess drag and drop

@nicolodavis
Copy link
Member

For some reason this PR seems to be affecting the overall project coverage status (on master) as reported by Coveralls. Weird!

https://coveralls.io/github/nicolodavis/boardgame.io

@vdfdev
Copy link
Contributor Author

vdfdev commented Dec 18, 2018

For some reason this PR seems to be affecting the overall project coverage status (on master) as reported by Coveralls. Weird!

https://coveralls.io/github/nicolodavis/boardgame.io

How so? This is not even merged yet, very weird.
Maybe the badge is the minimum coverage of all branches? 3d seems to have only 89% coverage, thats probably why?

@nicolodavis
Copy link
Member

No, for some reason Coveralls thinks that this is a commit made on the master branch (it doesn't seem to distinguish between PR's and actual commits on master).

I'm not sure if it is Coveralls that's broken or the CI that's sending over incorrect branch data for PR's.

@nicolodavis
Copy link
Member

Some behavioral comments:

  1. Dragging a piece should cause it to appear on top of everything else (right now you can drag a white piece and it appears below the black).

  2. Dragging a piece outside the left or bottom sides of the board results in an error.

  3. Dragging a piece out and back into the original location should probably result in the same state prior to dragging (right now it still leaves the moves highlighted).

@vdfdev
Copy link
Contributor Author

vdfdev commented Jan 1, 2019

Some behavioral comments:

  1. Dragging a piece should cause it to appear on top of everything else (right now you can drag a white piece and it appears below the black).
  2. Dragging a piece outside the left or bottom sides of the board results in an error.
  3. Dragging a piece out and back into the original location should probably result in the same state prior to dragging (right now it still leaves the moves highlighted).

Ok, I will take a look into this later and get back to you.

@vdfdev
Copy link
Contributor Author

vdfdev commented Jan 13, 2019

@nicolodavis These issues should be addressed now. I also needed to add some logic to handle mobile better, which is so annoying, because touch screen behaves a little bit different than mouse. I am using this in my new game, and it works well in mobile with the right CSS to avoid scrolling.

Do you have any idea when you will merge the 3d branch into master? because I am having to maintain a separate fork in order to be able to use this in my project. Do you need help with anything in order to merge? I saw that there is a lot of untested code in the 3d folder.

@vdfdev
Copy link
Contributor Author

vdfdev commented Jan 13, 2019

The code I sent works in desktop, android but not iOs. I am changing it to work everywhere. It is very ugly but each environment behaves differently...

@vdfdev
Copy link
Contributor Author

vdfdev commented Jan 13, 2019

OK, the code now works on iOs, Android, Chrome desktop... I spent hours and hours trying to figure out a way to abstract away all this mess, and I think I succeeded :D

@nicolodavis
Copy link
Member

I'll try to merge the 3d branch in after this PR. There hasn't been much progress on the 3D components, so maybe I'll scrap it for the v1 release and add it as a nice to have for later.

@vdfdev
Copy link
Contributor Author

vdfdev commented Jan 13, 2019

FYI, I just found out that the click isnt working for Android. I will fix it soon... wait for merging

@vdfdev
Copy link
Contributor Author

vdfdev commented Jan 13, 2019

Ok, now it works on Android too :)

@nicolodavis nicolodavis mentioned this pull request Jan 18, 2019
4 tasks
@nicolodavis nicolodavis merged commit 230da1e into boardgameio:3d Jan 21, 2019
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.

3 participants