lichess-org / mobile

Lichess mobile app v2
GNU General Public License v3.0
1.37k stars 198 forks source link

feat: coordinate trainer #898

Closed tom-anders closed 2 months ago

tom-anders commented 3 months ago

I think it makes sense to use the new ChessboardEditor widget here as well, since we can reuse the onEditedSquare callback to get notified when the user taps a square. With the normal Chessboard widget, this is currently not implemented, we only have callback when a (legal) move is made.

Haven't done any work on this yet, just opening the draft to let people know I'll start working on this next.

tom-anders commented 3 months ago

One open question: In the Website and the old app, the coordinate trainer is in the "Learn" category. The new app does not have that category, so I put it under "Tools" for now. Not sure if this is the ideal solution, we might add more features from the "Learn" category in the future. I don't think we want to add them all to "Tools"?

@veloce I attached a video of what I have so far. Code is not ready to be reviewed yet, but design/layout suggestions are welcome :)

coordinate_trainer.webm

(EDIT: Updated video)

tom-anders commented 3 months ago

As I added more settings, I ran out of space below the board (before starting the training). My current solution is to show a separate settings/configuration screen:

Screenshot_1722774385

ijm8710 commented 3 months ago

Only three pieces of feedback I have

veloce commented 3 months ago

It looks quite good @tom-anders !

One suggestion for the settings: separate the switch toggles to a different screen but keep the side and time selection below the board. It makes sense because the switches are settings and the rest is more a training session kind selector.

tom-anders commented 3 months ago

It looks quite good @tom-anders !

One suggestion for the settings: separate the switch toggles to a different screen but keep the side and time selection below the board. It makes sense because the switches are settings and the rest is more a training session kind selector.

Thanks, sounds good! Maybe we could move the "Show pieces" and "Show coordinates" to a menu button in the bottom bar (like in the board editor). Then I could also add a "Help" Button to that bottom bar, that will display the same explanation as on https://lichess.org/training/coordinate

tom-anders commented 3 months ago

Only three pieces of feedback I have

should you have an infographic button or section that describes what the trainer does? (I see on mobile they have a detail section)

Yeah, I'll plan to add one with the same text as on the website

minor, but for side choice personally I like showing a king or queen piece image in white, black and a half back/white piece. On mobile they show that versus having the words black white in the settings. And "random" should probably be the middle tile

What do you think about this, @veloce ? I saw that in "Custom Game" we already have a white/black/random button that uses text instead of icons, so I went with that here as well.

does the api allow for hi score to be tracked? I'm not sure if it exists but would be nice to have if possible

I don't think so, I took a quick look at the old app's code, and it seems to save the score locally. I don't think we even need to implement score saving in the first iteration of this feature. I think it sounds like a "Good first issue" for new contributors :)

veloce commented 3 months ago

What do you think about this, @veloce ? I saw that in "Custom Game" we already have a white/black/random button that uses text instead of icons, so I went with that here as well.

We don't have the asset and to be consistent let's keep it like you did for now.

veloce commented 3 months ago

I don't think so, I took a quick look at the old app's code, and it seems to save the score locally. I don't think we even need to implement score saving in the first iteration of this feature. I think it sounds like a "Good first issue" for new contributors :)

right

tom-anders commented 3 months ago

Okay, changed the UI according to your suggestions. I also added a three second countdown (I thought this would be useful when trying to beat your highscore, as you have some more time to focus)

Code is ready to be reviewed now, I'll add unit tests after the first round of review.

coordinate_trainer.webm

EmmetSchuler commented 3 months ago

Thanks for all the work on this, it looks really nice! I just have a few questions / suggestions that I think could improve it even more. Is there a plan on adding animation to the coordinate prompts as they move towards the center of the board? This appears in the website and older mobile app.

Also, the padding / font size of the prompts seem to take up a lot more of the screen than I'm used to with the website version. image Above is the website version, and below is a screenshot of the video for the Mobile App. image

Firstly, the main prompt on the website is placed off to the side, so it doesn't fill as much of the board center, and the text itself is also smaller, taking up less individual squares of space. I also don't think we need a 3rd coordinate prompt, as it just takes up even more board real estate, and it isn't seen on the website either. Secondly, the second coordinate prompt is much smaller on the website compared to this pr, and is more off to the bottom right side. Finally, the font that is chosen on the website does a good job at keeping similar sizes for the numbers and letters, but I notice a much bigger difference between number size and letter size with the font being used in the pr. This isn't too important though, maybe I'm just used to the website font.

Obviously the beta app doesn't have to (and shouldn't) be a perfect replication of the website, but I think these changes could really improve the end experience for the user. Again, the PR looks really good, thanks for all your work on this!

tom-anders commented 3 months ago

Thanks for the feedback, much appreciated!

Thanks for all the work on this, it looks really nice! I just have a few questions / suggestions that I think could improve it even more. Is there a plan on adding animation to the coordinate prompts as they move towards the center of the board? This appears in the website and older mobile app.

Yeah that sounds like a nice touch, will try to add it.

Firstly, the main prompt on the website is placed off to the side, so it doesn't fill as much of the board center, and the text itself is also smaller, taking up less individual squares of space. I also don't think we need a 3rd coordinate prompt, as it just takes up even more board real estate, and it isn't seen on the website either. Secondly, the second coordinate prompt is much smaller on the website compared to this pr, and is more off to the bottom right side.

I based the design on the old mobile app, which has the three coordinate prompts and rather large letters. But I agree that the website version looks much cleaner! I'll try to adapt the design.

Finally, the font that is chosen on the website does a good job at keeping similar sizes for the numbers and letters, but I notice a much bigger difference between number size and letter size with the font being used in the pr. This isn't too important though, maybe I'm just used to the website font.

Hmm, I'm not using any special font here, it's just the default font. Not sure what @veloce thinks about explicitly using a different font here

EmmetSchuler commented 3 months ago

I just thought of something, and although it's different to the other methods that we've used, it seems pretty cool.

What about when you tap a square, and you get the coordinate right, the square turns green, just like it does when selecting a piece (the whole square is highlighted). When getting the wrong coordinate, it would light up red.

I've always felt just a little bit uncoordinated (pun intended) on mobile for coordinate training, because it's hard to see exactly which square you select. It could also help with grasping the coordinates, and seeing exactly the square that lights up.

Alternatively there's this interesting highlight square function when using the 'name the coordinate' mode on the website. Image below Screenshot_20240806-231832.png

I don't think this mode is planned, and it probably won't work too well on mobile, but the ring highlight around the square is an interesting idea when selecting the correct square.

Let me know what you think about this. Obviously it's a pretty foreign concept compared to what we already have, but I think it's interesting.

tom-anders commented 3 months ago

Sounds very cool, I like the idea with the green and red highlight! Will try that out

veloce commented 3 months ago

Hmm, I'm not using any special font here, it's just the default font. Not sure what @veloce thinks about explicitly using a different font here

If really the font makes a diff, why not. Have you tried tabularFigures?

Alternatively there's this interesting highlight square function when using the 'name the coordinate' mode on the website. Image below

The square highlight feature would be nice on mobile too. Only using the buttons and not the keyboard would work well I think. (Voice recognition also, but that's not for a V1 😄 )

tom-anders commented 3 months ago

I figured out that the main reason why the text looked "weird" was that the font was not monospaced. I fixed that now, did some more adjustments and also added an animation. Still WIP, but here's a quick demonstration of how it looks right now:

coordinate_trainer.webm

EmmetSchuler commented 3 months ago

Much improved, it looks awesome! The only feedback I can think of would be to slightly increase the opacity of the primary coordinate prompt and keep the secondary prompt how it currently is. I think it would complete that 'bold' look that the website has. Thanks again, super excited to try it out once it reaches the app!

veloce commented 3 months ago

Much better like that indeed. Feel it would be better if the prompt was centered though.

tom-anders commented 3 months ago

Made some more adaptions to the "Find Square" mode, I've attached a new video.

@veloce I haven't implemented the "Name Square" mode yet, but I think I want to first clean up this PR's code and add unit tests. Once this PR is merged, I'd start working on "Name Square" in a new PR. wdyt?

coordinate_trainer.webm

tom-anders commented 3 months ago

Cleaned up the code and added some widget tests, this is now ready to review :)

tom-anders commented 3 months ago

(Needs https://github.com/lichess-org/flutter-chessground/pull/47 for successful CI build)

veloce commented 3 months ago

This will be queued a little, sorry about that @tom-anders , as I need to focus on the next release with board editor and explorer. But I'll deal with the chessground PRs asap.

tom-anders commented 3 months ago

This will be queued a little, sorry about that @tom-anders , as I need to focus on the next release with board editor and explorer. But I'll deal with the chessground PRs asap.

Yeah no problem! I totally understand that other features have higher priority

veloce commented 2 months ago

One open question: In the Website and the old app, the coordinate trainer is in the "Learn" category. The new app does not have that category, so I put it under "Tools" for now. Not sure if this is the ideal solution, we might add more features from the "Learn" category in the future. I don't think we want to add them all to "Tools"?

I'd rename the tools tab to learn tab. Then inside the tab we can have a "Tools" subsection. Think of them as learning tools. What do you think?

tom-anders commented 2 months ago

One open question: In the Website and the old app, the coordinate trainer is in the "Learn" category. The new app does not have that category, so I put it under "Tools" for now. Not sure if this is the ideal solution, we might add more features from the "Learn" category in the future. I don't think we want to add them all to "Tools"?

I'd rename the tools tab to learn tab. Then inside the tab we can have a "Tools" subsection. Think of them as learning tools. What do you think?

The clock being a "learning tool" is a bit far fetched I guess, but yeah it's better than the current solution at least.

One other thought I had would be to actually get rid of the "Settings" tab and make them accessible from a button in the top bar instead. This would make room in the bottom bar for a separate "Learn" tab button

ijm8710 commented 2 months ago

Personally I prefer Veloce approach where tools and learn are somewhat intertwined and settings still exists as a tab

EmmetSchuler commented 2 months ago

I think the coordinate trainer could be justified as a tool, and even the study feature could eventually be seen as one. It just depends on whether the app will eventually have real learning elements, such as the 'practice' and 'chess basics' section - although these are a long way away regardless.

A split bottom bar for the learn tab may feel like we have too many bottom rows dedicated to lists, so maybe a split section is the way to go.

tom-anders commented 2 months ago

Yeah I guess let's keep the "Tools" tab for now then, we can revisit this discussion once we have more "Learn" stuff in there and it becomes to crowded I'd say

tom-anders commented 2 months ago

(rebased against main, finally a green checkmark, yay!)

tom-anders commented 2 months ago

Thanks for the review and the helpful comments!