Add accept/decline buttons to game result dialog after rematch offer received #1153

Jimima · 2024-11-11T13:16:42Z

Just looking at this, seems straightforward to do but just wondering how to handle the UI. Initial implementation (not styled). UI suggestions welcome!

Relates to #1148

veloce · 2024-11-11T13:38:12Z

In some language "Rematch offered" can be a long text, so I'm not sure it would display nicely on one line.

We could instead have the "Rematch offered" on one line and below the centered buttons "Accept Decline".

Jimima · 2024-11-11T13:38:38Z

One thing I'm noticing here is that swapping out the button to offer a rematch might be problematic in the case that the user is about the offer a rematch and the UX changes under them. Not sure how big of a problem this is in reality

Jimima · 2024-11-11T13:39:43Z

Alternate approach

veloce · 2024-11-11T13:40:27Z

Good question, now sure either how to handle this properly. Perhaps it is not that much a problem.

Also, how about having the whole rematch offer block stand out with a light background color for instance?

Jimima · 2024-11-11T13:41:59Z

@veloce thanks, I am a bit worried if we add a label above the buttons it will cause a layout shift which could be annoying. I wonder how clear the latest approach is, the "Accept rematch" button implies one has been offered but it's maybe not clear. I think the design is clean, at least

veloce · 2024-11-11T13:44:43Z

@veloce thanks, I am a bit worried if we add a label above the buttons it will cause a layout shift which could be annoying. I wonder how clear the latest approach is, the "Accept rematch" button implies one has been offered but it's maybe not clear. I think the design is clean, at least

Layout shift can be handled by animating the change I think. Like a height change animation coupled with an opacity animation maybe?

veloce · 2024-11-11T13:45:20Z

Feel free to experiment with this and share screen recording of the UX that you find works well also.

Jimima · 2024-11-11T13:45:56Z

Sure I have not really worked with animations so I will have a play around, appreciate the feedback

veloce · 2024-11-11T13:53:16Z

Animation in flutter is not that hard, once you've chosen the right approach:

😆

Jimima · 2024-11-13T13:10:33Z

Here is an attempt at using animation, I think it works. In summary:

Remove rematch buttons when opponent offers rematch
Add (hidden) label to indicate the opponent has offered a rematch
Add (hidden) buttons to accent or decline
Animate dialog to fit new content (over 400ms)
Show label buttons once animation is complete

400ms based on gut feel and this

Simulator.Screen.Recording.-.iPhone.16.Plus.-.2024-11-13.at.13.17.40.mp4

Boubou78000 · 2024-11-13T13:18:38Z

I think it would be better without animation 😅 Or with a faster one

Jimima · 2024-11-13T13:21:47Z

It can be faster I thought it would be easier to show it a bit slower for review. Not much point going too fast without simply removing it and I think without it the layout shift could be annoying. Happy for someone to make the call, ultimately, just trying things out at the moment

Boubou78000 · 2024-11-13T13:24:35Z

Alternate approach

This one is good 👍

veloce · 2024-11-14T09:47:27Z

As suggested by @Boubou78000 your alternate approach is better to avoid the layout shift (not showing the "Your opponent offered a rematch").

We'd still use a FadeTransition (with a duration ~300ms I'd say) when the widgets change.

It should be pretty obvious that the opponent offers a rematch, seeing the labels "Accept rematch", "Decline".

Can you post a video showing all the states:

offering a rematch
cancel rematch offer
opponent asking for rematch

Thanks!

Jimima · 2024-11-14T10:31:00Z

Yeah I'm a little concerned that the buttons alone are not enough but happy to go with that for now. I suspect there will be feedback if it's not clear! Will make the changes, no problem

Jimima · 2024-11-14T14:11:35Z

Changes as discussed. Video:

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-11-14.at.13.41.28.mp4

Boubou78000 · 2024-11-14T15:42:45Z

@Jimima there is still the rematch button when clicked it? Should it not just show a cancel rematch button, not both? Weird 😅

…imima/lichess-mobile into 1148-accept-or-decline-rematch-request

Jimima · 2024-11-14T16:29:18Z

Oops, yeah that's just a bug. I should have noticed that. Should be better now

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-11-14.at.16.20.28.mp4

Boubou78000 · 2024-11-14T18:21:17Z

Oops, yeah that's just a bug. I should have noticed that. Should be better now

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-11-14.at.16.20.28.mp4

Nice 👍

I just noticed another small thing: the rematch and the cancel rematch buttons are the same design, so MAYBE add a box or anything, just to see a visual change 🤔

Othetwise I think this PR is ready to merge 😃

Jimima · 2024-11-19T14:19:47Z

@veloce I thought I had the commit but apparently not, IDE local history had it though :-)

I have pushed up the animated version.

I had a quick look at how I might keep the rematch button but disable it and I couldn't work out how the enabled/disabled state was being managed. Can you enlighten me?

