-
Notifications
You must be signed in to change notification settings - Fork 4.7k
[RNMobile] Audio Player UI for audio block #27467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
|
To be clear, this has no functionality. The only "important" piece of functionality missing is the calculation of the audio file's time length, but we can obtain it from the {
"ID": 3865,
"//": "...",
"file": "christmas-time-6.mp3",
"mime_type": "audio/mpeg",
"extension": "mp3",
"length": 238, <---
"exif": {
"//": "...",
"length": 238, <---
"length_formatted": "3:58", <---
},
"//": "...",
}But this won't be available for files added via external URL. If this is really necessary, I would implement this after this approach is accepted. |
|
Nice work @etoledom! I think this should work pretty nicely visually! My one concern is that there could be edge cases where this type of placeholder/façade UI will be confusing — such as when there's an error. I suppose this is pretty similar to how the Video block works so I don't expect any major issues, but let's make sure consider the various stress cases. For example:
For all of these examples, I think we can probably simply replace the "play" button depending on state:
|
If I may add, even if not an error has occurred, I think it might be confusing anyway since the buttons won't be actionable at all. For example, even if they look disabled, it's not clear why they are disabled or how can someone enable them.
I agree that the video block is semi-fake as well, but user can indeed tap the "Play" button and playback will commence in a browser screen. On the Audio block, tapping the Play will do nothing at all, right? |
6acd4ef to
276b07d
Compare
1e4d554 to
8ad70a2
Compare
|
Hey there 👋 I'll be continuing the work here.
That's right and it's the same behavior on web. But do we want the mobile audio block to play audio in the editor the end? That could introduce some other complexities and challenges as mentioned in the PR description. @iamthomasbishop WDYT? Should we maybe make the current "fake" disabled UI look more like it cannot be interacted instead? |
|
Thanks for these thoughts @ceyhun
I honestly think this could become a possibility in the future.
Yes, it will need to look disabled so the user doesn't try to interact with it. Honestly, now when I think about it, I would prefer if the Audio block looked more like the File block ( with a few minor alterations like adding an Audio Icon or something to it). The reason, I am thinking this is we are attempting to create a disabled UI similar to the web but as @iamthomasbishop stated there are several cases where the player UI could become confusing. So going down the File block path would allow us to avoid these cases as we already solved for them already. Of course, this would deviate away from the experience of the web a bit but I think that's an okay compromise for now since the functionality being offered by both blocks on their respective platforms would be the same. The work done already could be utilized in a project that would require the Audio block to have playback functionality. Let me know what you think peeps. |
As I see it, the final goal of this it is to deliver the WISIWIG experience. It's not as much as it look similar to the Web editor, but closer to the final published post. This concept is mainly visual, so it's fine if the editor won't play audios on the editor.
I agree that this would be good to avoid some confusion 👍 (Just wanted to share my thoughts of why I started this PR and why I thought it could be a good approach 😃 ) |
Ohh I never thought of it from this point of view. Thanks for this.
I agree with this as well.
Appreciated @etoledom 😄 |
|
Thanks everyone! Then I guess it makes sense to continue with the current approach for now.
@iamthomasbishop any ideas about how to achieve this?
@iamthomasbishop also again I couldn't really picture this because compared to the Video block we have in the Audio block also the slider, time indicators and volume controls. Could you maybe share some visuals? |
|
@iamthomasbishop I wanted to provide some more context about some of the suggestions you made:
We won't be shipping that circular indicator this time around as there are some upload flow improvements that need to be made and optimizing a new control for the current functionality won't be beneficial for us long term. More context about this in Eduardo's comment here So with that said, would we be able to utilize the current upload progress UI/UX as one of the states and pair it with the state definitions you created that are based on replacing the play button in the disabled state of the control? A compromise I just thought of is to utilize the RN ActivityIndicator as you mentioned to replace the play button and then the actual progress of the upload would be similar to the File/Image block etc. Let me know what your thoughts are on this. I think this combination would work and would create a consistent experience of the play button being replaced for all state changes. Let me know your thoughts. |
|
@ceyhun 👋 Until we're ready to implement a true inline player UI, I think it might be prudent to design something explicitly "placeholder-ish", rather than try to replicate the native player. I spent a few minutes sketching out an approach that I think could work, which borrows elements both from the placeholder/starter state of various media blocks and also the general layout of our Cell component. Here's a quick sketch of what I'm imagining for the component itself (which could be re-used for other scenarios where we want to show a "compact" representation: And based on that, here are a bunch of variations, using either icon or text button as the right accessory: I'm leaning towards a text label (middle column) because the primary action is very clear. In terms of the copy for that label, my suggestion would be either "Listen", "Open", or "View" — "Preview" might be confused with post/page preview, although a pre-pended/appended "play" icon on the text label might be enough of an affordance to avoid that issue. Here are some quick higher-fidelity mocks to get a feel (note: colors, sizing, etc. not final): I'd love to hear y'all's thoughts and/or suggestions! // cc @ceyhun @etoledom @jd-alexander and @kyleaparker |
|
Thanks for these high fidelity mockups @iamthomasbishop I am in agreement with the placeholder UI being represented like this until we have an actual player. I am in favor of the rounded designs on the right because it looks similar to the preview of the Audio block on Chrome. I also like the text label as the primary action so we can be clear on what the user will be able to perform. Would we still utilize the approach of changing the main icon based on the state? |
This reverts commit b1eb8dd.
Co-authored-by: Ceyhun Ozugur <[email protected]>
Co-authored-by: Joel Dean <[email protected]>
This reverts commit e6b5b6d.
jd-alexander
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior here was tested and the changes work as expected. LGTM 🚢
* Basics of Audio block working * Add audio support to MediaUpload * Add handling of file uploads and replace * WPMediaLibrary support for Audio block * Avoid removing media info on error state * Linting * Added an AUDIO file to the test requestMediaPickFromMediaLibrary func * Fixed typo in ToolbarButton of Audio Block. * Removed auto help behavior present on web that's not used on mobile. * [Android] Wired the click of the Audio Media Library button. * Added Audio media options for choosing audio file locally. * [RNMobile] Audio Block: Proper caption field (#27689) * Audio Player UI for audio block * Add extension to styles import * Show file name while loading and retry message on error * Pass state props to audio player component * Implement placeholder-ish player UI structure * added styles for icon, title and subtitle * added blue-wordpress as link color. * added sizing based on design specs. * Fixed object destructuring error. * added icon styling state for upload in progress or upload failed. * implemented error style and behavior. * added styling for upload failed text. * Override MediaUploadProgress styles * Update filename extension spseration to handle two dots * Update UI structure and styles * Make retry message translateable * Update snaphots * Fix lint error * Set initial file name * Remove devOnly flag from audio block * Increase tap target of button * Add 1px between title and subtitle * Align title, subtitle and button vertically * On iOS use VideoPlayer to play audio files in-app * Revert "On iOS use VideoPlayer to play audio files in-app" This reverts commit b1eb8dd. * [RNMobile] Audio Block - Cancel and Retry Dialog (#28540) Co-authored-by: Ceyhun Ozugur <[email protected]> * Rename button title * Add pill-shaped background to button * Add padding to title * Fix file title being empty when selecting from media library * Use safeDecodeURI * On iOS use VideoPlayer to play audio files in-app * Decrease button font size Co-authored-by: Joel Dean <[email protected]> * Revert "Remove devOnly flag from audio block" This reverts commit e6b5b6d. * Add right padding to title container to fix error state Co-authored-by: Joel Dean <[email protected]> Co-authored-by: Ceyhun Ozugur <[email protected]>
* Release script: Update react-native-editor version to 1.46.0 * Release script: Update with changes from 'npm run core preios' * [RNMobile] Correct condition in setting blockWidth (#28635) * [RNMobile] Fix height of the bottom-sheet after using back button (#28632) * remove unused param and setHeight even if it is the same * [RNMobile] Audio Player UI for audio block (#27467) * Basics of Audio block working * Add audio support to MediaUpload * Add handling of file uploads and replace * WPMediaLibrary support for Audio block * Avoid removing media info on error state * Linting * Added an AUDIO file to the test requestMediaPickFromMediaLibrary func * Fixed typo in ToolbarButton of Audio Block. * Removed auto help behavior present on web that's not used on mobile. * [Android] Wired the click of the Audio Media Library button. * Added Audio media options for choosing audio file locally. * [RNMobile] Audio Block: Proper caption field (#27689) * Audio Player UI for audio block * Add extension to styles import * Show file name while loading and retry message on error * Pass state props to audio player component * Implement placeholder-ish player UI structure * added styles for icon, title and subtitle * added blue-wordpress as link color. * added sizing based on design specs. * Fixed object destructuring error. * added icon styling state for upload in progress or upload failed. * implemented error style and behavior. * added styling for upload failed text. * Override MediaUploadProgress styles * Update filename extension spseration to handle two dots * Update UI structure and styles * Make retry message translateable * Update snaphots * Fix lint error * Set initial file name * Remove devOnly flag from audio block * Increase tap target of button * Add 1px between title and subtitle * Align title, subtitle and button vertically * On iOS use VideoPlayer to play audio files in-app * Revert "On iOS use VideoPlayer to play audio files in-app" This reverts commit b1eb8dd. * [RNMobile] Audio Block - Cancel and Retry Dialog (#28540) Co-authored-by: Ceyhun Ozugur <[email protected]> * Rename button title * Add pill-shaped background to button * Add padding to title * Fix file title being empty when selecting from media library * Use safeDecodeURI * On iOS use VideoPlayer to play audio files in-app * Decrease button font size Co-authored-by: Joel Dean <[email protected]> * Revert "Remove devOnly flag from audio block" This reverts commit e6b5b6d. * Add right padding to title container to fix error state Co-authored-by: Joel Dean <[email protected]> Co-authored-by: Ceyhun Ozugur <[email protected]> * [RNMobile] Enable Audio Block (#28594) Co-authored-by: Joel Dean <[email protected]> * Update react-native-editor CHANGELOG * Update unsupported block test data for UI tests (#28649) The unsupported block now used is jetpack/gif since Audio block is now supported. * Revert indentation of modified package.json files * Update react-native-editor CHANGELOG * [RNMobile] Move custom indicator to be rendered above the colors (#28698) * [RNMobile] Move back disabled style for button in File block (#28704) Co-authored-by: Luke Walczak <[email protected]> Co-authored-by: Drapich Piotr <[email protected]> Co-authored-by: etoledom <[email protected]> Co-authored-by: Joel Dean <[email protected]> Co-authored-by: Ceyhun Ozugur <[email protected]>









Gutenberg Mobilewordpress-mobile/gutenberg-mobile#3037WPAndroidwordpress-mobile/WordPress-Android#13843WPiOSwordpress-mobile/WordPress-iOS#15694Description
This PR adds a placeholder-ish Audio Player UI for the Audio Block.
How has this been tested?
Screenshots
Success
Uploading
Error
Checklist: