[RNMobile] Audio Block: Add from URL #27817
Conversation
ceyhun
added
the
Mobile App - i.e. Android or iOS
|
Size Change: 0 B Total Size: 1.62 MB ℹ️ View Unchanged
|
|
@iamthomasbishop For android, in the picker's "Insert from URL" option, using which icon do you think would be the most appropriate? |
|
@ceyhun assuming we have access to the "globe" icon in the main GB icon set, we can use that one. If not, we also have one in the app-level Gridicons icon set. If neither of those work, the "link" icon would also work. 😄 |
|
@jd-alexander I gave this a spin on iOS and it's working great! Not sure what's the best way to handle any unsupported file extensions though 😄
Something close to a list of extensions I could find in the code was this: But it doesn't seem to be extensive as self-hosted sites can support many more like for example I'm planning to try Android when the WPAndroid crash is resolved. |
Ah, good eye. Thanks for testing @ceyhun This might be a tricky one especially since the URL doesn't have to end with an extension ( and I have seen situations where a file has a different extension than its actual MIME type). On the web, an instance of Audio() can be created to determine if a file is audio based but we don't have that behavior on mobile. The most reliable way we could determine if the URL is actually audio related would be to do a HEAD request and have a look at the headers of the response. For eg, utilizing https://reqbin.com/ , doing a HEAD request with
Interesting. Could we create an object with these extensions and match them with the resolved content type based on the idea explained above. Let me know if you think it's worth the effort and you can share your overall feeling about this solution. |
jd-alexander
force-pushed
the
rnmobile/audio-block-IV-from-url
branch
from
396caaa to
614d57b
Compare
@jd-alexander I really liked this idea! But feels like it'll need some more effort considering parsing the response and also we may need to show a loading UI when this request is happening and maybe a different error state as well. If we're going to put more effort into this block, I think it should be towards making the UI look more like the web counterpart by having that in-line audio player (that also plays the audio) to have the WYSIWYG look when the preview of the web page is rendered.
I think it could be a bit fragile for the future and we're possibly already missing some extensions. |
| }, | ||
| { | ||
| text: __( 'Apply' ), | ||
| onPress: ( url ) => onSelectURL( url ), |
There was a problem hiding this comment.
Nitpick: I think we could simplify this line with onPress: onSelectURL, as both functions match parameters.
There was a problem hiding this comment.
Nice! Good catch. Done here e68550e
There was a problem hiding this comment.
I've noticed that it allowed me to add links that don't reference an audio file, in that case, the block is not presenting any warning and it shows an empty file. I saw some comments related to this issue, are you planning to address it in this PR?
RPReplay_Final1621445109.MP4
Thanks for logging this @fluiddot I created an issue to track this wordpress-mobile/gutenberg-mobile#3530 We can move ahead with this PR since the web also has this behavior. If it pops up again in the community or support we can allocate some time to address it. |
Thanks @jd-alexander for creating the issue 🙇 !
Ok, from my side this PR looks great to me so you can feel free to merge unless there's anything blocking 👍 . |
| @@ -18,7 +18,7 @@ For each user feature we should also add a importance categorization label to i | |||
| - [*] Add enableCaching param to fetch request on Android [#31186] | |||
| - [***] Add reusable blocks to the inserter menu. [#28495] | |||
| - [*] The BottomSheet Cell component now supports the help prop so that a hint can be supplied to all Cell based components. [#30885] | |||
|
|
|||
| - [*] Audio block: Add Insert from URL functionality. [#27817] | |||
There was a problem hiding this comment.
I think this entry should go to the Unreleased section since release 1.53.0 was already cut.
Gutenberg Mobile PR wordpress-mobile/gutenberg-mobile#3031
Description
Adding "Insert from URL" capability to Audio Block insertion options.
TODO
How has this been tested?
Valid Url
Screenshots
Invalid Url
Globe icon changes.
You can compare the changes below with the Android options screenshot above to see the difference in icons.
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: