✅ Fix "Download CAR file" on mobile #3816
Conversation
|
Your Render PR Server URL is https://social-app-pr-3816.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-copoejvsc6pc73dgrrtg. |
|
|
|
matthieusieben
changed the title
|
I'll check this today if Eric doesn't get around to it 👍 |
|
FYI i was able to test on web & ios. I couldn't get my android emulator to work (for an unrelated reason I believe). |
| }, [currentAccount, getAgent]) | ||
| const did = agent.session.did | ||
| const res = await agent.com.atproto.sync.getRepo({did}) | ||
| await saveBytesToDisk('repo.car', res.data, res.headers['content-type']) |
There was a problem hiding this comment.
At least on a simulator the problem I've been having with this (or really, the underlying Share) is that it keeps opening in mail on Android with this mime type (I think it's ending up as application/x-octet-stream). I tried just removing the mime altogether worked to give me the full list of options, but there isn't a "save" option.
With that said, could be that I'm on a simulator. Do regular devices have this same problem?
There was a problem hiding this comment.
I'll try on my native device.
There was a problem hiding this comment.
The android share menu does not allow to "save file" as ios does (some custom file manager apps do enable that but the default one does not).
So to actually share on device, I had to change the code for android. There is still a fallback to regular sharing through apps if the user denied access to disk for the save operation.
I tested using my own (Google Pixel 8 Pro) phone and a lot of apps do appear in the sharing menu.
| return url.toString() | ||
| }, [currentAccount, getAgent]) | ||
| const did = agent.session.did | ||
| const res = await agent.com.atproto.sync.getRepo({did}) |
There was a problem hiding this comment.
Aside from the issue with opening in mail on Android, this promise can take a while to resolve. For me it takes about 10 seconds or so for the download to complete (my CAR file is about 15 MB)
I think what we should do is
- Keep the old functionality on web. The browser should be handling the download there I think.
- Set the button to disabled with a spinner inside of it once you press download on native, so that the user knows something is going on.
I have some local changes that do all of that, but wondering about this mail issue right now 🙏
There was a problem hiding this comment.
The main reason for me to change this behavior is that I want to remove the code that builds the download URL. Indeed, with the oauth refactor coming up, that URL won't be as straightforward to build and I don't want to add complex logic in the client, when the building of the url is already handled by the client when making the API call. For this reason, I would prefer if we could keep the same behavior on web.
I agree that having a loading state would be significantly better. I didn't add one because I wanted my changes to be minimal. If you have branch with these changes, can you link it here? Or simply add the commit with the loader on this branch?
There was a problem hiding this comment.
Ahh makes sense! Yes I can push those up here in a bit, would have the other day but didn't want to push to your branch w/o asking first 😄 will clean it up and push it up here a bit later today!
There was a problem hiding this comment.
I added a loader.
matthieusieben
force-pushed
the
fix-mobile-car-download
branch
from
7187def to
99d7966
Compare
matthieusieben
force-pushed
the
fix-mobile-car-download
branch
from
99d7966 to
7b15c8d
Compare
|
@haileyok Everything should be good now. Could you kindly review this again :-) |
There was a problem hiding this comment.
Okay I fixed a few styling and localization nits. Tested this on all three platforms and it does work very well! Thank you for digging at the Expo docs to figure out how to do this correctly on Android (their new permissionless API is pretty interesting too!)
I also removed the fallback to sharing because it causes some issues. For example, if you open the file explorer and then press back instead of selecting a destination directory, it will try and open Gmail (at least on the simulator). I'll test this on a variety of devices today once it's merged to make sure this works on various Android versions, but I'm optimistic this will be okay (API 27 works fine for me on the simulator).
Also added some Sentry logging if errors are encountered at any point.
Tested on:
- iOS
- Android 27
- Android 31
- Android 34
- Safari mobile
- Safari desktop
- Firefox desktop
- Chrome desktop
* Update Japanese translations * Updated as PR on Plural is merged (#3882) * Feedback API (#3498) etc. * Improve localization marks (#3285) * Update Lightbox.tsx * Change strings for easier localization * Update DeleteAccount.tsx * Update LabelsOnMeDialog.tsx * Update FeedCard.tsx * Update index.tsx * Update LabelsOnMeDialog.tsx * Update index.tsx * Update FeedCard.tsx * Update SelfLabel.tsx * Update Hashtag.tsx * Update index.tsx * Update Hashtag.tsx * Update ChangeHandle.tsx * Update index.web.tsx * Update index.web.tsx * Update index.tsx * Remove unnecessary `<Trans>` tags * Update Drawer.tsx * Finnish translation update (#3755) * Update messages.po Translated new strings (GIF's etc) and cleaned up deprecated ones. * Update messages.po resolve conflict * Update messages.po removed double quotes. Thanks @lapanti * fix bad bool check in action (#3885) * fix bad bool check in action * add `fetch-depth` so we can get the commit hash * `.env` should be in `.easignore` 🙃 * Updated to follow #3285 etc. * The translation of 'place' and 'apply' of labels has been unified into '適用する'. * Fix translation of "repost" * Updated * Updated Japanese translation * Updated Japanese translation - #3816 * Fixed translations * Updated translation : #3995 * Updated Japanese translation * Updated Japanese translation (ref. #3962 #4028) * Updated Japanese translation * Updated Japanese translation * Updated Japanese translation --------- Co-authored-by: Minseo Lee <itoupluk427@gmail.com> Co-authored-by: Jan-Olof Eriksson <jan-olof.eriksson@iki.fi> Co-authored-by: Hailey <me@haileyok.com>
This change allows to download the CAR file in mobile apps.