Add filter to allow more video types in AMP story background selection#3171
Add filter to allow more video types in AMP story background selection#3171swissspidy merged 10 commits intoampproject:developfrom thrijith:add/filter-forvideo-extension
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
@thrijith I took the liberty to make some changes directly so you can better see the direction I think this should take. Let me know what you think. I also saw that we still have many areas where |
Thanks @swissspidy 🙂, I was not sure, how to go about adding changes needed for store.
Are you referring the accept part? amp-wp/assets/src/stories-editor/components/custom-video-block-edit.js Lines 234 to 237 in 217d592 It doesn't allow selection for other extensions when using |
index 412fabac..d45a5580 100644
--- a/assets/src/stories-editor/components/custom-video-block-edit.js
+++ b/assets/src/stories-editor/components/custom-video-block-edit.js
@@ -235,7 +235,7 @@ class CustomVideoBlockEdit extends Component {
className={ className }
onSelect={ onSelectVideo }
onSelectURL={ this.onSelectURL }
- accept="video/mp4"
+ accept={ allowedVideoMimeTypes.join( ',' ) }
allowedTypes={ allowedVideoMimeTypes }
value={ this.props.attributes }
notices={ noticeUI }
diff --git a/assets/src/stories-editor/components/with-enforced-video-upload-type.js b/assets/src/stories-editor/components/with-enforced-video-upload-type.js
index 39c49239..7520580c 100644
--- a/assets/src/stories-editor/components/with-enforced-video-upload-type.js
+++ b/assets/src/stories-editor/components/with-enforced-video-upload-type.js
@@ -14,11 +14,11 @@ import PropTypes from 'prop-types';
*/
export default ( InitialMediaPlaceholder ) => {
const withEnforcedVideoUploadType = ( props ) => {
- const { accept, className } = props;
+ const { accept, className, allowedTypes } = props;
let newProps = { ...props };
if ( 'wp-block-video' === className && 'video/*' === accept ) {
- newProps = { ...props, accept: 'video/mp4' };
+ newProps = { ...props, accept: allowedTypes.join( ',' ) };
}
return <InitialMediaPlaceholder { ...newProps } />;
@@ -27,6 +27,7 @@ export default ( InitialMediaPlaceholder ) => {
withEnforcedVideoUploadType.propTypes = {
accept: PropTypes.string,
className: PropTypes.string,
+ allowedTypes: PropTypes.arrayOf( PropTypes.string ).isRequired,
};
return withEnforcedVideoUploadType;Since allowed video type is available in props, we could do something like this to remove usage of hardcoded mime type from custom video block element. |
|
Otherwise your suggested change looks good 👍 |
|
I have updated the |
swissspidy
left a comment
There was a problem hiding this comment.
Approved from my side, but requesting 1 more review as I did add lots of changes myself here.
…ode-support * 'develop' of github.com:ampproject/amp-wp: (28 commits) Exclude development files from production build ZIPs Add filter to allow more video types in AMP story background se… (#3171) Update dependency @babel/cli to v7.6.0 (#3203) Update dependency @babel/core to v7.6.0 (#3204) Remove deprecated AMP_WP_Utils class Refresh Composer lock file Run build in a temporary folder Add shell script to clean up the current folder and then trigger a build Adapt Gruntfile to fetch optimized Composer dependencies within the build folder Retrieve Sabberworm patch directly from the GitHub PR Adding regression test for vertical text alignment issues Fixing minor errors in test descriptions Only restrict height inside non-fit text blocks Rewritten block size tests Limit height declaration to only direct descendant Remove now obsolete withEnforcedVideoUploadType HOC Update eslint-plugin-jest and apply fixes (#3192) Update dependency postcss to v7.0.18 (#3200) Update dependency terser-webpack-plugin to v2 (#3195) Update dependency webpack-cli to v3.3.8 (#3194) ...
Fixes #3168