Jimima · 2024-11-19T16:54:29Z

Actually never mind, I figured it out I think. Here is a version as requested (I think)

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-11-19.at.16.48.27.mp4

veloce · 2024-11-19T17:07:30Z

Yeah I think it's better like that! Thank you. Looks like the lint is failing.

Jimima · 2024-11-19T17:16:17Z

It is. I have a bit of tidying up to do anyway so I will do that and let you know

tom-anders · 2024-11-19T17:40:11Z

@veloce I thought I had the commit but apparently not, IDE local history had it though :-)

I have pushed up the animated version.

I had a quick look at how I might keep the rematch button but disable it and I couldn't work out how the enabled/disabled state was being managed. Can you enlighten me?

FYI for next time, in case your IDE does not have the code in its undo history: you can recover "lost" commits (they're not actually lost, just unreachable) via git reflog

Boubou78000 · 2024-11-19T18:52:16Z

Actually never mind, I figured it out I think. Here is a version as requested (I think)

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-11-19.at.16.48.27.mp4

I think there is a small padding problem between the rematch button and the accept rematch one... But I'm not sure...

…imima/lichess-mobile into 1148-accept-or-decline-rematch-request

Jimima · 2024-11-19T21:12:41Z

Latest (possibly final 🙂) iteration

Edit: I should probably do a bit more testing of edge cases but here it is in the meantime

Screen.Recording.2024-11-19.at.21.00.05.mov

Boubou78000 · 2024-11-19T21:18:44Z

Latest (possibly final 🙂) iteration

Edit: I should probably do a bit more testing of edge cases but here it is in the meantime

Screen.Recording.2024-11-19.at.21.00.05.mov

Again, it looks like there is extra space above the rematch button...

Jimima · 2024-11-19T21:25:02Z

You're right I'll take a look at that tomorrow

Jimima · 2024-11-19T22:25:56Z

veloce · 2024-11-20T10:00:44Z

lib/src/view/game/game_result_dialog.dart

@@ -89,6 +89,7 @@ class _GameEndDialogState extends ConsumerState<GameResultDialog> {
  Widget build(BuildContext context) {
    final ctrlProvider = gameControllerProvider(widget.id);
    final gameState = ref.watch(ctrlProvider).requireValue;
+    final ValueNotifier<bool> animationFinished = ValueNotifier(false);


A ValueNotifier should not be declared in build method I think. If you do that you cannot properly dispose it.

Rather than using ValueNotifier and ValueListenableBuilder you can directly declare a bool in the state object and use setState in onEnd.

veloce · 2024-11-20T10:12:16Z

lib/src/view/game/game_result_dialog.dart

+                visible: animationFinished.value,
+                child: Column(
+                  children: [
+                    if (gameState.game.opponent?.offeringRematch == true)


Instead of returning an empty column if gameState.game.opponent?.offeringRematch is false, you could use a ternary operator to return either the column or a Sizebox.shrink() if the opponent is not offering rematch.

veloce · 2024-11-20T10:23:21Z

lib/src/view/game/game_result_dialog.dart

@@ -98,6 +99,62 @@ class _GameEndDialogState extends ConsumerState<GameResultDialog> {
          padding: const EdgeInsets.only(bottom: 16.0),
          child: GameResult(game: gameState.game),
        ),
+        AnimatedSize(


Instead of using AnimatedSize with Visibility have you tried AnimatedCrossfade? This is higher level and the result may be better since it animates size and opacity.

AnimatedCrossFade( duration: const Duration(milliseconds: 400), firstChild: Sizedbox.shrink(), secondChild: RematchOfferDialog(...), crossFadeState: gameState.game.opponent?.offeringRematch == true ? CrossFadeState.showSecond : CrossFadeState.showFirst, )

I wasn't aware of this, will take a look!

Jimima · 2024-11-20T12:03:28Z

@veloce I think I addressed your comments. The AnimatedCrossfade works well. I used the curves to somewhat mimic the animation I had before but it's a little nicer, much nicer in code. You can play around with the curves to your hearts content. I think what I have is good but you (or anyone) might prefer a slightly different UX when fading in/out

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-11-20.at.11.53.20.mp4

Boubou78000 · 2024-11-20T12:46:25Z

Again, there is extra space above the rematch button, maybe because of the box around accept rematch?

I mean that there is not the same distance between the accept rematch button (text) and the rematch button and between the rematch button the new opponent button...

Jimima · 2024-11-20T13:09:25Z

I added some padding because otherwise the layout felt cramped. This looks well spaced to my eyes

Boubou78000 · 2024-11-20T13:15:51Z

Ok. Maybe keep padding, but less?

veloce

Looks great! Thanks @Jimima

Boubou78000 · 2024-11-27T13:19:52Z

@Jimima On android, the accept rematch and decline buttons aren’t good spaced: there is no horizontal padding...

Jimima added 2 commits November 11, 2024 13:11

Initial implimentation

30eec0e

Alternate implimentation

3e0c643

Fix formatting

d5b88bc

Jimima force-pushed the 1148-accept-or-decline-rematch-request branch from bffa752 to eec9d65 Compare November 13, 2024 15:23

Jimima marked this pull request as ready for review November 14, 2024 09:39

Jimima added 2 commits November 14, 2024 13:44

Back in time

4ac1342

Fade in buttons, no label

b0d9fc5

Jimima force-pushed the 1148-accept-or-decline-rematch-request branch from f54c9ed to b0d9fc5 Compare November 14, 2024 14:08

Merge branch 'main' into 1148-accept-or-decline-rematch-request

99909e4

Jimima added 2 commits November 14, 2024 16:21

Fixed persistand rematch button bug

4050090

Merge branch '1148-accept-or-decline-rematch-request' of github.com:J…

2e816a4

…imima/lichess-mobile into 1148-accept-or-decline-rematch-request

Tweaked layout

fbc105f

Version with persisted-but-disabled rematch button

734acfd

Jimima added 4 commits November 19, 2024 21:01

Improved layout and logic

a0f325a

Merge branch 'main' into 1148-accept-or-decline-rematch-request

180fb02

Removed accidental old(?) code

f2c144b

Merge branch '1148-accept-or-decline-rematch-request' of github.com:J…

0bb66ef

…imima/lichess-mobile into 1148-accept-or-decline-rematch-request

Removed unnecessary padding

8159bce

veloce reviewed Nov 20, 2024

View reviewed changes

Jimima added 2 commits November 20, 2024 11:00

Improved rendering logic

132664d

Improved animation

cff7251

Jimima force-pushed the 1148-accept-or-decline-rematch-request branch from 8fd87d8 to cff7251 Compare November 20, 2024 12:01

Merge branch 'main' into 1148-accept-or-decline-rematch-request

a86d7b0

veloce approved these changes Nov 20, 2024

View reviewed changes

veloce merged commit 28cb95f into lichess-org:main Nov 20, 2024
1 check passed

Jimima mentioned this pull request Nov 21, 2024

Ability to decline a rematch #1148

Closed

Add accept/decline buttons to game result dialog after rematch offer received #1153

Add accept/decline buttons to game result dialog after rematch offer received #1153

Conversation

Jimima commented Nov 11, 2024

veloce commented Nov 11, 2024

Jimima commented Nov 11, 2024

Jimima commented Nov 11, 2024

veloce commented Nov 11, 2024

Jimima commented Nov 11, 2024

veloce commented Nov 11, 2024

veloce commented Nov 11, 2024

Jimima commented Nov 11, 2024

veloce commented Nov 11, 2024

Jimima commented Nov 13, 2024 • edited Loading

Boubou78000 commented Nov 13, 2024 • edited Loading

Jimima commented Nov 13, 2024 • edited Loading

Boubou78000 commented Nov 13, 2024

veloce commented Nov 14, 2024 • edited Loading

Jimima commented Nov 14, 2024

Jimima commented Nov 14, 2024

Boubou78000 commented Nov 14, 2024

Jimima commented Nov 14, 2024

Boubou78000 commented Nov 14, 2024

Jimima commented Nov 19, 2024

Jimima commented Nov 19, 2024

veloce commented Nov 19, 2024 • edited Loading

Jimima commented Nov 19, 2024

tom-anders commented Nov 19, 2024

Boubou78000 commented Nov 19, 2024

Jimima commented Nov 19, 2024 • edited Loading

Boubou78000 commented Nov 19, 2024

Jimima commented Nov 19, 2024

Jimima commented Nov 19, 2024

veloce Nov 20, 2024

Choose a reason for hiding this comment

veloce Nov 20, 2024

Choose a reason for hiding this comment

veloce Nov 20, 2024

Choose a reason for hiding this comment

Jimima Nov 20, 2024

Choose a reason for hiding this comment

Jimima commented Nov 20, 2024

Boubou78000 commented Nov 20, 2024 • edited Loading

Jimima commented Nov 20, 2024

Boubou78000 commented Nov 20, 2024

veloce left a comment

Choose a reason for hiding this comment

Boubou78000 commented Nov 27, 2024 • edited Loading

Jimima commented Nov 13, 2024 •

edited

Loading

Boubou78000 commented Nov 13, 2024 •

edited

Loading

Jimima commented Nov 13, 2024 •

edited

Loading

veloce commented Nov 14, 2024 •

edited

Loading

veloce commented Nov 19, 2024 •

edited

Loading

Jimima commented Nov 19, 2024 •

edited

Loading

Boubou78000 commented Nov 20, 2024 •

edited

Loading

Boubou78000 commented Nov 27, 2024 •

edited

Loading