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

feat: add detail view to game history screen #1057

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

tom-anders
Copy link
Contributor

Had this lying around for a while, got the time today to include the feedback I got on the original draft I posted in the issue. Closes #947

Screenshot_1727612203

@ijm8710
Copy link

ijm8710 commented Sep 29, 2024

Hmm excited for the accuracy number to have a way back in but I prefer your old draft in some other places

I still think the actual variant/time should show and so I preferred the time/game type display of before

And the opponent name seems bigger? Especially if you try to show rating change the huge name with just the opponent and not yourself probably makes that a harder obstacle?

@veloce
Copy link
Contributor

veloce commented Sep 30, 2024

It looks quite nice!! thank you. I'd see the accuracy in even smaller font and a little more transparent. I know some people here really want to see it, but it is least important item here, so should have less emphasis while still being here.

Looks like you also implemented a grid view, judging by the icon. Can we see the screenshot for it as well please?

@tom-anders
Copy link
Contributor Author

It looks quite nice!! thank you. I'd see the accuracy in even smaller font and a little more transparent. I know some people here really want to see it, but it is least important item here, so should have less emphasis while still being here.

Thanks, will take a look

Looks like you also implemented a grid view, judging by the icon. Can we see the screenshot for it as well please?

Oh, that's a bug actually, it should show a different icon. Pressing that icon/button will switch the view to the "old" list view

@veloce
Copy link
Contributor

veloce commented Oct 1, 2024

Oh, I thought it would be a grid view like in the puzzle history. Could make sense by the way, it would show more games at once. But not needed in this PR I guess.

I'm not sure about having the switch icon to see the old list. We probably should display the new list only (perhaps keeping the old list preview on home page only).

@tom-anders
Copy link
Contributor Author

Oh, I thought it would be a grid view like in the puzzle history. Could make sense by the way, it would show more games at once. But not needed in this PR I guess.

I'm not sure about having the switch icon to see the old list. We probably should display the new list only (perhaps keeping the old list preview on home page only).

Will do 👍

Yeah I thought the old list is more compact and doesn't take as much space, so I kept it on the home page.

@julien4215
Copy link
Contributor

Would it make sense to keep both views ? Since the list view is more compact, it can be faster to find a game that is far in the history and you can also glance more games at once.

@veloce
Copy link
Contributor

veloce commented Oct 1, 2024

I don't know, need to think about it. I don't like adding too much toggles and settings. We're fetching the same number of games per page, so it is just a matter of vertical space. If the new view allows to quickly see the important information (player name, end position), it could be that the new view would allow to find even more quickly the games.

@ijm8710
Copy link

ijm8710 commented Oct 1, 2024

Just a minor example (but one of a few) is it like on homepage currently when scanning an opponents games you can see a day ago, 3 days ago etc.

But when you dive into this detailed view its time stamped.

If all views including the first 10 on homepage were just timestamp you have to think more to be like is October 2nd a day ago, few days ago etc

Actually another idea here:
I prefer if it said 20 hours ago etc (but clicking that turns it into the actual time)!

I also think the compact current view does a better job to quickly scan wins and losses since it's not caught in the clutter.
And it's easier to quickly see if a game has computer analysis pulled yet.

I still think there is a bit more refinement that can be done before this becomes the official game list.
Still hoping we can see rating change
And I think it's worthwhile info to show what the variant was (ie 5+0)

@tom-anders
Copy link
Contributor Author

A note about the time stamps: I agree that "3 days ago" is better than a full date, but for longer times I actually prefer the full date. "2023-07-24" is more helpful than "3 months" ago IMO. I think there should be some kind of cut off where we switch from "x ago" to the full date.

@tom-anders
Copy link
Contributor Author

Also, I really like how the old app says "Last Sunday" instead of "2 days ago", I think this is more helpful as well, but probably something for another PR

@ijm8710
Copy link

ijm8710 commented Oct 1, 2024

What are your thoughts Tom on relative date that can be tapped to Change to actual date or with a popup.

I could have sworn GitHub used to do this. I think I've seen this with a few other apps.

@tom-anders
Copy link
Contributor Author

What are your thoughts Tom on relative date that can be tapped to Change to actual date or with a popup.

I could have sworn GitHub used to do this. I think I've seen this with a few other apps.

idk, might be too much complexity, but not for me to decide

@tom-anders
Copy link
Contributor Author

Did some style adjustments, removed the old list view, added rating diffs:

Screenshot_1727816559

@ijm8710
Copy link

ijm8710 commented Oct 2, 2024

Few thoughts,

Thanks for adding rating diff. I think the way opponent and your usernames now show does look better now.

For draw where rating does not change, I think it looks better in old app (rather than show +/- 0 which looks funky, they just omit the diff)

I also will push back once on Veloce regarding accuracy being even more downsized. You said yourself Veloce, a lot of players (including myself) care about it a lot. To be honest, I sometimes even care more about accuracy than win/loss. I think accuracy looked better slightly bigger in toms older mock. Understood this will stay as is unless you change your mind here Veloce but I think it deserves another shout since it's so minimized that it makes it hard to see at glance

As far as old app, I still prefer the extra line with 3+0•standard•rated and then time on line below
I mentioned I'd like to see the variant subtype (ie 3+0) but differentiating between standard vs 360 etc I think is also necessary. Old app was able to fit it so I think it can work

Lastly, I would still heavily hope 1 of 2 things happens

  • either current minimzie game list on home feed stays
  • or have a toggle option to switch between minimized and detailed game view

