From 590dd73d74611a587d113de217999d5584609c54 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 8 Dec 2021 12:19:18 +1100 Subject: [PATCH 1/4] Image: Update naturalWidth and naturalHeight to pull from image ref instead of local component state --- packages/block-library/src/image/image.js | 80 +++++++++++------------ 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 43c5cc6dc37738..7113ae2796447d 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -79,6 +79,7 @@ export default function Image( { context, clientId, } ) { + const imageRef = useRef(); const captionRef = useRef(); const prevUrl = usePrevious( url ); const { allowResize = true } = context; @@ -141,7 +142,6 @@ export default function Image( { ); const isLargeViewport = useViewportMatch( 'medium' ); const isWideAligned = includes( [ 'wide', 'full' ], align ); - const [ { naturalWidth, naturalHeight }, setNaturalSize ] = useState( {} ); const [ isEditingImage, setIsEditingImage ] = useState( false ); const [ externalBlob, setExternalBlob ] = useState(); const clientWidth = useClientWidth( containerRef, [ align ] ); @@ -266,6 +266,43 @@ export default function Image( { } }, [ isSelected ] ); + const filename = getFilename( url ); + let defaultedAlt; + + if ( alt ) { + defaultedAlt = alt; + } else if ( filename ) { + defaultedAlt = sprintf( + /* translators: %s: file name */ + __( 'This image has an empty alt attribute; its file name is %s' ), + filename + ); + } else { + defaultedAlt = __( 'This image has an empty alt attribute' ); + } + + let img = ( + // Disable reason: Image itself is not meant to be interactive, but + // should direct focus to block. + /* eslint-disable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */ + <> + { onImageError() } + ref={ imageRef } + /> + { temporaryURL && } + + /* eslint-enable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */ + ); + + let imageWidthWithinContainer; + let imageHeightWithinContainer; + + const naturalWidth = imageRef.current?.naturalWidth; + const naturalHeight = imageRef.current?.naturalHeight; + const canEditImage = id && naturalWidth && naturalHeight && imageEditing; const allowCrop = ! multiImageSelection && canEditImage && ! isEditingImage; @@ -386,47 +423,6 @@ export default function Image( { ); - const filename = getFilename( url ); - let defaultedAlt; - - if ( alt ) { - defaultedAlt = alt; - } else if ( filename ) { - defaultedAlt = sprintf( - /* translators: %s: file name */ - __( 'This image has an empty alt attribute; its file name is %s' ), - filename - ); - } else { - defaultedAlt = __( 'This image has an empty alt attribute' ); - } - - let img = ( - // Disable reason: Image itself is not meant to be interactive, but - // should direct focus to block. - /* eslint-disable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */ - <> - { onImageError() } - onLoad={ ( event ) => { - setNaturalSize( - pick( event.target, [ - 'naturalWidth', - 'naturalHeight', - ] ) - ); - } } - /> - { temporaryURL && } - - /* eslint-enable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */ - ); - - let imageWidthWithinContainer; - let imageHeightWithinContainer; - if ( clientWidth && naturalWidth && naturalHeight ) { const exceedMaxWidth = naturalWidth > clientWidth; const ratio = naturalHeight / naturalWidth; From 62d08a64c97e9894bc8ff4b207877bb8385d03ad Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 8 Dec 2021 16:16:07 +1100 Subject: [PATCH 2/4] Re-introduce onLoad setState behaviour to allow fallback while cropping images --- packages/block-library/src/image/image.js | 38 ++++++++++++++++++----- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 7113ae2796447d..4ae3a6352124d1 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -30,7 +30,7 @@ import { __experimentalImageEditor as ImageEditor, __experimentalImageEditingProvider as ImageEditingProvider, } from '@wordpress/block-editor'; -import { useEffect, useState, useRef } from '@wordpress/element'; +import { useEffect, useMemo, useState, useRef } from '@wordpress/element'; import { __, sprintf, isRTL } from '@wordpress/i18n'; import { getFilename } from '@wordpress/url'; import { createBlock, switchToBlockType } from '@wordpress/blocks'; @@ -142,6 +142,10 @@ export default function Image( { ); const isLargeViewport = useViewportMatch( 'medium' ); const isWideAligned = includes( [ 'wide', 'full' ], align ); + const [ + { loadedNaturalWidth, loadedNaturalHeight }, + setLoadedNaturalSize, + ] = useState( {} ); const [ isEditingImage, setIsEditingImage ] = useState( false ); const [ externalBlob, setExternalBlob ] = useState(); const clientWidth = useClientWidth( containerRef, [ align ] ); @@ -179,6 +183,23 @@ export default function Image( { } }, [ url, prevUrl ] ); + // Get naturalWidth and naturalHeight from image ref, and fall back to loaded natural + // width and height. This resolves an issue in Safari where the loaded natural + // witdth and height is otherwise lost when switching between alignments. + // See: https://github.com/WordPress/gutenberg/pull/37210. + const { naturalWidth, naturalHeight } = useMemo( () => { + return { + naturalWidth: + imageRef.current?.naturalWidth || + loadedNaturalWidth || + undefined, + naturalHeight: + imageRef.current?.naturalHeight || + loadedNaturalHeight || + undefined, + }; + }, [ loadedNaturalWidth, loadedNaturalHeight, imageRef.current ] ); + function onResizeStart() { toggleSelection( false ); } @@ -290,6 +311,12 @@ export default function Image( { src={ temporaryURL || url } alt={ defaultedAlt } onError={ () => onImageError() } + onLoad={ ( event ) => { + setLoadedNaturalSize( { + loadedNaturalWidth: event.target?.naturalWidth, + loadedNaturalHeight: event.target?.naturalHeight, + } ); + } } ref={ imageRef } /> { temporaryURL && } @@ -297,12 +324,6 @@ export default function Image( { /* eslint-enable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */ ); - let imageWidthWithinContainer; - let imageHeightWithinContainer; - - const naturalWidth = imageRef.current?.naturalWidth; - const naturalHeight = imageRef.current?.naturalHeight; - const canEditImage = id && naturalWidth && naturalHeight && imageEditing; const allowCrop = ! multiImageSelection && canEditImage && ! isEditingImage; @@ -423,6 +444,9 @@ export default function Image( { ); + let imageWidthWithinContainer; + let imageHeightWithinContainer; + if ( clientWidth && naturalWidth && naturalHeight ) { const exceedMaxWidth = naturalWidth > clientWidth; const ratio = naturalHeight / naturalWidth; From 832d1c53e9bca886851050f26907a9a2e6c55339 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 10 Dec 2021 17:03:37 +1100 Subject: [PATCH 3/4] Remove useMemo --- packages/block-library/src/image/image.js | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 4ae3a6352124d1..9fabb16c8277d8 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -30,7 +30,7 @@ import { __experimentalImageEditor as ImageEditor, __experimentalImageEditingProvider as ImageEditingProvider, } from '@wordpress/block-editor'; -import { useEffect, useMemo, useState, useRef } from '@wordpress/element'; +import { useEffect, useState, useRef } from '@wordpress/element'; import { __, sprintf, isRTL } from '@wordpress/i18n'; import { getFilename } from '@wordpress/url'; import { createBlock, switchToBlockType } from '@wordpress/blocks'; @@ -187,18 +187,10 @@ export default function Image( { // width and height. This resolves an issue in Safari where the loaded natural // witdth and height is otherwise lost when switching between alignments. // See: https://github.com/WordPress/gutenberg/pull/37210. - const { naturalWidth, naturalHeight } = useMemo( () => { - return { - naturalWidth: - imageRef.current?.naturalWidth || - loadedNaturalWidth || - undefined, - naturalHeight: - imageRef.current?.naturalHeight || - loadedNaturalHeight || - undefined, - }; - }, [ loadedNaturalWidth, loadedNaturalHeight, imageRef.current ] ); + const naturalWidth = + imageRef.current?.naturalWidth || loadedNaturalWidth || undefined; + const naturalHeight = + imageRef.current?.nautralHeight || loadedNaturalHeight || undefined; function onResizeStart() { toggleSelection( false ); From bb7301d78c8115d65ae6589ee150224b47fbefd9 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 10 Dec 2021 17:29:42 +1100 Subject: [PATCH 4/4] Re-instate useMemo as removing it caused the resizer to disappear intermittently --- packages/block-library/src/image/image.js | 96 +++++++++++++---------- 1 file changed, 54 insertions(+), 42 deletions(-) diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 9fabb16c8277d8..8c49687685d87b 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -30,7 +30,7 @@ import { __experimentalImageEditor as ImageEditor, __experimentalImageEditingProvider as ImageEditingProvider, } from '@wordpress/block-editor'; -import { useEffect, useState, useRef } from '@wordpress/element'; +import { useEffect, useMemo, useState, useRef } from '@wordpress/element'; import { __, sprintf, isRTL } from '@wordpress/i18n'; import { getFilename } from '@wordpress/url'; import { createBlock, switchToBlockType } from '@wordpress/blocks'; @@ -187,10 +187,22 @@ export default function Image( { // width and height. This resolves an issue in Safari where the loaded natural // witdth and height is otherwise lost when switching between alignments. // See: https://github.com/WordPress/gutenberg/pull/37210. - const naturalWidth = - imageRef.current?.naturalWidth || loadedNaturalWidth || undefined; - const naturalHeight = - imageRef.current?.nautralHeight || loadedNaturalHeight || undefined; + const { naturalWidth, naturalHeight } = useMemo( () => { + return { + naturalWidth: + imageRef.current?.naturalWidth || + loadedNaturalWidth || + undefined, + naturalHeight: + imageRef.current?.naturalHeight || + loadedNaturalHeight || + undefined, + }; + }, [ + loadedNaturalWidth, + loadedNaturalHeight, + imageRef.current?.complete, + ] ); function onResizeStart() { toggleSelection( false ); @@ -279,43 +291,6 @@ export default function Image( { } }, [ isSelected ] ); - const filename = getFilename( url ); - let defaultedAlt; - - if ( alt ) { - defaultedAlt = alt; - } else if ( filename ) { - defaultedAlt = sprintf( - /* translators: %s: file name */ - __( 'This image has an empty alt attribute; its file name is %s' ), - filename - ); - } else { - defaultedAlt = __( 'This image has an empty alt attribute' ); - } - - let img = ( - // Disable reason: Image itself is not meant to be interactive, but - // should direct focus to block. - /* eslint-disable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */ - <> - { onImageError() } - onLoad={ ( event ) => { - setLoadedNaturalSize( { - loadedNaturalWidth: event.target?.naturalWidth, - loadedNaturalHeight: event.target?.naturalHeight, - } ); - } } - ref={ imageRef } - /> - { temporaryURL && } - - /* eslint-enable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */ - ); - const canEditImage = id && naturalWidth && naturalHeight && imageEditing; const allowCrop = ! multiImageSelection && canEditImage && ! isEditingImage; @@ -436,6 +411,43 @@ export default function Image( { ); + const filename = getFilename( url ); + let defaultedAlt; + + if ( alt ) { + defaultedAlt = alt; + } else if ( filename ) { + defaultedAlt = sprintf( + /* translators: %s: file name */ + __( 'This image has an empty alt attribute; its file name is %s' ), + filename + ); + } else { + defaultedAlt = __( 'This image has an empty alt attribute' ); + } + + let img = ( + // Disable reason: Image itself is not meant to be interactive, but + // should direct focus to block. + /* eslint-disable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */ + <> + { onImageError() } + onLoad={ ( event ) => { + setLoadedNaturalSize( { + loadedNaturalWidth: event.target?.naturalWidth, + loadedNaturalHeight: event.target?.naturalHeight, + } ); + } } + ref={ imageRef } + /> + { temporaryURL && } + + /* eslint-enable jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events */ + ); + let imageWidthWithinContainer; let imageHeightWithinContainer;