ubuntu / archive_yaru.dart

Ubuntu Yaru Flutter Theme
https://ubuntu.github.io/yaru.dart/
Mozilla Public License 2.0
334 stars 40 forks source link

refactor: improved code readibility and moved YaruVariant methods inside the enum #398

Closed TheLiux closed 11 months ago

TheLiux commented 1 year ago

Hi I made some changes to improve the code readability of the library. I strongly believe that is better to use early returns instead of nested if. Additionally, since Dart allows adding methods inside enums, I put inside the YaruVariant enum some methods that was scattered on the package some methods that return YaruVariant.

This will close the following issue: ubuntu/yaru_widgets.dart#851

Let me now what you think about it :D

Feichtmeier commented 11 months ago

@TheLiux thank you

It would be better if you would split this PR into smaller ones. Let's start with the typo part please

TheLiux commented 11 months ago

@TheLiux thank you

It would be better if you would split this PR into smaller ones. Let's start with the typo part please

Hi, thanks for reply. I removed the typo commit and added in another PR 😄 https://github.com/ubuntu/yaru.dart/pull/402

Feichtmeier commented 11 months ago

Nice, ty :) ! Can you do the same with splitting YaruVariant stuff and the nested-if thing? Our CI is not optimal and we still have the golden test PR pending (which we have working in yaru widgets), but we need to make sure that we dont end up with visual regressions For the nested-if-PR please add screenshots showing that the theme stays the same For the YaruVariant PR we need maybe a video where you toggle through the variants in gnome settings and see if a yaru app follows those changes (sorry for the additional work, but we are now at a point where a lot of canonical apps depend on this repo. We need to become better with CI/CD)

TheLiux commented 11 months ago

Nice, ty :) ! Can you do the same with splitting YaruVariant stuff and the nested-if thing? Our CI is not optimal and we still have the golden test PR pending (which we have working in yaru widgets), but we need to make sure that we dont end up with visual regressions For the nested-if-PR please add screenshots showing that the theme stays the same For the YaruVariant PR we need maybe a video where you toggle through the variants in gnome settings and see if a yaru app follows those changes (sorry for the additional work, but we are now at a point where a lot of canonical apps depend on this repo. We need to become better with CI/CD)

Yeah sure, I understand that! I will do ASAP with the screenshots and media you need! Just give me some time, i'll do it once I'm done with work 😅

TheLiux commented 11 months ago

I closed this PR and make new ones to have commit history aligned