Edit: I also like the variant icon bigger like in the old app. The type of game you just played is more Important than the random name of your opponent which currently has the most focus

@ijm8710
Copy link

ijm8710 commented Oct 2, 2024

I also think I prefer from old app the subsequent line in the result (ie currently is says black resigned; in old app it would say black resigned, white is victorious).

It adds consistency to see what color won at a glance for all entries going down

@veloce
Copy link
Contributor

veloce commented Oct 3, 2024

I'm not sure about having the rating diff in that screen. Is that really important?

Thing is now we have both players names displayed, where the only important information is the opponent's name (because we already know the player name). If we were not on constrained space (width) that would not be an issue. Here it works in the screenshot because player names are short, but longer names would be cut.

I think this screen is interesting to have, but since it is a bit cluttered, having a toggle to switch to the other simple list view would be nice indeed (we just need a different icon).

What about having a grid view as well (2-4 columns, board thumbnail with date, opponent's name and result icon)? Seems easy to do (we have it already for puzzles). We'd then need a button to switch between the 3 formats.

@ijm8710
Copy link

ijm8710 commented Oct 3, 2024

Few things,

Just noticed (I think) in old app white is first, black second. Currently in this mock, it's "you" always first. I think the old app has better logic here

I don't quite often, he is usually spot on but I do disagree about Veloce about rating diff being too much. Is it possible to see what a photo would look like with max-length player names+master title? I feel like only a small shrink if any would be needed to account for all lengths.
Even the opponents name is not that important a detail so shrinking it slightly (if necessary) probably wouldn't hurt, no?

If all 3 views were shown which sounds fine, I still think this version should be default but with the current condensed the default on homepage

@tom-anders
Copy link
Contributor Author

I'm not sure about having the rating diff in that screen. Is that really important?

Thing is now we have both players names displayed, where the only important information is the opponent's name (because we already know the player name). If we were not on constrained space (width) that would not be an issue. Here it works in the screenshot because player names are short, but longer names would be cut.

I guess only your rating diff is important, your opponent's not so much.
Maybe we could go back to only show the opponent name, and then on a separate line the rating diff? Would still be less clutter than the current version.

I think this screen is interesting to have, but since it is a bit cluttered, having a toggle to switch to the other simple list view would be nice indeed (we just need a different icon).

What about having a grid view as well (2-4 columns, board thumbnail with date, opponent's name and result icon)? Seems easy to do (we have it already for puzzles). We'd then need a button to switch between the 3 formats.

Sure why not, we should then probably also remember the chosen view in the shared preferences, right?

@tom-anders
Copy link
Contributor Author

I also think I prefer from old app the subsequent line in the result (ie currently is says black resigned; in old app it would say black resigned, white is victorious).

It adds consistency to see what color won at a glance for all entries going down

This would add one more line of clutter/noise though

@ijm8710
Copy link

ijm8710 commented Oct 4, 2024

I also think I prefer from old app the subsequent line in the result (ie currently is says black resigned; in old app it would say black resigned, white is victorious).

It adds consistency to see what color won at a glance for all entries going down

This would add one more line of clutter/noise though

In old app, this seemed to always be one line.

@ijm8710
Copy link

ijm8710 commented Oct 4, 2024

Maybe we could go back to only show the opponent name, and then on a separate line the rating diff? Would still be less clutter than the current version.

I still think it looks a lot nicer in your most recent mock than the one before where YOUR name is included too
Especially if it dynamically orders to have white first
Worked fine in old app and if you're giving the choice of 3 views...
I really don't agree with Veloce on this one!

lib/src/view/game/game_list_detail_tile.dart Outdated Show resolved Hide resolved
lib/src/view/game/game_list_detail_tile.dart Outdated Show resolved Hide resolved
lib/src/view/game/game_list_detail_tile.dart Outdated Show resolved Hide resolved
@tom-anders
Copy link
Contributor Author

I'm not sure about having the rating diff in that screen. Is that really important?

Thing is now we have both players names displayed, where the only important information is the opponent's name (because we already know the player name). If we were not on constrained space (width) that would not be an issue. Here it works in the screenshot because player names are short, but longer names would be cut.

I think this screen is interesting to have, but since it is a bit cluttered, having a toggle to switch to the other simple list view would be nice indeed (we just need a different icon).

What about having a grid view as well (2-4 columns, board thumbnail with date, opponent's name and result icon)? Seems easy to do (we have it already for puzzles). We'd then need a button to switch between the 3 formats.

Added the toggle to switch, but for now it's only between the new (detailed) view and the old one. I think grid view would be something for another PR, wdyt?

I kept both user names for now, but the player being white is now displayed on the left always. This way, it actually adds some value, because you can quickly see in which games you were white and in which games you were black (which wouldn't be so obvious if we only display the opponents name).

lichess usernames seem to be capped to 20 characters, so maybe we could simply have some logic that descreases the font size if the length is above 10 characters or so? This way we should be able to always fit both names onto the screen

@tom-anders
Copy link
Contributor Author

lichess usernames seem to be capped to 20 characters, so maybe we could simply have some logic that descreases the font size if the length is above 10 characters or so? This way we should be able to always fit both names onto the screen

Added some logic for that, here's what it looks like if both players have the maximum name length:

Screenshot_2024-11-01_11-24-21

@veloce
Copy link
Contributor

veloce commented Nov 1, 2024

Added the toggle to switch, but for now it's only between the new (detailed) view and the old one. I think grid view would be something for another PR, wdyt?

sure and we should also not do it for now

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.

[Suggestion] Add played opening to "Recent Games"
4 participants