From 6c170961d07110bbb9a35c626da774b59424260c Mon Sep 17 00:00:00 2001 From: Ravi Chandra Date: Mon, 14 Sep 2020 19:54:16 +0530 Subject: [PATCH 1/5] Refactor GalleryImage to use React hooks --- .../src/gallery/gallery-image.js | 358 ++++++++---------- 1 file changed, 154 insertions(+), 204 deletions(-) diff --git a/packages/block-library/src/gallery/gallery-image.js b/packages/block-library/src/gallery/gallery-image.js index 411ea9114932bb..dc5cc91920a07b 100644 --- a/packages/block-library/src/gallery/gallery-image.js +++ b/packages/block-library/src/gallery/gallery-image.js @@ -7,7 +7,7 @@ import { get, omit } from 'lodash'; /** * WordPress dependencies */ -import { Component } from '@wordpress/element'; +import { useEffect, useRef, useState } from '@wordpress/element'; import { Button, Spinner, ButtonGroup } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { BACKSPACE, DELETE } from '@wordpress/keycodes'; @@ -30,105 +30,82 @@ import { pickRelevantMediaFiles } from './shared'; const isTemporaryImage = ( id, url ) => ! id && isBlobURL( url ); -class GalleryImage extends Component { - constructor() { - super( ...arguments ); +function GalleryImage( { + image, + url, + alt, + id, + linkTo, + link, + isFirstItem, + isLastItem, + isSelected, + caption, + sizeSlug, + onSelect, + onRemove, + onMoveForward, + onMoveBackward, + setAttributes, + 'aria-label': ariaLabel, + __unstableMarkNextChangeAsNotPersistent, +} ) { + const [ captionSelected, setCaptionSelected ] = useState( false ); + const [ isEditing, setIsEditing ] = useState( false ); - this.onSelectImage = this.onSelectImage.bind( this ); - this.onSelectCaption = this.onSelectCaption.bind( this ); - this.onRemoveImage = this.onRemoveImage.bind( this ); - this.bindContainer = this.bindContainer.bind( this ); - this.onEdit = this.onEdit.bind( this ); - this.onSelectImageFromLibrary = this.onSelectImageFromLibrary.bind( - this - ); - this.onSelectCustomURL = this.onSelectCustomURL.bind( this ); - this.state = { - captionSelected: false, - isEditing: false, - }; - } + const container = useRef( null ); - bindContainer( ref ) { - this.container = ref; - } - - onSelectCaption() { - if ( ! this.state.captionSelected ) { - this.setState( { - captionSelected: true, + useEffect( () => { + if ( image && ! url ) { + __unstableMarkNextChangeAsNotPersistent(); + setAttributes( { + url: image.source_url, + alt: image.alt_text, } ); } - if ( ! this.props.isSelected ) { - this.props.onSelect(); + // unselect the caption so when the user selects other image and comeback + // the caption is not immediately selected + if ( captionSelected && ! isSelected ) { + setCaptionSelected( false ); } - } + }, [ isSelected, image, url ] ); - onSelectImage() { - if ( ! this.props.isSelected ) { - this.props.onSelect(); + const onSelectCaption = () => { + if ( ! captionSelected ) { + setCaptionSelected( true ); } - if ( this.state.captionSelected ) { - this.setState( { - captionSelected: false, - } ); + if ( ! isSelected ) { + onSelect(); } - } + }; - onRemoveImage( event ) { - if ( - this.container === document.activeElement && - this.props.isSelected && - [ BACKSPACE, DELETE ].indexOf( event.keyCode ) !== -1 - ) { - event.stopPropagation(); - event.preventDefault(); - this.props.onRemove(); + const onSelectImage = () => { + if ( ! isSelected ) { + onSelect(); } - } - onEdit() { - this.setState( { - isEditing: true, - } ); - } - - componentDidUpdate( prevProps ) { - const { - isSelected, - image, - url, - __unstableMarkNextChangeAsNotPersistent, - } = this.props; - if ( image && ! url ) { - __unstableMarkNextChangeAsNotPersistent(); - this.props.setAttributes( { - url: image.source_url, - alt: image.alt_text, - } ); + if ( captionSelected ) { + setCaptionSelected( false ); } + }; - // unselect the caption so when the user selects other image and comeback - // the caption is not immediately selected + const onRemoveImage = ( event ) => { if ( - this.state.captionSelected && - ! isSelected && - prevProps.isSelected + container.current === document.activeElement && + isSelected && + [ BACKSPACE, DELETE ].indexOf( event.keyCode ) !== -1 ) { - this.setState( { - captionSelected: false, - } ); + event.stopPropagation(); + event.preventDefault(); + onRemove(); } - } + }; - deselectOnBlur() { - this.props.onDeselect(); - } + const onEdit = () => setIsEditing( true ); - onSelectImageFromLibrary( media ) { - const { setAttributes, id, url, alt, caption, sizeSlug } = this.props; + const onSelectImageFromLibrary = ( media ) => { if ( ! media || ! media.url ) { return; } @@ -150,142 +127,115 @@ class GalleryImage extends Component { } setAttributes( mediaAttributes ); - this.setState( { - isEditing: false, - } ); - } + setIsEditing( false ); + }; - onSelectCustomURL( newURL ) { - const { setAttributes, url } = this.props; + const onSelectCustomURL = ( newURL ) => { if ( newURL !== url ) { setAttributes( { url: newURL, id: undefined, } ); - this.setState( { - isEditing: false, - } ); + setIsEditing( false ); } - } + }; - render() { - const { - url, - alt, - id, - linkTo, - link, - isFirstItem, - isLastItem, - isSelected, - caption, - onRemove, - onMoveForward, - onMoveBackward, - setAttributes, - 'aria-label': ariaLabel, - } = this.props; - const { isEditing } = this.state; + let href; - let href; + switch ( linkTo ) { + case 'media': + href = url; + break; + case 'attachment': + href = link; + break; + } - switch ( linkTo ) { - case 'media': - href = url; - break; - case 'attachment': - href = link; - break; - } + const img = ( + // Disable reason: Image itself is not meant to be interactive, but should + // direct image selection and unfocus caption fields. + /* eslint-disable jsx-a11y/no-noninteractive-element-interactions */ + <> + { + { isBlobURL( url ) && } + + /* eslint-enable jsx-a11y/no-noninteractive-element-interactions */ + ); - const img = ( - // Disable reason: Image itself is not meant to be interactive, but should - // direct image selection and unfocus caption fields. - /* eslint-disable jsx-a11y/no-noninteractive-element-interactions */ - <> - { - { isBlobURL( url ) && } - - /* eslint-enable jsx-a11y/no-noninteractive-element-interactions */ - ); + const className = classnames( { + 'is-selected': isSelected, + 'is-transient': isBlobURL( url ), + } ); - const className = classnames( { - 'is-selected': isSelected, - 'is-transient': isBlobURL( url ), - } ); - - return ( -
- { ! isEditing && ( href ? { img } : img ) } - { isEditing && ( - - ) } - -
- ); - } + return ( +
+ { ! isEditing && ( href ? { img } : img ) } + { isEditing && ( + + ) } + +
+ ); } export default compose( [ From 00e6011e3b34ace7ffe828b04e1907cdfec85b00 Mon Sep 17 00:00:00 2001 From: Ravi Chandra Date: Mon, 14 Sep 2020 21:13:27 +0530 Subject: [PATCH 2/5] Replace withDispatch and withSelect with hooks --- .../src/gallery/gallery-image.js | 39 +++++++------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/packages/block-library/src/gallery/gallery-image.js b/packages/block-library/src/gallery/gallery-image.js index dc5cc91920a07b..67435dca0b3644 100644 --- a/packages/block-library/src/gallery/gallery-image.js +++ b/packages/block-library/src/gallery/gallery-image.js @@ -11,10 +11,9 @@ import { useEffect, useRef, useState } from '@wordpress/element'; import { Button, Spinner, ButtonGroup } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { BACKSPACE, DELETE } from '@wordpress/keycodes'; -import { withSelect, withDispatch } from '@wordpress/data'; +import { useDispatch, useSelect } from '@wordpress/data'; import { RichText, MediaPlaceholder } from '@wordpress/block-editor'; import { isBlobURL } from '@wordpress/blob'; -import { compose } from '@wordpress/compose'; import { closeSmall, chevronLeft, @@ -30,8 +29,7 @@ import { pickRelevantMediaFiles } from './shared'; const isTemporaryImage = ( id, url ) => ! id && isBlobURL( url ); -function GalleryImage( { - image, +export default function GalleryImage( { url, alt, id, @@ -48,13 +46,25 @@ function GalleryImage( { onMoveBackward, setAttributes, 'aria-label': ariaLabel, - __unstableMarkNextChangeAsNotPersistent, } ) { const [ captionSelected, setCaptionSelected ] = useState( false ); const [ isEditing, setIsEditing ] = useState( false ); const container = useRef( null ); + const image = useSelect( + ( select ) => { + const { getMedia } = select( 'core' ); + + return id ? getMedia( parseInt( id, 10 ) ) : null; + }, + [ id ] + ); + + const { __unstableMarkNextChangeAsNotPersistent } = useDispatch( + 'core/block-editor' + ); + useEffect( () => { if ( image && ! url ) { __unstableMarkNextChangeAsNotPersistent(); @@ -237,22 +247,3 @@ function GalleryImage( { ); } - -export default compose( [ - withSelect( ( select, ownProps ) => { - const { getMedia } = select( 'core' ); - const { id } = ownProps; - - return { - image: id ? getMedia( parseInt( id, 10 ) ) : null, - }; - } ), - withDispatch( ( dispatch ) => { - const { __unstableMarkNextChangeAsNotPersistent } = dispatch( - 'core/block-editor' - ); - return { - __unstableMarkNextChangeAsNotPersistent, - }; - } ), -] )( GalleryImage ); From a48be26489414dad804f7109cd6e9254d97f1126 Mon Sep 17 00:00:00 2001 From: Ravi Chandra Date: Tue, 15 Sep 2020 01:09:33 +0530 Subject: [PATCH 3/5] Add missing dep for hook --- packages/block-library/src/gallery/gallery-image.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/gallery/gallery-image.js b/packages/block-library/src/gallery/gallery-image.js index 67435dca0b3644..52023cfe80973d 100644 --- a/packages/block-library/src/gallery/gallery-image.js +++ b/packages/block-library/src/gallery/gallery-image.js @@ -79,7 +79,7 @@ export default function GalleryImage( { if ( captionSelected && ! isSelected ) { setCaptionSelected( false ); } - }, [ isSelected, image, url ] ); + }, [ isSelected, image, url, captionSelected ] ); const onSelectCaption = () => { if ( ! captionSelected ) { @@ -105,7 +105,7 @@ export default function GalleryImage( { if ( container.current === document.activeElement && isSelected && - [ BACKSPACE, DELETE ].indexOf( event.keyCode ) !== -1 + [ BACKSPACE, DELETE ].includes( event.keyCode ) ) { event.stopPropagation(); event.preventDefault(); From eb104189cb35527d5c4b6a2240726b91893dc52d Mon Sep 17 00:00:00 2001 From: Ravi Chandra Date: Tue, 15 Sep 2020 12:21:24 +0530 Subject: [PATCH 4/5] Update comment --- packages/block-library/src/gallery/gallery-image.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/gallery/gallery-image.js b/packages/block-library/src/gallery/gallery-image.js index 52023cfe80973d..e6a513190800a6 100644 --- a/packages/block-library/src/gallery/gallery-image.js +++ b/packages/block-library/src/gallery/gallery-image.js @@ -74,8 +74,8 @@ export default function GalleryImage( { } ); } - // unselect the caption so when the user selects other image and comeback - // the caption is not immediately selected + // Unselect the caption so when the user selects other image and comeback + // the caption is not immediately selected. if ( captionSelected && ! isSelected ) { setCaptionSelected( false ); } From e3d0f8db94d507fb48a1cc049e536b0a0c218d80 Mon Sep 17 00:00:00 2001 From: Ravi Chandra Date: Fri, 9 Oct 2020 22:51:41 +0530 Subject: [PATCH 5/5] Refactor gallery-image native component to use React hooks --- .../src/gallery/gallery-image.native.js | 271 +++++++----------- 1 file changed, 109 insertions(+), 162 deletions(-) diff --git a/packages/block-library/src/gallery/gallery-image.native.js b/packages/block-library/src/gallery/gallery-image.native.js index 4289485b7ac3c3..2350c0077480bd 100644 --- a/packages/block-library/src/gallery/gallery-image.native.js +++ b/packages/block-library/src/gallery/gallery-image.native.js @@ -17,7 +17,7 @@ import { requestImageUploadCancelDialog, requestImageFullscreenPreview, } from '@wordpress/react-native-bridge'; -import { Component } from '@wordpress/element'; +import { useEffect, useState } from '@wordpress/element'; import { Image } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; import { Caption, MediaUploadProgress } from '@wordpress/block-editor'; @@ -39,53 +39,57 @@ const separatorStyle = compose( style.separator, { const buttonStyle = compose( style.button, { aspectRatio: 1 } ); const ICON_SIZE_ARROW = 15; -class GalleryImage extends Component { - constructor() { - super( ...arguments ); - - this.onSelectImage = this.onSelectImage.bind( this ); - this.onSelectCaption = this.onSelectCaption.bind( this ); - this.onMediaPressed = this.onMediaPressed.bind( this ); - this.onCaptionChange = this.onCaptionChange.bind( this ); - this.onSelectMedia = this.onSelectMedia.bind( this ); - - this.updateMediaProgress = this.updateMediaProgress.bind( this ); - this.finishMediaUploadWithSuccess = this.finishMediaUploadWithSuccess.bind( - this - ); - this.finishMediaUploadWithFailure = this.finishMediaUploadWithFailure.bind( - this - ); - this.renderContent = this.renderContent.bind( this ); - - this.state = { - captionSelected: false, - isUploadInProgress: false, - didUploadFail: false, - }; - } - - onSelectCaption() { - if ( ! this.state.captionSelected ) { - this.setState( { - captionSelected: true, +function GalleryImage( { + id, + url, + image, + isSelected, + isBlockSelected, + isFirstItem, + isLastItem, + caption, + isCropped, + getStylesFromColorScheme, + isRTL, + 'aria-label': ariaLabel, + onSelect, + onSelectBlock, + onRemove, + onMoveForward, + onMoveBackward, + setAttributes, +} ) { + const [ captionSelected, setCaptionSelected ] = useState( false ); + const [ isUploadInProgress, setIsUploadInProgress ] = useState( false ); + const [ didUploadFail, setDidUploadFail ] = useState( false ); + + useEffect( () => { + if ( image && ! url ) { + setAttributes( { + url: image.source_url, + alt: image.alt_text, } ); } - if ( ! this.props.isSelected ) { - this.props.onSelect(); + // unselect the caption so when the user selects other image and comeback + // the caption is not immediately selected + if ( captionSelected && ! isSelected ) { + setCaptionSelected( false ); } - } + }, [ isSelected, image, url, captionSelected ] ); - onMediaPressed() { - const { id, url, isSelected } = this.props; - const { - captionSelected, - isUploadInProgress, - didUploadFail, - } = this.state; + const onSelectCaption = () => { + if ( ! captionSelected ) { + setCaptionSelected( true ); + } + + if ( ! isSelected ) { + onSelect(); + } + }; - this.onSelectImage(); + const onMediaPressed = () => { + onSelectImage(); if ( isUploadInProgress ) { requestImageUploadCancelDialog( id ); @@ -97,98 +101,52 @@ class GalleryImage extends Component { } else if ( isSelected && ! captionSelected ) { requestImageFullscreenPreview( url ); } - } + }; - onSelectImage() { - if ( ! this.props.isBlockSelected ) { - this.props.onSelectBlock(); + const onSelectImage = () => { + if ( ! isBlockSelected ) { + onSelectBlock(); } - if ( ! this.props.isSelected ) { - this.props.onSelect(); + if ( ! isSelected ) { + onSelect(); } - if ( this.state.captionSelected ) { - this.setState( { - captionSelected: false, - } ); + if ( captionSelected ) { + setCaptionSelected( false ); } - } + }; - onSelectMedia( media ) { - const { setAttributes } = this.props; + const onSelectMedia = ( media ) => { setAttributes( media ); - } + }; - onCaptionChange( caption ) { - const { setAttributes } = this.props; - setAttributes( { caption } ); - } + const onCaptionChange = ( newCaption ) => { + setAttributes( { newCaption } ); + }; - componentDidUpdate( prevProps ) { - const { isSelected, image, url } = this.props; - if ( image && ! url ) { - this.props.setAttributes( { - url: image.source_url, - alt: image.alt_text, - } ); + const updateMediaProgress = () => { + if ( ! isUploadInProgress ) { + setIsUploadInProgress( true ); } + }; - // unselect the caption so when the user selects other image and comeback - // the caption is not immediately selected - if ( - this.state.captionSelected && - ! isSelected && - prevProps.isSelected - ) { - this.setState( { - captionSelected: false, - } ); - } - } - - updateMediaProgress() { - if ( ! this.state.isUploadInProgress ) { - this.setState( { isUploadInProgress: true } ); - } - } - - finishMediaUploadWithSuccess( payload ) { - this.setState( { - isUploadInProgress: false, - didUploadFail: false, - } ); + const finishMediaUploadWithSuccess = ( payload ) => { + setIsUploadInProgress( false ); + setDidUploadFail( false ); - this.props.setAttributes( { + setAttributes( { id: payload.mediaServerId, url: payload.mediaUrl, } ); - } + }; - finishMediaUploadWithFailure() { - this.setState( { - isUploadInProgress: false, - didUploadFail: true, - } ); - } - - renderContent( params ) { - const { - url, - isFirstItem, - isLastItem, - isSelected, - caption, - onRemove, - onMoveForward, - onMoveBackward, - 'aria-label': ariaLabel, - isCropped, - getStylesFromColorScheme, - isRTL, - } = this.props; - - const { isUploadInProgress, captionSelected } = this.state; + const finishMediaUploadWithFailure = () => { + setIsUploadInProgress( false ); + setDidUploadFail( true ); + }; + + const renderContent = ( params ) => { const { isUploadFailed, retryMessage } = params; const resizeMode = isCropped ? 'cover' : 'contain'; @@ -229,7 +187,7 @@ class GalleryImage extends Component { isUploadFailed={ isUploadFailed } isUploadInProgress={ isUploadInProgress } mediaPickerOptions={ mediaPickerOptions } - onSelectMediaUploadOption={ this.onSelectMedia } + onSelectMediaUploadOption={ onSelectMedia } resizeMode={ resizeMode } url={ url } retryMessage={ retryMessage } @@ -284,8 +242,8 @@ class GalleryImage extends Component { ); - } - - render() { - const { - id, - onRemove, - getStylesFromColorScheme, - isSelected, - } = this.props; - - const containerStyle = getStylesFromColorScheme( - style.galleryImageContainer, - style.galleryImageContainerDark - ); + }; - return ( - - - - - - ); - } - - accessibilityLabelImageContainer() { - const { caption, 'aria-label': ariaLabel } = this.props; + const containerStyle = getStylesFromColorScheme( + style.galleryImageContainer, + style.galleryImageContainerDark + ); + const accessibilityLabelImageContainer = () => { return isEmpty( caption ) ? ariaLabel : ariaLabel + @@ -354,7 +277,31 @@ class GalleryImage extends Component { __( 'Image caption. %s' ), caption ); - } + }; + + return ( + + + + + + ); } export default withPreferredColorScheme( GalleryImage );