Add isFileURL method and use it on all native media upload checks. #20985
Conversation
SergioEstevao
added
the
Mobile App - i.e. Android or iOS
|
Size Change: +518 B (0%) Total Size: 857 kB
ℹ️ View Unchanged
|
packages/url/src/is-file-url.js
Outdated
| * @return {boolean} Whether or not it looks like a file URL. | ||
| */ | ||
| export function isFileURL( url ) { | ||
| return url.startsWith( 'file:' ); |
There was a problem hiding this comment.
I'm still testing, but I wonder if we will have a case where url is undefined. In some places we check the url exists before calling this but in some other places there aren't any checks. What do you think if we do:
| return url.startsWith( 'file:' ); | |
| return url && url.startsWith( 'file:' ); |
Just to be safe =D
There was a problem hiding this comment.
Good catch, I updated the code with that suggestion.
|
@geriux sorry about this but due to the latest changes can you please review this again, and test with the main apps? |
There was a problem hiding this comment.
I've not been able to test the impact this might have on the mobile application itself, but at least in addressing my previous concern and in understanding the problem to be that the mobile code previously relied upon a value of false from isURL as an indication that a URL is a file: URL, then the specific code revisions appear to be sensible, and furthermore bring some consistency even within the same file where these conditions are considered (replacing mixed conditions of url.startsWith( 'file:' ) and ! isURL( url ).
Thanks for your feedback and review @aduth, that's why it's always good to have more than one reviewer =)
Don't worry @SergioEstevao ! I'll start testing now 👍 |
|
Tested it again, the retry options of Media & Text, Image, Video, and Gallery on both iOS and Android and ✅ LGTM! |
…20985) * Add isFileURL method and use it on all native media upload checks. * Check if url is defined. * Add test for undefined and fix lint error for isFileURL * Replace isFileURL method for getProcotol == 'file' mettod
Description
This PR updates the mobile native code instances where the method isURL was being used to check if URL were local URLs (i.e: file urls like:
file:///folder/file.txt)This PR adds a method to check for file URLs,isFileURL, and updates media uploading code to use this method instead of!isURL(...)This PR replaces the uses of
isURLto check for file URLs, and usesgetProtocol(url) === ':file'instead.How has this been tested?
Unit tests were added to check the new method.
This can be tested using this mobile GB PR:
Screenshots
Types of changes
Checklist: