IDEMSInternational / open-app-builder

PLH App Frontend
GNU General Public License v3.0
5 stars 24 forks source link

Feat: share assets; save assets to device #2160

Closed jfmcquade closed 8 months ago

jfmcquade commented 8 months ago

PR Checklist

Description

Adds functionality to share files and download files.

Share

See videos below demonstrating the share functionality on web and Android platforms.

Save to device

Open external

Testing

Using both the PR web preview and the appetize link, test all of the new functionality (ideally try multiple Android versions with the latter).

The relevant templates are feature_share, feature_share_file and feature_save_open_file.

Dev notes

New permissions are required, namely READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE.

The Capacitor docs says the following regarding writing to external storage on Android:

It's not accesible on Android 10 unless the app enables legacy External Storage by adding android:requestLegacyExternalStorage="true" in the application tag in the AndroidManifest.xml. It's not accesible on Android 11 or newer.

And they offer no alternative method of writing to external storage. However, testing on Android 12, the functionality does work as expected (see videos below).

If any of this causes an issue, we could potentially remove the "save to device" functionality for now, and rely on the "open external" functionality (which does not write to the external storage or need extra permissions).

Git Issues

Closes #1951 Closes #2064

Screenshots/Videos

Share

feature_share template

Screenshot 2023-12-08 at 11 33 24

feature_share_file template

Screenshot 2023-12-08 at 11 33 49

Share in browser (Safari, macOS)

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/7e5bf63e-1a7a-4eac-b544-3de4ad3de64f

Share on Android (emulator)

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/24d22e61-5e5a-420f-9480-a74bb6d7713a

Save to device

feature_save_open_file template

Screenshot 2023-12-13 at 13 26 53

Web (Chrome, macOS)

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/67cbe4b9-07e5-4cf1-a416-aef6eeb9585a

Android (emulator)

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/42b2bb1f-0ab1-476c-b73b-6bf6c4ff7699

Open external

feature_save_open_file template

Screenshot 2023-12-13 at 13 26 53

Web (Chrome, macOS)

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/89542bb0-6845-490f-be3e-4e1b396b002d

Android (emulator)

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/253c8061-787e-45b4-8652-88f356acdba3

github-actions[bot] commented 8 months ago

Visit the preview URL for this PR (updated for commit 5549cd2):

https://idems-debug--pr2160-feat-asset-download-8iw636i0.web.app

(expires Fri, 29 Dec 2023 15:46:35 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f2bfd21fc73bf25db0e74968614b72bee30e3476

github-actions[bot] commented 8 months ago

Android Appetize URL https://appetize.io/app/wbt5facrtrcczv7wcbbnhh386r?device=pixel4&osVersion=12.0&scale=75

jfmcquade commented 8 months ago

UPDATE: this problem with permissions should now be fixed, see newer comment.

I've done some more research into the new permissions needed to save files to the Download folder (triggered by the save_to_device action), and I think it may be worth excluding that functionality for the imminent release.

The capacitor filesystem docs state that in order to Directory.ExternalStorage, in Android 10 and older, the permissions READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE are required. These are considered "dangerous" (docs here) and so require additional checks. Namely, according to these docs, apps requesting these permissions:

must successfully pass an appropriate access review prior to publishing. Apps allowed to use this permission must clearly prompt users to enable 'All files access' for their app under 'Special app access' settings.

They must also complete a permissions declaration form.

Devs report that adding these permissions can mean that the update is rejected by Google because this is not core functionality of the app (which seems to apply in our case).

So whilst the functionality of saving to the Downloads folder should work fine on devices running Android 11 and higher (see the first comment on this question), for earlier versions of Android, we would need those permissions which comes with significant complications.

I propose removing this functionality for now and removing those requested permissions, and at least temporarily setting save_to_device to save the file to the app's ring-fenced storage rather than the general "Downloads" folder.

@chrismclarke, let me know if you have any experience of similar permissions issues or general advice.

jfmcquade commented 8 months ago

The above should be fixed by this latest commit. The permissions are no longer required if we save the file to the device's Documents folder, as opposed to EXTERNAL_STORAGE/Download folder, see capacitor filesystem docs. I think this is because saving in the former location does in fact save to a location ring-fenced for the app, but files saved there do show up in the user's general Documents folder, where no equivalent exists for the Downloads folder.

See new demonstration video:

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/b98e2597-466e-40c0-8ab8-ec10e2ad135a

esmeetewinkel commented 8 months ago

@jfmcquade when I'm testing "save" now on Appetize I can't find the file in the file explorer:

screen-capture (79).webm

jfmcquade commented 8 months ago

@jfmcquade when I'm testing "save" now on Appetize I can't find the file in the file explorer:

I think this is just a quirk of the appetize viewer. You can find the file if you go through the storage location named sdk_gphone***, where it appears inside the enclosed Documents folder, but not the top-level Documents folder in the sidebar. See screen capture below. Although I've just been testing a few times in a row, and I'm sure it did appear in the Recent folder during one test (after first finding it through sdk_gphone*** > Documents > etc.), so it appears to be temperamental.

Anyway, I think this issue is only present when running in an emulator, due to the storage itself being emulated in some sense. @chrismclarke (sorry to keep tagging you...), do you have any insight?

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/8c1b843d-226f-4c61-9cee-37c3c9c1b51c