-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[RNMobile] Audio block capability now enables/disables media upload's media sources #31966
Conversation
66511fa
to
f4aa392
Compare
@fluiddot sorry for the long commit history. I had branched from another PR so I think because of the |
This is ready for another review @fluiddot Thanks! 🙇🏾 |
...droid/react-native-bridge/src/main/java/org/wordpress/mobile/WPAndroidGlue/GutenbergProps.kt
Outdated
Show resolved
Hide resolved
@jd-alexander I've spotted an error when uploading an audio file via the "Choose from device" option, it happened on the installable builds of both platforms iOS - build Steps:
audio-block-device-file-error.mp4 |
Thanks for spotting. I will address this in another PR. It's a simple fix that involves reverting to the current URL parsing logic for the time being and adding a test case for the URLs that caused this issue so they will be covered so there isn't a future occurrence of this. |
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.
I've just added a couple of comments related to formatting, apart from that from my side the PR is ready to go. Nevertheless, before giving the approval, I'd like to run a final test but I won't be able to do that before next Monday, sorry 🙏 .
packages/react-native-editor/ios/GutenbergDemo/GutenbergViewController.swift
Outdated
Show resolved
Hide resolved
No problem @fluiddot . I will update all the PRs so you can do your final tests then. Thanks much for the reviews so far. They have been thorough and helpful 🙇🏾♂️ |
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.
LGTM 🎊 !
- Tested on iPhone 11 (iOS 14.2) with build
49354
- Tested on Samsung Galaxy S20 FE 5G (Android 10) with build
108007
# Conflicts: # packages/react-native-editor/CHANGELOG.md
# Conflicts: # packages/react-native-editor/CHANGELOG.md
# Conflicts: # packages/react-native-editor/CHANGELOG.md
WordPress iOS
wordpress-mobile/WordPress-iOS#16533WordPress Android
wordpress-mobile/WordPress-Android#14680Gutenberg Mobile
wordpress-mobile/gutenberg-mobile#3523Description
The
Insert from URL
functionality that is available on free plans was added in #27817 This PR removes the hook that was showing/hiding the Audio block depending on the value from the Audio block capability that is passed over the bridge. Now that capability is utilized in the main apps to determine which media upload options are shown.Manual Testing
WP.com sites
Note:
Support for Audio block audio uploads, and access to the media library is restricted to sites that are on a paid plan.
Free Plan
Insert from URL
option is available as users aren't able to upload audio on free plans.Paid plan
Self hosted sites
Automated Testing
npm run test-unit:native
to verify themedia-upload
(file) test changes.I did not add any additional tests in this file since it doesn't seem like we are utilizing
enzyme
in our newer tests.Regression
Note
The blocks Cover, File, Gallery, Video, Media & Text all utilise the
media-upload
component that was modified. Verify that these blocks function as expected and all the relevant options are still shown for them in both paid and free plans.Types of changes
capability
was accessed within themedia-upload
component and utilized within a filter to determine which media options should be shown whether the capability is enabled or not.Checklist:
*.native.js
files for terms that need renaming or removal).