willb335 / chessboardjsx

:black_square_button: Chessboard built for React
https://chessboardjsx.com
MIT License
267 stars 79 forks source link

Updating the position of only one piece outside `onDrop` doesn't update the UI #35

Closed slig closed 5 years ago

slig commented 5 years ago

Hi,

I've forked one demo here with this possible bug reproduced https://codesandbox.io/s/kxqxo8lwo3

One second after manually making a move (WithMoveValidation.js lines 80-89), it calls this.chess.undo() and updates the state. But somehow, as far as I could tell, getDerivedStateFromProps returns null to that change and now the internal state and the displayed board are different. (Try moving a white piece, waiting a second, and then moving another white piece)

Thank you very much and please let me know if you need any more details or a better bug report.

willb335 commented 5 years ago

Check out the fork of your demo below. I believe it was an async issue with setState. I changed

this.setState(...)
setTimeout(this.setState(...), 1000)

to

this.setState(..., () => setTimeout(this.setState(...), 1000) )

just put the setTimeout into the initial setState's callback.

let me know if this solves the issue https://codesandbox.io/s/2xpxvq06wj

slig commented 5 years ago

Hi!

Thank you for checking this issue.

On my end I'm still seeing the same problem. Opening the Dev Console, I see that it prints the fen correctly, but the UI stills shows the older state.

Here for instance I moved Kg2, and I'm hovering the f2 pawn:

screen shot 2018-10-09 at 09 08 34

Thank you very much!

willb335 commented 5 years ago

Ok, thanks

I'm looking into it now.

willb335 commented 5 years ago

going to have to figure this out later today. it's a bug with how the board his handling the prevState and currentState

willb335 commented 5 years ago

*prevPos and currentPos

willb335 commented 5 years ago

I'm going to expose a new prop called undo to fix this for now.

I wrote the chessboard to compare positions only if the current position is different than the previous position and when the current position becomes the previous position then there is no comparing.

I'll try to code up a demo sometime soon

willb335 commented 5 years ago

Check out this branch : https://github.com/willb335/chessboardjsx/tree/undo Run npm run start Click the Undo demo

In the demo I set all pieces except the knights to 'undo' by specifying an undo prop

Wish I could have integrated a fix without an extra prop but couldn't see an easy way without a substantial rewrite.

Let me know if this works!

slig commented 5 years ago

Thank you very much! Will try and get back to you.

slig commented 5 years ago

Just tested and this works. Thank you for working on this fix!

willb335 commented 5 years ago

great! i'll update the docs and push up a new release soon

willb335 commented 5 years ago

closed by: 2fff88a298bca4e8a67da9d4a3e25e9b1b4c7765