lotusprey / otraku

An unofficial AniList client for Android and iOS
GNU General Public License v3.0
327 stars 11 forks source link

Make images respect data saver mode #122

Closed dlsf closed 6 months ago

dlsf commented 6 months ago

The image quality now also depends on the data saver mode the user configured in the operating system. This behavior can be bypassed via a new config option (might be unnecessary). Since Flutter doesn't allow you to access this setting natively, I had to include a new library whose size should (hopefully?) be negligible. Whether the connection is metered is currently not checked, and would most likely require yet another library. Changes to the device's native data saver setting are ignored unless the app is restarted or one of the relevant config options changes.

The new file lib/common/utils/image_quality.dart currently does not contain a class, but it might be preferable. I can change it if that's the case.

The motivation for this change is the fact that users who are regularly on very limited connections might not want to change this kind of behavior for each app manually. Moreover, starting the app to update the quality in the settings already loads a considerable number of images in the home feed, which might cost them precious data volume or even money.

On Android, users can manually whitelist apps. If Otraku has been whitelisted, the app will display images normally. Only if data saver mode is enabled and the app hasn't been whitelisted, the images' resolution is lowered to medium. Whitelisted apps are supposed to still limit their data usage, so setting the quality to high at most might be worth considering.

lotusprey commented 6 months ago

I'm not sure how i feel about this. My concern with this package would be not so much the size, but the fact that it's only a month old and we don't know what maintenance we can expect. Granted it covers a small surface, but I'm just not sure if it's worth it right now, especially with the presence of the image quality setting. It's true images get loaded immediately on app start, but if even opening the app to change the image quality setting is a burden on your data, even low quality images won't save you. I'm not sold on the request yet. But maybe I could leave it open, so we may reassess it someday. Sorry that your work kind of got waster here; feel free to contact me before starting a feature, so we could agree on some decisions beforehand.

Notes:

dlsf commented 6 months ago

I'm not sure how i feel about this. My concern with this package would be not so much the size, but the fact that it's only a month old and we don't know what maintenance we can expect.

I've looked into this beforehand, but enabling a Flutter application to call these native methods isn't hard. I was about to implement it myself when I found that someone very recently made this and had the thought of integrating it into Otraku. I doubt they will make any breaking changes to this anytime soon, especially given that data limitations are starting to become less of a concern.

Sorry that your work kind of got waster here; feel free to contact me before starting a feature, so we could agree on some decisions beforehand.

Lol, it's fine, if this had been a major time investment I would have mentioned it beforehand. It just seemed like a small addition with very little downsides. My train of thought right now that on the off chance it breaks with some Android or iOS update, it could just be removed again.

lotusprey commented 6 months ago

Yeah, maybe I should look into custom platform code someday. I also saw pigeon mentioned in the flutter docs, which will apparently generate platform code for you? Sounds interesting.

dlsf commented 6 months ago

It's not very pretty with message channels, though. Personally, I just think that apps should try to respect a system-wide setting like this as a good practice when it's feasible, and I honestly don't quite see why it wouldn't be here even if this isn't the optimal implementation. If the dependency is worrisome, it could be replaced with native code, and if the additional config is too bothersome, it could simply be removed or replaced with a check for whether the network is metered.

If you are still not sure about it, we can just leave it, though.

lotusprey commented 6 months ago

I agree this would be nice to add at some point, but yes, I'm not sure about it yet.