-
-
Notifications
You must be signed in to change notification settings - Fork 212
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 support for the new "hide rating except in game" option #1285
Conversation
0686925
to
79c5961
Compare
A tappable trailing "info" icon that displays the explanation in a tooltip. We once had this (or still have it?) for one setting. |
Ah yes, we have it for the move confirmation setting, thanks for the hint! |
Hi, when selecting this option and then trying to open the preferences in the new beta mobile app it throws an error. Reverting to "Yes" por "No" and it works fine again. |
That's to be expected, this PR will fix this |
Ok, added the explanation back. @veloce Since settings are currently broken when selecting the new option (see comment by @utterlyrandomuser above, I can reproduce as well), it would probably make sense to turn this PR into a hotfix release |
4d70e2a
to
bc6d636
Compare
e76b70b
to
b4ebaf1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a last change and we're good.
? const CircularProgressIndicator.adaptive() | ||
: null, | ||
), | ||
child: SafeArea( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This SafeArea
is useless and prevents the header blur effect so it should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍 Maybe it's time to do some refactoring and add a common widget for these setting choice screens? I can have a look in the next few days
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
This adds support for the new "show ratings, except in game" option introduced in lichess-org/lila#16228
One thing that's missing now is the "explanation" for what "hide rating" means/does that was previously displayed as a subtitle - I'm not sure where the best place to display it would be now.
showRatingsExceptInGame.webm