Fix hovers and fix move unit duplicating state #72
Conversation
| for i, existing := range state.UnitsPlacement { | ||
| if existing.UnitID == placement.UnitID { | ||
| moved = true | ||
| state.UnitsPlacement[i] = placement |
There was a problem hiding this comment.
I don't see what has changed here. The outcome should be the same (?)
There was a problem hiding this comment.
Nope. There was a bug.
- Place unit1 on the gameboard (x = 1, y = 1)
- Place unit2 on the gameboard (x = 5, y = 5)
- With unit2 still selected, place it on (1, 1)
Then I expect the unitsPlacement state to be
unit2 x=1, y=1
But it was:
unit2 x=1, y=1
unit2 x=1, y=1
It's easy to see that the previous code never removed anything from the gameboard.
But in this case you had 2 units on the gameboard, and when you moved the one onto the other, the other should disappear, and be replaced.
| @@ -20,16 +20,14 @@ const BoardUnit: React.FunctionComponent<Props> = ({ unit }: Props) => { | |||
| onMouseEnter={() => setHover(true)} | |||
There was a problem hiding this comment.
Do we still need this if the trigger has been set to hover?
There was a problem hiding this comment.
Actually, yes! It does not really matter what the trigger is, I believe it's covered by some other element.
| @@ -11,6 +10,7 @@ interface Props { | |||
|
|
|||
| const BoardUnit: React.FunctionComponent<Props> = ({ unit }: Props) => { | |||
| const [hover, setHover] = useState<boolean>(false); | |||
| const [unitId] = useState<string>(unit.id); | |||
There was a problem hiding this comment.
Could you elaborate how these changes fix the hover being broken?
There was a problem hiding this comment.
The problem was that
- There is a unit with id=1 on board, and so with element id
#unit-1 - I change units on board
- The popover is trying to open itself but it has not yet gotten the new state (so it still thinks that it should show itself for the old id), but the DOM is already updated, so
#unit-1is not present there
When you change this to be the state, the component will re-render in proper order. So first, it gets the new state (from redux), than it changes its internal state, and then it updates itself.
There was a problem hiding this comment.
This gave me a clue: reactstrap/reactstrap#773
No description provided.