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

Challenge from a Position with Fen #804

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

HaonRekcef
Copy link
Contributor

implements #797
closes #803
Adds the possibility to challenge another player with a fen:
Behavior with an invalid fen:

invalid.fen.mp4

Behavior with a valid fen:

valid.fen.mp4

Some remarks:

  • From position games can not be rated, so the button is disabled
  • The last position used to challenge is not stored (in my opinion this is not necessary)
  • Assumes that the fen checker is the same as employed on the serverside, otherwise it might lead to strange behavior.
  • Was not tested on iOS

@veloce
Copy link
Contributor

veloce commented Jul 1, 2024

Thanks for taking the time to create this! Multiple screen recording and remarks as you do are always very appreciated. I'll review it asap but for now I can say it looks very good.

@@ -56,6 +63,25 @@ class _ChallengeBody extends ConsumerStatefulWidget {

class _ChallengeBodyState extends ConsumerState<_ChallengeBody> {
Future<void>? _pendingCreateGame;
final _controller = TextEditingController();

String? textInput;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

textInput is too vague, prefer, for a local property, a long named describing exactly what it is representing.

@@ -332,15 +382,23 @@ class _ChallengeBodyState extends ConsumerState<_ChallengeBody> {
child: FatButton(
semanticsLabel: context.l10n.challengeChallengeToPlay,
onPressed: timeControl == ChallengeTimeControlType.clock
? isValidTimeControl
? isValidTimeControl &&
((textInput != '' && textInput != null) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use textInput.isNotEmpty instead of comparing with empty string.

@veloce veloce merged commit 75f6acf into lichess-org:main Jul 5, 2024
1 check passed
@HaonRekcef HaonRekcef deleted the from-position-challenge branch July 6, 2024 09:09
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.

Able to challenge with 0+0 leading to unrecoverable error
2 participants