From 5891e47a5be56ed4977f3ba320939e66b147ad9d Mon Sep 17 00:00:00 2001 From: Nik Tsekouras Date: Tue, 7 Jul 2020 18:22:43 +0300 Subject: [PATCH] Refactor MediaPlaceholder to function component (#23671) * refactor MediaPlaceholder * fix test * refactor MediaUploadCheck * fix multiple upload * Fix dependency in useSelect Co-authored-by: Zebulan Stanphill * Fix dependency in useSelect Co-authored-by: Zebulan Stanphill * Use optional chaining Co-authored-by: Zebulan Stanphill * minor refactor Co-authored-by: Zebulan Stanphill * minor refactor Co-authored-by: Zebulan Stanphill * minor refactor * temp e2e fix test Co-authored-by: Zebulan Stanphill --- .../src/components/media-placeholder/index.js | 357 +++++++----------- .../media-placeholder/test/index.js | 5 +- .../src/components/media-upload/check.js | 20 +- .../editor/various/block-deletion.test.js | 1 + 4 files changed, 151 insertions(+), 232 deletions(-) diff --git a/packages/block-editor/src/components/media-placeholder/index.js b/packages/block-editor/src/components/media-placeholder/index.js index 9387d36d08587..146784e7b3073 100644 --- a/packages/block-editor/src/components/media-placeholder/index.js +++ b/packages/block-editor/src/components/media-placeholder/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { every, get, isArray, noop, startsWith } from 'lodash'; +import { noop } from 'lodash'; import classnames from 'classnames'; /** @@ -15,9 +15,8 @@ import { withFilters, } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; -import { Component } from '@wordpress/element'; -import { compose } from '@wordpress/compose'; -import { withSelect } from '@wordpress/data'; +import { useState, useEffect } from '@wordpress/element'; +import { useSelect } from '@wordpress/data'; import deprecated from '@wordpress/deprecated'; import { keyboardReturn } from '@wordpress/icons'; @@ -52,84 +51,74 @@ const InsertFromURLPopover = ( { src, onChange, onSubmit, onClose } ) => ( ); -export class MediaPlaceholder extends Component { - constructor() { - super( ...arguments ); - this.state = { - src: '', - isURLInputVisible: false, - }; - this.onChangeSrc = this.onChangeSrc.bind( this ); - this.onSubmitSrc = this.onSubmitSrc.bind( this ); - this.onUpload = this.onUpload.bind( this ); - this.onFilesUpload = this.onFilesUpload.bind( this ); - this.openURLInput = this.openURLInput.bind( this ); - this.closeURLInput = this.closeURLInput.bind( this ); - } - - onlyAllowsImages() { - const { allowedTypes } = this.props; - if ( ! allowedTypes ) { - return false; - } - return every( allowedTypes, ( allowedType ) => { - return ( - allowedType === 'image' || startsWith( allowedType, 'image/' ) - ); - } ); - } - - componentDidMount() { - this.setState( { src: get( this.props.value, [ 'src' ], '' ) } ); - } +export function MediaPlaceholder( { + value = {}, + allowedTypes = [], + className, + icon, + labels = {}, + mediaPreview, + notices, + isAppender, + accept, + addToGallery, + multiple = false, + dropZoneUIOnly, + disableDropZone, + disableMediaButtons, + onError, + onSelect, + onCancel, + onSelectURL, + onDoubleClick, + onFilesPreUpload = noop, + onHTMLDrop = noop, + children, +} ) { + const mediaUpload = useSelect( ( select ) => { + const { getSettings } = select( 'core/block-editor' ); + return getSettings().mediaUpload; + }, [] ); + const [ src, setSrc ] = useState( '' ); + const [ isURLInputVisible, setIsURLInputVisible ] = useState( false ); + + useEffect( () => { + setSrc( value?.src ?? '' ); + }, [ value ] ); + + const onlyAllowsImages = () => + allowedTypes.every( + ( allowedType ) => + allowedType === 'image' || allowedType.startsWith( 'image/' ) + ); - componentDidUpdate( prevProps ) { - if ( - get( prevProps.value, [ 'src' ], '' ) !== - get( this.props.value, [ 'src' ], '' ) - ) { - this.setState( { src: get( this.props.value, [ 'src' ], '' ) } ); - } - } + const onChangeSrc = ( event ) => { + setSrc( event.target.value ); + }; - onChangeSrc( event ) { - this.setState( { src: event.target.value } ); - } + const openURLInput = () => { + setIsURLInputVisible( true ); + }; + const closeURLInput = () => { + setIsURLInputVisible( false ); + }; - onSubmitSrc( event ) { + const onSubmitSrc = ( event ) => { event.preventDefault(); - if ( this.state.src && this.props.onSelectURL ) { - this.props.onSelectURL( this.state.src ); - this.closeURLInput(); + if ( src && onSelectURL ) { + onSelectURL( src ); + closeURLInput(); } - } - - onUpload( event ) { - this.onFilesUpload( event.target.files ); - } - - onFilesUpload( files ) { - const { - addToGallery, - allowedTypes, - mediaUpload, - multiple, - onError, - onSelect, - onFilesPreUpload = noop, - } = this.props; + }; + const onFilesUpload = ( files ) => { onFilesPreUpload( files ); let setMedia; if ( multiple ) { if ( addToGallery ) { - // To allow changes to a gallery to be made while uploads are in progress - // (including trigging multiple upload groups and removing already in place images), - // we must be able to add newMedia based on the current value of the Gallery - // whenever the setMedia function runs (not destructuring 'value' from props). - // Additionally, since the setMedia function runs multiple times per upload group + // Since the setMedia function runs multiple times per upload group // and is passed newMedia containing every item in its group each time, we must - // also filter out whatever this upload group had previously returned to the + // filter out whatever this upload group had previously returned to the // gallery before adding and returning the image array with replacement newMedia // values. @@ -138,22 +127,19 @@ export class MediaPlaceholder extends Component { setMedia = ( newMedia ) => { // Remove any images this upload group is responsible for (lastMediaPassed). // Their replacements are contained in newMedia. - const filteredMedia = ( this.props.value || [] ).filter( - ( item ) => { - // If Item has id, only remove it if lastMediaPassed has an item with that id. - if ( item.id ) { - return ! lastMediaPassed.some( - // Be sure to convert to number for comparison. - ( { id } ) => - Number( id ) === Number( item.id ) - ); - } - // Compare transient images via .includes since gallery may append extra info onto the url. - return ! lastMediaPassed.some( ( { urlSlug } ) => - item.url.includes( urlSlug ) + const filteredMedia = ( value ?? [] ).filter( ( item ) => { + // If Item has id, only remove it if lastMediaPassed has an item with that id. + if ( item.id ) { + return ! lastMediaPassed.some( + // Be sure to convert to number for comparison. + ( { id } ) => Number( id ) === Number( item.id ) ); } - ); + // Compare transient images via .includes since gallery may append extra info onto the url. + return ! lastMediaPassed.some( ( { urlSlug } ) => + item.url.includes( urlSlug ) + ); + } ); // Return the filtered media array along with newMedia. onSelect( filteredMedia.concat( newMedia ) ); // Reset lastMediaPassed and set it with ids and urls from newMedia. @@ -176,33 +162,14 @@ export class MediaPlaceholder extends Component { onFileChange: setMedia, onError, } ); - } - - openURLInput() { - this.setState( { isURLInputVisible: true } ); - } + }; - closeURLInput() { - this.setState( { isURLInputVisible: false } ); - } + const onUpload = ( event ) => { + onFilesUpload( event.target.files ); + }; - renderPlaceholder( content, onClick ) { - const { - allowedTypes = [], - className, - icon, - isAppender, - labels = {}, - onDoubleClick, - mediaPreview, - notices, - onSelectURL, - mediaUpload, - children, - } = this.props; - - let instructions = labels.instructions; - let title = labels.title; + const renderPlaceholder = ( content, onClick ) => { + let { instructions, title } = labels; if ( ! mediaUpload && ! onSelectURL ) { instructions = __( @@ -211,10 +178,11 @@ export class MediaPlaceholder extends Component { } if ( instructions === undefined || title === undefined ) { + const [ firstAllowedType ] = allowedTypes; const isOneType = 1 === allowedTypes.length; - const isAudio = isOneType && 'audio' === allowedTypes[ 0 ]; - const isImage = isOneType && 'image' === allowedTypes[ 0 ]; - const isVideo = isOneType && 'video' === allowedTypes[ 0 ]; + const isAudio = isOneType && 'audio' === firstAllowedType; + const isImage = isOneType && 'image' === firstAllowedType; + const isVideo = isOneType && 'video' === firstAllowedType; if ( instructions === undefined && mediaUpload ) { instructions = __( @@ -272,25 +240,19 @@ export class MediaPlaceholder extends Component { { children } ); - } - - renderDropZone() { - const { disableDropZone, onHTMLDrop = noop } = this.props; + }; + const renderDropZone = () => { if ( disableDropZone ) { return null; } return ( - + ); - } + }; - renderCancelLink() { - const { onCancel } = this.props; + const renderCancelLink = () => { return ( onCancel && ( - { isURLInputVisible && ( - - ) } - + onSelectURL && ( +
+ + { isURLInputVisible && ( + + ) } +
+ ) ); - } - - renderMediaUploadChecked() { - const { - accept, - addToGallery, - allowedTypes = [], - isAppender, - mediaUpload, - multiple = false, - onSelect, - value = {}, - } = this.props; + }; + const renderMediaUploadChecked = () => { const mediaLibraryButton = ( id ) : value.id + Array.isArray( value ) + ? value.map( ( { id } ) => id ) + : value.id } render={ ( { open } ) => { return ( @@ -374,9 +324,9 @@ export class MediaPlaceholder extends Component { if ( mediaUpload && isAppender ) { return ( <> - { this.renderDropZone() } + { renderDropZone() } { @@ -392,14 +342,11 @@ export class MediaPlaceholder extends Component { { __( 'Upload' ) } { mediaLibraryButton } - { this.renderUrlSelectionUI() } - { this.renderCancelLink() } + { renderUrlSelectionUI() } + { renderCancelLink() } ); - return this.renderPlaceholder( - content, - openFileDialog - ); + return renderPlaceholder( content, openFileDialog ); } } /> @@ -409,72 +356,50 @@ export class MediaPlaceholder extends Component { if ( mediaUpload ) { const content = ( <> - { this.renderDropZone() } + { renderDropZone() } { __( 'Upload' ) } { mediaLibraryButton } - { this.renderUrlSelectionUI() } - { this.renderCancelLink() } + { renderUrlSelectionUI() } + { renderCancelLink() } ); - return this.renderPlaceholder( content ); + return renderPlaceholder( content ); } - return this.renderPlaceholder( mediaLibraryButton ); - } - - render() { - const { disableMediaButtons, dropZoneUIOnly } = this.props; - - if ( dropZoneUIOnly || disableMediaButtons ) { - if ( dropZoneUIOnly ) { - deprecated( - 'wp.blockEditor.MediaPlaceholder dropZoneUIOnly prop', - { - alternative: 'disableMediaButtons', - } - ); - } + return renderPlaceholder( mediaLibraryButton ); + }; - return ( - { this.renderDropZone() } - ); + if ( dropZoneUIOnly || disableMediaButtons ) { + if ( dropZoneUIOnly ) { + deprecated( 'wp.blockEditor.MediaPlaceholder dropZoneUIOnly prop', { + alternative: 'disableMediaButtons', + } ); } - return ( - - { this.renderMediaUploadChecked() } - - ); + return { renderDropZone() }; } -} -const applyWithSelect = withSelect( ( select ) => { - const { getSettings } = select( 'core/block-editor' ); - - return { - mediaUpload: getSettings().mediaUpload, - }; -} ); + return ( + + { renderMediaUploadChecked() } + + ); +} /** * @see https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/media-placeholder/README.md */ -export default compose( - applyWithSelect, - withFilters( 'editor.MediaPlaceholder' ) -)( MediaPlaceholder ); +export default withFilters( 'editor.MediaPlaceholder' )( MediaPlaceholder ); diff --git a/packages/block-editor/src/components/media-placeholder/test/index.js b/packages/block-editor/src/components/media-placeholder/test/index.js index 32496cf65ddd5..5dacf563e56d7 100644 --- a/packages/block-editor/src/components/media-placeholder/test/index.js +++ b/packages/block-editor/src/components/media-placeholder/test/index.js @@ -9,11 +9,10 @@ import { mount } from 'enzyme'; import { MediaPlaceholder } from '../'; jest.mock( '../../media-upload/check', () => () => null ); +jest.mock( '@wordpress/data/src/components/use-select', () => () => ( {} ) ); describe( 'MediaPlaceholder', () => { it( 'renders successfully when allowedTypes property is not specified', () => { - expect( () => - mount( ) - ).not.toThrow(); + expect( () => mount( ) ).not.toThrow(); } ); } ); diff --git a/packages/block-editor/src/components/media-upload/check.js b/packages/block-editor/src/components/media-upload/check.js index 7059dfc3f196c..becb7f030949e 100644 --- a/packages/block-editor/src/components/media-upload/check.js +++ b/packages/block-editor/src/components/media-upload/check.js @@ -1,23 +1,17 @@ /** * WordPress dependencies */ -import { withSelect } from '@wordpress/data'; +import { useSelect } from '@wordpress/data'; -export function MediaUploadCheck( { - hasUploadPermissions, - fallback = null, - children, -} ) { +export function MediaUploadCheck( { fallback = null, children } ) { + const hasUploadPermissions = useSelect( ( select ) => { + const { getSettings } = select( 'core/block-editor' ); + return !! getSettings().mediaUpload; + }, [] ); return hasUploadPermissions ? children : fallback; } /** * @see https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/media-upload/README.md */ -export default withSelect( ( select ) => { - const { getSettings } = select( 'core/block-editor' ); - - return { - hasUploadPermissions: !! getSettings().mediaUpload, - }; -} )( MediaUploadCheck ); +export default MediaUploadCheck; diff --git a/packages/e2e-tests/specs/editor/various/block-deletion.test.js b/packages/e2e-tests/specs/editor/various/block-deletion.test.js index 56c5496e46151..b54348fab54fd 100644 --- a/packages/e2e-tests/specs/editor/various/block-deletion.test.js +++ b/packages/e2e-tests/specs/editor/various/block-deletion.test.js @@ -189,6 +189,7 @@ describe( 'deleting all blocks', () => { // Add and remove a block. await insertBlock( 'Image' ); + await page.waitForSelector( 'figure[data-type="core/image"]' ); await page.keyboard.press( 'Backspace' ); // Verify there is no selected block.