feat: add onTappedSquare to Chessboard widget, add squareHighlights field #66
Conversation
We'll need this if we want to switch the coordinate trainer in the app from `ChessboardEditor` to `Chessboard` in order to fix lichess-org/mobile#1017
tom-anders
changed the title
lib/src/widgets/board.dart
Outdated
| @@ -549,6 +570,8 @@ class _BoardState extends State<Chessboard> { | |||
| final square = widget.offsetSquare(details.localPosition); | |||
| if (square == null) return; | |||
|
|
|||
| widget.onTappedSquare?.call(square); | |||
There was a problem hiding this comment.
Calling this onTapped is misleading since the callback is executed on pointer down.
There was a problem hiding this comment.
Hmm yeah I wasn't sure whether to put this into pointer down or up - Now that I think about it again, if we call it onTapped it should probably be in pointer up instead? And we should only invoke the callback if the pointer up happened on the same square as the previous pointer down event
There was a problem hiding this comment.
Or should we keep it in _onPointerDown and change the callback's name instead?
There was a problem hiding this comment.
It depends on what we want to do. Pointer down will feel more responsive compared to tap down. But, iirc we want to use it for the coordinates learn tool, right? I think on tap is more relevant here, what do you think?
In that case the best, and easiest way to implement it is to directly use the onTap handler of the GestureDetector.
There was a problem hiding this comment.
yeah it's for the coordinate trainer - added the GestureDetector 👍
| child: Stack( | ||
| alignment: Alignment.topLeft, | ||
| children: highlightedBackground, | ||
| final board = GestureDetector( |
There was a problem hiding this comment.
Oh I forgot that we weren't using a GestureDetector when I suggested to use it... 😄
What you wrote looks fine, but I wonder if there is any drawback to have the Listener as a child of a GestureDetector?
There was a problem hiding this comment.
Yeah that was the reason why I also initially hesitated to add it :D but I think with HitTestBehavior.translucent it shouldn't cause any problems, all the events will be passed along to the child, regardless of whether we handled them or not
There was a problem hiding this comment.
(and the tests still pass, so looks like at least there aren't any regression (can't promise no new bugs of course)
We'll need this if we want to switch the coordinate trainer in the app
from
ChessboardEditortoChessboardin order to fix lichess-org/mobile#1017