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

Maintenance PR #445

Merged
merged 19 commits into from
Nov 9, 2023
Merged

Maintenance PR #445

merged 19 commits into from
Nov 9, 2023

Conversation

12people
Copy link
Contributor

@12people 12people commented Nov 5, 2023

Various minor fixes, partly to get the project to build on macOS with the latest version of Flutter, partly to fix lint warnings.

All changes are annotated in the relevant commits.

@12people
Copy link
Contributor Author

12people commented Nov 5, 2023

The CI tests all fail because they use an old Dart version.
Could the Dart SDK for those checks be updated?

@12people
Copy link
Contributor Author

12people commented Nov 6, 2023

I also added new icons for all platforms.
iOS now has sharper (but optimized via optipng) icons, macOS now has squircle icons, as is the norm on latest macOS versions, and Android now has a vector-based icon, which also serves as a monochrome icon (solving the problem that is mentioned in #442 ).

@rolandgeider
Copy link
Member

Cool, thanks!

Yeah, we need to finish work on fix/replace-chart-lib, the old chart lib was discontinued and is preventing us from updating flutter to something not old, but it's just some polishing left (#433)

@rolandgeider
Copy link
Member

and Android now has a vector-based icon, which also serves as a monochrome icon (solving the problem that is mentioned in #442 ).

aweome, will check it out

@12people
Copy link
Contributor Author

12people commented Nov 6, 2023

Cool, thanks!

Yeah, we need to finish work on fix/replace-chart-lib, the old chart lib was discontinued and is preventing us from updating flutter to something not old, but it's just some polishing left (#433)

Oh, what I did was just replace it with the community-maintained fork: community_charts_flutter. It's basically a drop-in replacement.

I do agree, though, that using fl_charts is probably best in the long run.

@rolandgeider
Copy link
Member

BTW, how did you generate the images? We were (or rather, used once) the flutter_launcher_icons package

@12people
Copy link
Contributor Author

12people commented Nov 6, 2023

@rolandgeider Manually.

I made a template for app icons in Figma for myself, which I've used for all my projects and it was easy to just plug the wger icon into that too.

Figma lets you specify multiple export sizes as well as suffixes, so once you've got that set up in Figma, getting the iOS and macOS icons is just a simple matter of dragging those exports into the right folder. For Android, I exported an SVG for the foreground and converted that into an Android Drawable via Android Studio.

(I didn't update any of the old Android PNGs, as that would require manually renaming each exported PNG and dragging it into the right folder — while Figma supports specifying export prefixes and suffixes, it does not support specifying different folders for each export, so it's not quite as simple to generate PNG icons for Android. I suppose you could write an automated script for this, but a vector version works well enough for everything v26 and onward, so I didn't see a need to.)

@rolandgeider
Copy link
Member

so if/when we change the target sdk to 26 we could remove the android png icons, right?

@12people
Copy link
Contributor Author

12people commented Nov 8, 2023

so if/when we change the target sdk to 26 we could remove the android png icons, right?

Sure.

That said, the vector drawable in the PR is used automatically for all platforms 26 and above, so the PNGs only serve as fallback for older platforms.

@rolandgeider
Copy link
Member

👍🏻

yeah, that's what I meant. I think at the moment we have something lower than that, like 23 or so

rolandgeider and others added 2 commits November 9, 2023 20:24
# Conflicts:
#	lib/theme/theme.dart
#	lib/widgets/core/charts.dart
#	lib/widgets/dashboard/widgets.dart
#	lib/widgets/nutrition/charts.dart
#	lib/widgets/workouts/charts.dart
#	pubspec.lock
#	pubspec.yaml
#	test_driver/screenshot_driver.dart
@rolandgeider rolandgeider merged commit 345a99e into wger-project:master Nov 9, 2023
1 check passed
@rolandgeider
Copy link
Member

Merged thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants