From 1cfcf9dd88bbed68375b66d9f906c9c0cef12945 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 16:10:01 +0800 Subject: [PATCH 1/6] Add MediaUploadCheck component and unit tests - Hardcode hasUploadPermissions value to true for now until implementation of server-side and redux state has been finalised Co-authored-by: imath --- packages/editor/src/components/index.js | 1 + .../src/components/media-upload/check.js | 10 +++ .../src/components/media-upload/test/check.js | 62 +++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 packages/editor/src/components/media-upload/check.js create mode 100644 packages/editor/src/components/media-upload/test/check.js diff --git a/packages/editor/src/components/index.js b/packages/editor/src/components/index.js index 3e909f4f784f6..d73f240ee12cc 100644 --- a/packages/editor/src/components/index.js +++ b/packages/editor/src/components/index.js @@ -22,6 +22,7 @@ export { default as RichText } from './rich-text'; export { default as RichTextProvider } from './rich-text/provider'; export { default as MediaPlaceholder } from './media-placeholder'; export { default as MediaUpload } from './media-upload'; +export { default as MediaUploadCheck } from './media-upload/check'; export { default as URLInput } from './url-input'; export { default as URLInputButton } from './url-input/button'; diff --git a/packages/editor/src/components/media-upload/check.js b/packages/editor/src/components/media-upload/check.js new file mode 100644 index 0000000000000..80ad2b20d2c87 --- /dev/null +++ b/packages/editor/src/components/media-upload/check.js @@ -0,0 +1,10 @@ +// Hardcode hasUploadPermissions to true for now - in future this value will be +// correctly populated from application state. +// See github issue for more information: +// https://github.com/WordPress/gutenberg/issues/3672 +export function MediaUploadCheck( { hasUploadPermissions = true, fallback, children } ) { + const optionalFallback = fallback || null; + return hasUploadPermissions ? children : optionalFallback; +} + +export default MediaUploadCheck; diff --git a/packages/editor/src/components/media-upload/test/check.js b/packages/editor/src/components/media-upload/test/check.js new file mode 100644 index 0000000000000..0fb81670965bc --- /dev/null +++ b/packages/editor/src/components/media-upload/test/check.js @@ -0,0 +1,62 @@ +/** + * External dependencies + */ +import { shallow } from 'enzyme'; + +/** + * Internal dependencies + */ +import { MediaUploadCheck } from '../check.js'; + +describe( 'MediaUploadCheck', () => { + it( 'renders its child if hasUploadPermissions is true', () => { + const wrapper = shallow( + +
Child
+
+ ); + + expect( wrapper.children() ).toHaveLength( 1 ); + expect( wrapper.children().first().text() ).toBe( 'Child' ); + } ); + + it( 'renders its child if hasUploadPermissions is true and a fallback is provided', () => { + const fallback = ( +
Fallback
+ ); + + const wrapper = shallow( + +
Child
+
+ ); + + expect( wrapper.children() ).toHaveLength( 1 ); + expect( wrapper.children().first().text() ).toBe( 'Child' ); + } ); + + it( 'renders the fallback if hasUploadPermissions is false and a fallback is provided', () => { + const fallback = ( +
Fallback
+ ); + + const wrapper = shallow( + +
Child
+
+ ); + + expect( wrapper.children() ).toHaveLength( 1 ); + expect( wrapper.children().first().text() ).toBe( 'Fallback' ); + } ); + + it( 'renders nothing if hasUploadPermissions is false and no fallback is provided', () => { + const wrapper = shallow( + +
Child
+
+ ); + + expect( wrapper.children() ).toHaveLength( 0 ); + } ); +} ); From 4212968e2805a72da08f629adc5171845b5dca82 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 16:30:26 +0800 Subject: [PATCH 2/6] Use MediaUploadCheck in MediaPlaceholder MediaUploadCheck component will conditionally show/hide parts of the UI related to file uploads dependent on whether the user has the capability to upload files. Co-authored-by: imath --- .../src/components/media-placeholder/index.js | 55 ++++++++++--------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index f30e68d1cfb6e..0e9207c1cd3f2 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -20,6 +20,7 @@ import { Component } from '@wordpress/element'; * Internal dependencies */ import MediaUpload from '../media-upload'; +import MediaUploadCheck from '../media-upload/check'; import { mediaUpload } from '../../utils/'; class MediaPlaceholder extends Component { @@ -96,10 +97,12 @@ class MediaPlaceholder extends Component { className={ classnames( 'editor-media-placeholder', className ) } notices={ notices } > - + + + { onSelectURL && (
) } - - { __( 'Upload' ) } - - ( - - ) } - /> + + + { __( 'Upload' ) } + + ( + + ) } + /> + ); } From d597daf6d9446441c99ec5b170558033e58e07bc Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 16:52:58 +0800 Subject: [PATCH 3/6] Simplify rendering logic of PostFeaturedImage Use early return when no featuredImageId is set. Reduce boolean checks around component rendering. Co-authored-by: imath --- .../components/post-featured-image/index.js | 88 ++++++++++--------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/packages/editor/src/components/post-featured-image/index.js b/packages/editor/src/components/post-featured-image/index.js index 849e49ec2d713..632bcdd6cf080 100644 --- a/packages/editor/src/components/post-featured-image/index.js +++ b/packages/editor/src/components/post-featured-image/index.js @@ -17,12 +17,33 @@ import { withSelect, withDispatch } from '@wordpress/data'; */ import PostFeaturedImageCheck from './check'; import MediaUpload from '../media-upload'; +import MediaUploadCheck from '../media-upload/check'; // Used when labels from post type were not yet loaded or when they are not present. const DEFAULT_SET_FEATURE_IMAGE_LABEL = __( 'Set featured image' ); const DEFAULT_REMOVE_FEATURE_IMAGE_LABEL = __( 'Remove image' ); function PostFeaturedImage( { currentPostId, featuredImageId, onUpdateImage, onRemoveImage, media, postType } ) { + if ( ! featuredImageId ) { + return ( + +
+ ( + + ) } + /> +
+
+ ); + } + const postLabel = get( postType, [ 'labels' ], {} ); let mediaWidth, mediaHeight, mediaSourceUrl; @@ -42,60 +63,43 @@ function PostFeaturedImage( { currentPostId, featuredImageId, onUpdateImage, onR return (
- { !! featuredImageId && - ( - - ) } - /> - } - { !! featuredImageId && media && ! media.isLoading && ( - ) } /> + { media && ! media.isLoading && + ( + + ) } + /> } - { ! featuredImageId && -
- ( - - ) } - /> -
- } - { !! featuredImageId && + - } +
); From ca337fac5fe891a7adb9d3472aeee92af0592fe4 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 17:25:24 +0800 Subject: [PATCH 4/6] Hard code hasUploadPermissions in redux state instead of MediaUploadCheck component Co-authored-by: imath --- docs/data/data-core.md | 8 ++++++++ packages/core-data/src/selectors.js | 13 +++++++++++++ .../editor/src/components/media-upload/check.js | 17 +++++++++++------ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/docs/data/data-core.md b/docs/data/data-core.md index 39f89c0c2c6ca..f258764c904a1 100644 --- a/docs/data/data-core.md +++ b/docs/data/data-core.md @@ -175,6 +175,14 @@ get back from the oEmbed preview API. Is the preview for the URL an oEmbed link fallback. +### hasUploadPermissions + +Return whether the user has media upload permissions. + +*Returns* + +Does the user have media upload permissions? + ## Actions ### receiveTerms diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js index 00af63aa6e147..e0fabbdd1a825 100644 --- a/packages/core-data/src/selectors.js +++ b/packages/core-data/src/selectors.js @@ -238,3 +238,16 @@ export function isPreviewEmbedFallback( state, url ) { } return preview.html === oEmbedLinkCheck; } + +/* + * Return whether the user has media upload permissions. + * + * @return {boolean} Does the user have media upload permissions? + */ +export function hasUploadPermissions() { + // Hardcode hasUploadPermissions to true for now - in future this value will be + // correctly populated from a REST API call that will store user permissions + // See github issue for more information: + // https://github.com/WordPress/gutenberg/issues/3672 + return true; +} diff --git a/packages/editor/src/components/media-upload/check.js b/packages/editor/src/components/media-upload/check.js index 80ad2b20d2c87..df9bab268a1d5 100644 --- a/packages/editor/src/components/media-upload/check.js +++ b/packages/editor/src/components/media-upload/check.js @@ -1,10 +1,15 @@ -// Hardcode hasUploadPermissions to true for now - in future this value will be -// correctly populated from application state. -// See github issue for more information: -// https://github.com/WordPress/gutenberg/issues/3672 -export function MediaUploadCheck( { hasUploadPermissions = true, fallback, children } ) { +/** + * WordPress dependencies + */ +import { withSelect } from '@wordpress/data'; + +export function MediaUploadCheck( { hasUploadPermissions, fallback, children } ) { const optionalFallback = fallback || null; return hasUploadPermissions ? children : optionalFallback; } -export default MediaUploadCheck; +export default withSelect( ( select ) => { + return { + hasUploadPermissions: select( 'core' ).hasUploadPermissions(), + }; +} )( MediaUploadCheck ); From d1907b0008c5ed98f6950849618d07e17308f132 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 18:42:24 +0800 Subject: [PATCH 5/6] Implement MediaUploadCheck wherever a user might interact with file uploads Co-authored-by: imath --- .../block-library/src/cover-image/index.js | 33 ++--- packages/block-library/src/file/edit.js | 33 ++--- packages/block-library/src/gallery/edit.js | 63 ++++----- packages/block-library/src/image/edit.js | 29 +++-- .../src/components/media-placeholder/index.js | 25 +++- .../components/post-featured-image/index.js | 123 +++++++++++------- 6 files changed, 181 insertions(+), 125 deletions(-) diff --git a/packages/block-library/src/cover-image/index.js b/packages/block-library/src/cover-image/index.js index d692ebb5be04a..e40d7b47ac146 100644 --- a/packages/block-library/src/cover-image/index.js +++ b/packages/block-library/src/cover-image/index.js @@ -17,6 +17,7 @@ import { BlockAlignmentToolbar, MediaPlaceholder, MediaUpload, + MediaUploadCheck, AlignmentToolbar, RichText, } from '@wordpress/editor'; @@ -130,21 +131,23 @@ export const settings = { setAttributes( { contentAlign: nextAlign } ); } } /> - - ( - - ) } - /> - + + + ( + + ) } + /> + + { !! url && ( diff --git a/packages/block-library/src/file/edit.js b/packages/block-library/src/file/edit.js index 9a7eb417345b6..e70933e4d396f 100644 --- a/packages/block-library/src/file/edit.js +++ b/packages/block-library/src/file/edit.js @@ -18,6 +18,7 @@ import { withSelect } from '@wordpress/data'; import { Component, Fragment } from '@wordpress/element'; import { MediaUpload, + MediaUploadCheck, MediaPlaceholder, BlockControls, RichText, @@ -171,21 +172,23 @@ class FileEdit extends Component { } } /> - - ( - - ) } - /> - + + + ( + + ) } + /> + +
diff --git a/packages/block-library/src/gallery/edit.js b/packages/block-library/src/gallery/edit.js index 5190799474f11..4a592da36f5f5 100644 --- a/packages/block-library/src/gallery/edit.js +++ b/packages/block-library/src/gallery/edit.js @@ -22,6 +22,7 @@ import { import { BlockControls, MediaUpload, + MediaUploadCheck, MediaPlaceholder, InspectorControls, mediaUpload, @@ -165,23 +166,25 @@ class GalleryEdit extends Component { const controls = ( { !! images.length && ( - - img.id ) } - render={ ( { open } ) => ( - - ) } - /> - + + + img.id ) } + render={ ( { open } ) => ( + + ) } + /> + + ) } ); @@ -252,18 +255,20 @@ class GalleryEdit extends Component { ) ) } { isSelected && -
  • - - { __( 'Upload an image' ) } - -
  • + +
  • + + { __( 'Upload an image' ) } + +
  • +
    } diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 2ae93346c923c..7b77b34593592 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -35,6 +35,7 @@ import { InspectorControls, MediaPlaceholder, MediaUpload, + MediaUploadCheck, BlockAlignmentToolbar, mediaUpload, } from '@wordpress/editor'; @@ -219,19 +220,21 @@ class ImageEdit extends Component { /> - ( - - ) } - /> + + ( + + ) } + /> + ); diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index 0e9207c1cd3f2..5c4cf757101db 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -15,6 +15,7 @@ import { } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; import { Component } from '@wordpress/element'; +import { withSelect } from '@wordpress/data'; /** * Internal dependencies @@ -23,7 +24,7 @@ import MediaUpload from '../media-upload'; import MediaUploadCheck from '../media-upload/check'; import { mediaUpload } from '../../utils/'; -class MediaPlaceholder extends Component { +export class MediaPlaceholder extends Component { constructor() { super( ...arguments ); this.state = { @@ -86,14 +87,24 @@ class MediaPlaceholder extends Component { onHTMLDrop = noop, multiple = false, notices, + hasUploadPermissions, } = this.props; + let instructions; + if ( hasUploadPermissions ) { + instructions = sprintf( __( 'Drag %s, upload a new one, or select a file from your library.' ), labels.name ); + } else if ( onSelectURL ) { + instructions = sprintf( __( 'Given your current role, you can only link %s, you cannot upload.' ), labels.name ); + } else { + instructions = __( 'To edit this block, you need permission to upload media.' ); + } + return ( @@ -148,4 +159,12 @@ class MediaPlaceholder extends Component { } } -export default MediaPlaceholder; +export default withSelect( ( select ) => { + const { + hasUploadPermissions, + } = select( 'core' ); + + return { + hasUploadPermissions: hasUploadPermissions(), + }; +} )( MediaPlaceholder ); diff --git a/packages/editor/src/components/post-featured-image/index.js b/packages/editor/src/components/post-featured-image/index.js index 632bcdd6cf080..47fb7c44a3fe9 100644 --- a/packages/editor/src/components/post-featured-image/index.js +++ b/packages/editor/src/components/post-featured-image/index.js @@ -11,6 +11,7 @@ import { applyFilters } from '@wordpress/hooks'; import { Button, Spinner, ResponsiveWrapper, withFilters } from '@wordpress/components'; import { compose } from '@wordpress/compose'; import { withSelect, withDispatch } from '@wordpress/data'; +import { Fragment } from '@wordpress/element'; /** * Internal dependencies @@ -23,29 +24,45 @@ import MediaUploadCheck from '../media-upload/check'; const DEFAULT_SET_FEATURE_IMAGE_LABEL = __( 'Set featured image' ); const DEFAULT_REMOVE_FEATURE_IMAGE_LABEL = __( 'Remove image' ); +const PostFeaturedImageContainer = ( { mediaUploadCheckFallback, children } ) => ( + + +
    + { children } +
    +
    +
    +); + function PostFeaturedImage( { currentPostId, featuredImageId, onUpdateImage, onRemoveImage, media, postType } ) { + const postLabel = get( postType, [ 'labels' ], {} ); + if ( ! featuredImageId ) { + const fallback = ( +

    { __( 'To edit the featured image, you need permission to upload media.' ) }

    + ); + return ( - -
    - ( - - ) } - /> -
    -
    + + ( + + ) } + /> + ); } - const postLabel = get( postType, [ 'labels' ], {} ); - let mediaWidth, mediaHeight, mediaSourceUrl; if ( media ) { const mediaSize = applyFilters( 'editor.PostFeaturedImage.imageSize', 'post-thumbnail', media.id, currentPostId ); @@ -60,48 +77,54 @@ function PostFeaturedImage( { currentPostId, featuredImageId, onUpdateImage, onR } } + const imagePreview = ( + + { media && + + { + + } + { ! media && } + + ); + return ( - -
    + + ( + + ) } + /> + { media && ! media.isLoading && ( - ) } /> - { media && ! media.isLoading && - ( - - ) } - /> - } - - - -
    -
    + } + + + + ); } From d537404c4b1ac4f6c2b296856da7b501c42ccc6f Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 22 Aug 2018 19:17:21 +0800 Subject: [PATCH 6/6] Remove tests that can no longer be run due to use of withSelect in nested component --- .../audio/test/__snapshots__/index.js.snap | 107 ------------------ .../block-library/src/audio/test/index.js | 13 --- .../test/__snapshots__/index.js.snap | 92 --------------- .../src/cover-image/test/index.js | 13 --- .../gallery/test/__snapshots__/index.js.snap | 93 --------------- .../block-library/src/gallery/test/index.js | 13 --- .../video/test/__snapshots__/index.js.snap | 107 ------------------ .../block-library/src/video/test/index.js | 13 --- 8 files changed, 451 deletions(-) delete mode 100644 packages/block-library/src/audio/test/__snapshots__/index.js.snap delete mode 100644 packages/block-library/src/audio/test/index.js delete mode 100644 packages/block-library/src/cover-image/test/__snapshots__/index.js.snap delete mode 100644 packages/block-library/src/cover-image/test/index.js delete mode 100644 packages/block-library/src/gallery/test/__snapshots__/index.js.snap delete mode 100644 packages/block-library/src/gallery/test/index.js delete mode 100644 packages/block-library/src/video/test/__snapshots__/index.js.snap delete mode 100644 packages/block-library/src/video/test/index.js diff --git a/packages/block-library/src/audio/test/__snapshots__/index.js.snap b/packages/block-library/src/audio/test/__snapshots__/index.js.snap deleted file mode 100644 index 71c38018c590f..0000000000000 --- a/packages/block-library/src/audio/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,107 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`core/audio block edit matches snapshot 1`] = ` -
    -
    - - Audio -
    -
    - Drag an audio, upload a new one or select a file from your library. -
    -
    -
    -
    - - - Drop files to upload - -
    -
    -
    - - -
    -
    - - -
    -
    -
    -`; diff --git a/packages/block-library/src/audio/test/index.js b/packages/block-library/src/audio/test/index.js deleted file mode 100644 index a5611a3dd8ce6..0000000000000 --- a/packages/block-library/src/audio/test/index.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Internal dependencies - */ -import { name, settings } from '../'; -import { blockEditRender } from '../../test/helpers'; - -describe( 'core/audio', () => { - test( 'block edit matches snapshot', () => { - const wrapper = blockEditRender( name, settings ); - - expect( wrapper ).toMatchSnapshot(); - } ); -} ); diff --git a/packages/block-library/src/cover-image/test/__snapshots__/index.js.snap b/packages/block-library/src/cover-image/test/__snapshots__/index.js.snap deleted file mode 100644 index f6690fdae6fe9..0000000000000 --- a/packages/block-library/src/cover-image/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,92 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`core/cover-image block edit matches snapshot 1`] = ` -
    -
    - - Cover Image -
    -
    - Drag an image, upload a new one or select a file from your library. -
    -
    -
    -
    - - - Drop files to upload - -
    -
    -
    - - -
    -
    -
    -`; diff --git a/packages/block-library/src/cover-image/test/index.js b/packages/block-library/src/cover-image/test/index.js deleted file mode 100644 index ceb0bc9d5123c..0000000000000 --- a/packages/block-library/src/cover-image/test/index.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Internal dependencies - */ -import { name, settings } from '../'; -import { blockEditRender } from '../../test/helpers'; - -describe( 'core/cover-image', () => { - test( 'block edit matches snapshot', () => { - const wrapper = blockEditRender( name, settings ); - - expect( wrapper ).toMatchSnapshot(); - } ); -} ); diff --git a/packages/block-library/src/gallery/test/__snapshots__/index.js.snap b/packages/block-library/src/gallery/test/__snapshots__/index.js.snap deleted file mode 100644 index fa8820564390d..0000000000000 --- a/packages/block-library/src/gallery/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,93 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`core/gallery block edit matches snapshot 1`] = ` - -`; diff --git a/packages/block-library/src/gallery/test/index.js b/packages/block-library/src/gallery/test/index.js deleted file mode 100644 index 5731cc00ba483..0000000000000 --- a/packages/block-library/src/gallery/test/index.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Internal dependencies - */ -import { name, settings } from '../'; -import { blockEditRender } from '../../test/helpers'; - -describe( 'core/gallery', () => { - test( 'block edit matches snapshot', () => { - const wrapper = blockEditRender( name, settings ); - - expect( wrapper ).toMatchSnapshot(); - } ); -} ); diff --git a/packages/block-library/src/video/test/__snapshots__/index.js.snap b/packages/block-library/src/video/test/__snapshots__/index.js.snap deleted file mode 100644 index 33d0d470ec7da..0000000000000 --- a/packages/block-library/src/video/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,107 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`core/video block edit matches snapshot 1`] = ` -
    -
    - - Video -
    -
    - Drag a video, upload a new one or select a file from your library. -
    -
    -
    -
    - - - Drop files to upload - -
    -
    -
    - - -
    -
    - - -
    -
    -
    -`; diff --git a/packages/block-library/src/video/test/index.js b/packages/block-library/src/video/test/index.js deleted file mode 100644 index a947f278a5882..0000000000000 --- a/packages/block-library/src/video/test/index.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Internal dependencies - */ -import { name, settings } from '../'; -import { blockEditRender } from '../../test/helpers'; - -describe( 'core/video', () => { - test( 'block edit matches snapshot', () => { - const wrapper = blockEditRender( name, settings ); - - expect( wrapper ).toMatchSnapshot(); - } ); -} );