brarcher / loyalty-card-locker

Stores your barcode-based store/loyalty cards on your phone
GNU General Public License v3.0
171 stars 29 forks source link

Choose export location #352

Closed TheLastProject closed 4 years ago

TheLastProject commented 4 years ago

Should hopefully fix #349 by making the following changes:

This change assumes that every single Android device has a file manager available. I have yet to see an Android device where this is not the case.

cazzoo commented 4 years ago

approved

brarcher commented 4 years ago

I have yet to see an Android device where this is not the case.

I'm wondering about devices which are running a variant of AOSP (such as LineageOS) and do not have Google's apps. When the application was first written I was running an AOSP build that did not have a built-in file viewer that could be triggered from an app. The 'default' location was a workaround.

Maybe at some point AOSP started shipping an embedded file browser. I recall another project of mine needed a file browser, and the only way to provide one for all the supported Android versions which also worked on AOSP was to use another open source project and embed the file browsing.

https://github.com/spacecowboy/NoNonsense-FilePicker

Android 6 did not have a formal one, as mentioned here: https://www.howtogeek.com/231401/how-to-use-android-6.0%E2%80%99s-built-in-file-manager/

I think that Google now has its own file browser called Files. And, that probably will work if one has Google's apps installed.

Anyway, this change probably works for many users. But, i do not think it will work for everyone. Or, maybe I'm missing something?

TheLastProject commented 4 years ago

I can only state that every LineageOS version I've ever used has a built-in file manager, but I understand your point.

I think the way to go here may be to try to call the file manager and if there is nothing to answer the intent, to then save to a fixed location. That way it'll work on all devices and everyone with a device modern enough to have a file manager will be able to choose the location themselves.

I would suggest that merging this even with the chance of breaking backup on some pre Android 6 devices with AOSP may be worth it, especially given Google's constant increasing limiting of direct file access and the feeling it may happen on all Android Q devices which will only increase in popularity (sadly, I don't have one myself and I don't have the processing power on my main device to properly run emulators). Still, I will look into trying to add a fallback at some point probably.

brarcher commented 4 years ago

You've convinced me. Let's pull this change in.

I notice that in the "Make linter happy" commit a string resource is removed that is used in the master branch. I had to remove that commit locally to build successfully. Can you update the PR to master? I'll pull the change in then.

TheLastProject commented 4 years ago

Should be all good now :)

brarcher commented 4 years ago

Note to self: Travis-CI deprecated how they posted status back to pull requests. The build did trigger and pass, but status was not reported. Link to passing build: https://travis-ci.org/brarcher/loyalty-card-locker/builds/659888648