riccardocescon / clean_chess

Lichess Client app made with Flutter
GNU General Public License v2.0
30 stars 7 forks source link

Settings pages #65

Closed Steccah closed 1 year ago

riccardocescon commented 1 year ago

First of all thank you @Steccah for your effort!🙏 I took a look on what you did and it's good. I improved the logic for those variables, Since we have lots of them we should consider groupping them up with some classes any use abstract classes as 'tags' in order to lighten the Widget page. As for the widgets, it is a very long page, i think it can be improved by removing a lot of boilerplate code just with the logic i just told you, if it is still long we may separate widgets into different files

I wrote an example class that is the DisplaySettingModel in order to show you how i think we should refactor this code. You can use the same logic for other classes on the settings.

Consider using enums rather than strings or enums for the settings, so we can track easly with variables and values

One last thing we're working on. Please don't use magic numbers inside widgets, you should create some constants explaining the meaning of the value and assign that rather then writing the war value directly inside the widgets

Take a look on my changes and feel free to ping me for any question!