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

Convert drawables to svg and use resource resolution for light/dark theming #68

Merged
merged 3 commits into from
Jul 3, 2022

Conversation

Abestanis
Copy link
Collaborator

@Abestanis Abestanis commented Jun 21, 2022

This converts all of our drawables to svgs. We can do this because svg drawables are supported from Android API 21 onward, which is our current minimum target sdk version.
By converting to svg we reduce the size of the application and make it look sharp regardless of the screen resolution.
We can't remove the launch_foreground.png yet, because gradient color is not supported below API 24.

This also removes the different resources for light and dark themes and the system brightness listener in AudioHandler, and instead relies on the automatic themed resource lookup, which get's triggered automatically when the system theme changes. This part is really hard to test, because most systems apply their own theming for media control buttons (e.g. in the Android emulators, they are always white). I observed the switching of colors on my real phone, but if you have a setup that you used to test this when developing it originally, please make sure to test with these changes again.

For context, this is part of my work towards a home screen widget, I want to reuse these drawables.

Copy link
Owner

@nt4f04uNd nt4f04uNd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested theme on 3 devices, everything is OK.

I noticed 2 subtle differences:

  1. Start screen looks different, it seems that 1.0.8 variant is better, but it's really subtle so we can ignore it
  2. Loop icon 1 text looks different, but I like it
1.0.8 This branch
start screen 1_1 1
notification 2_2 2

lib/logic/player/music_player.dart Outdated Show resolved Hide resolved
@Abestanis
Copy link
Collaborator Author

I tested theme on 3 devices, everything is OK.

Perfect, thanks.

I noticed 2 subtle differences

Yeah, those are the two svgs I had to recreate by hand, I tried to make them exactly the same but evidently didn't succeed.

Start screen looks different ...

It's much more visible in your screenshot than what I saw on the emulator. The visibility of the end of the gradient depends on how I look at the monitor, it seems 🤣. Getting this gradient right seems to be no easy task, let me thinker a bit with it and see if I can improve it.

@Abestanis Abestanis force-pushed the feature/svg_drawables branch from b170d7f to dd56fe1 Compare June 22, 2022 17:24
@Abestanis
Copy link
Collaborator Author

Ok, this is pretty close:

Before image
1.0.8 image
After image

@nt4f04uNd
Copy link
Owner

Yeah, those are the two svgs I had to recreate by hand, I tried to make them exactly the same but evidently didn't succeed.

Could you not just use an existing SVG for that?

@Abestanis
Copy link
Collaborator Author

Could you not just use an existing SVG for that?

I didn't realize those existed as SVGs already. If you can point me at them then I can definitely import them.

@nt4f04uNd
Copy link
Owner

Loop icon

Logo foreground

@nt4f04uNd
Copy link
Owner

nt4f04uNd commented Jul 2, 2022

(it looks like we no longer need loop icon, so we can delete it, I like the new one more)

@Abestanis
Copy link
Collaborator Author

Ah, i looked in the assets folder, but not in static_assets.

(it looks like we no longer need loop icon, so we can delete it, I like the new one more)

👍

@Abestanis
Copy link
Collaborator Author

Do you want me to put the two loop_one SVGs which I created as a source for the drawables into the static_assets folder? I'm not sure how easy it is to convert the drawable back into an SVG, if we would ever need to.

@nt4f04uNd
Copy link
Owner

No, we don't need that

@Abestanis
Copy link
Collaborator Author

Looks like I cant use the foreground logo unfortunately, it contains some SVG elements that are not supported in Android drawables:

ERROR @ line 14: <filter> is not supported
ERROR @ line 15: <feFlood> is not supported
ERROR @ line 16: <feBlend> is not supported
ERROR @ line 18: Semitransparent mask cannot be represented by a vector drawable

@nt4f04uNd
Copy link
Owner

Can you just copy the gradient values from it then?

Copy link
Owner

@nt4f04uNd nt4f04uNd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it's not easly doable, it seems. LGTM then

@nt4f04uNd nt4f04uNd merged commit 7fc7418 into nt4f04uNd:1.0.8 Jul 3, 2022
@Abestanis Abestanis deleted the feature/svg_drawables branch July 3, 2022 09:22
@Abestanis
Copy link
Collaborator Author

it's not easly doable

Yeah, I tried as well, but no success.
Do you just want to delete static_assets/edit/loop_on.svg, I forgot to push the commit.

@Abestanis Abestanis mentioned this pull request Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants