From 057adfc8df2cb903a9a68e50859ae3b2e0e8cf4e Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 22 Apr 2021 13:02:37 +0100 Subject: [PATCH 01/52] Add new helper to utilise new REST endpoint --- .../core-data/src/fetch/__experimental-fetch-remote-url-data.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js b/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js index 93602fe4149b18..9bfe0cfb8d1a9c 100644 --- a/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js +++ b/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js @@ -31,6 +31,7 @@ import { addQueryArgs, prependHTTP } from '@wordpress/url'; * ``` * @return {Promise< WPRemoteUrlData[] >} Remote URL data. */ + const fetchRemoteUrlData = async ( url ) => { const endpoint = '/__experimental/url-details'; From d095a91d68d2dfa1e984fdfece8383096faf0192 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 9 Jun 2021 09:21:10 +0100 Subject: [PATCH 02/52] Provide docblock --- .../test/__snapshots__/index.js.snap | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap b/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap index dd0bc6f80877ce..417d6996e6a247 100644 --- a/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap +++ b/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap @@ -1,3 +1,62 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`Basic rendering should render 1`] = `"
Currently selected link settings
"`; + +exports[`Rich link previews should not fetch or display rich previews by default 1`] = ` +.emotion-0 { + width: 1.4em; + height: 1.4em; + margin: -0.2em 0.1em 0; + vertical-align: middle; + fill: currentColor; +} + +
+ + + Wordpress.org + + (opens in a new tab) + + + + + wordpress.org + + + +
+`; From d3b0f248914320e4dfa59868b0bf6ac1a3bd9b69 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 22 Apr 2021 13:05:48 +0100 Subject: [PATCH 03/52] Provide docblock --- .../core-data/src/fetch/__experimental-fetch-remote-url-data.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js b/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js index 9bfe0cfb8d1a9c..93602fe4149b18 100644 --- a/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js +++ b/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js @@ -31,7 +31,6 @@ import { addQueryArgs, prependHTTP } from '@wordpress/url'; * ``` * @return {Promise< WPRemoteUrlData[] >} Remote URL data. */ - const fetchRemoteUrlData = async ( url ) => { const endpoint = '/__experimental/url-details'; From 81b2ec227a6895fad363232c36446569298a0bb0 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 22 Apr 2021 15:15:48 +0100 Subject: [PATCH 04/52] Get title tag contents from remote URL --- .../link-control/use-search-handler.js | 65 ++++++++++++++----- 1 file changed, 48 insertions(+), 17 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-search-handler.js b/packages/block-editor/src/components/link-control/use-search-handler.js index 37fc89951b9416..08cf7aeb82437e 100644 --- a/packages/block-editor/src/components/link-control/use-search-handler.js +++ b/packages/block-editor/src/components/link-control/use-search-handler.js @@ -19,7 +19,11 @@ import { store as blockEditorStore } from '../../store'; export const handleNoop = () => Promise.resolve( [] ); -export const handleDirectEntry = ( val ) => { +export const handleDirectEntry = async ( + val, + fetchRemoteUrlData, + { fetchUrlInfo = true } = {} +) => { let type = 'URL'; const protocol = getProtocol( val ) || ''; @@ -36,14 +40,34 @@ export const handleDirectEntry = ( val ) => { type = 'internal'; } - return Promise.resolve( [ - { - id: val, - title: val, - url: type === 'URL' ? prependHTTP( val ) : val, - type, - }, - ] ); + const defaultResponse = { + id: val, + title: val, + url: type === 'URL' ? prependHTTP( val ) : val, + type, + }; + + if ( + fetchUrlInfo && + type === 'URL' && + isURLLike( prependHTTP( val ) ) && + val.length > 3 + ) { + try { + const urlData = await fetchRemoteUrlData( val ); + + return [ + { + ...defaultResponse, + ...urlData, + }, + ]; + } catch ( error ) { + return [ defaultResponse ]; + } + } + + return [ defaultResponse ]; }; const handleEntitySearch = async ( @@ -109,13 +133,18 @@ export default function useSearchHandler( withCreateSuggestion, withURLSuggestion ) { - const { fetchSearchSuggestions } = useSelect( ( select ) => { - const { getSettings } = select( blockEditorStore ); - return { - fetchSearchSuggestions: getSettings() - .__experimentalFetchLinkSuggestions, - }; - }, [] ); + const { fetchSearchSuggestions, fetchRemoteUrlData } = useSelect( + ( select ) => { + const { getSettings } = select( blockEditorStore ); + return { + fetchSearchSuggestions: getSettings() + .__experimentalFetchLinkSuggestions, + fetchRemoteUrlData: getSettings() + .__experimentalFetchRemoteUrlData, + }; + }, + [] + ); const directEntryHandler = allowDirectEntry ? handleDirectEntry @@ -124,7 +153,9 @@ export default function useSearchHandler( return useCallback( ( val, { isInitialSuggestions } ) => { return isURLLike( val ) - ? directEntryHandler( val, { isInitialSuggestions } ) + ? directEntryHandler( val, fetchRemoteUrlData, { + isInitialSuggestions, + } ) : handleEntitySearch( val, { ...suggestionsQuery, isInitialSuggestions }, From 7eb22404bb01f2035509989ebcccd680d3ffced0 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 4 May 2021 15:53:14 +0100 Subject: [PATCH 05/52] Create distinct data object for rich meta rather than rely on overiding suggestion result data. --- .../src/components/link-control/search-item.js | 2 +- .../components/link-control/use-search-handler.js | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/link-control/search-item.js b/packages/block-editor/src/components/link-control/search-item.js index 804e5c95f05afa..8b3e7b006348cb 100644 --- a/packages/block-editor/src/components/link-control/search-item.js +++ b/packages/block-editor/src/components/link-control/search-item.js @@ -39,7 +39,7 @@ export const LinkControlSearchItem = ( { diff --git a/packages/block-editor/src/components/link-control/use-search-handler.js b/packages/block-editor/src/components/link-control/use-search-handler.js index 08cf7aeb82437e..8b33633be1f206 100644 --- a/packages/block-editor/src/components/link-control/use-search-handler.js +++ b/packages/block-editor/src/components/link-control/use-search-handler.js @@ -59,7 +59,17 @@ export const handleDirectEntry = async ( return [ { ...defaultResponse, - ...urlData, + richMeta: { + ...( urlData?.title && { + title: urlData.title, + } ), + ...( urlData?.description && { + description: urlData.description, + } ), + ...( urlData?.icon && { + icon: urlData.title, + } ), + }, }, ]; } catch ( error ) { From 1d0eae804e6b61a480396792f01b977eeb4545ea Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 4 May 2021 16:50:31 +0100 Subject: [PATCH 06/52] Add example of adding rich details to the link preview --- .../components/link-control/link-preview.js | 52 +++++++++++++++---- .../src/components/link-control/style.scss | 9 +++- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index 2207e42b810613..3b6832afbb5d70 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -9,13 +9,37 @@ import classnames from 'classnames'; import { __ } from '@wordpress/i18n'; import { Button, ExternalLink } from '@wordpress/components'; import { filterURLForDisplay, safeDecodeURI } from '@wordpress/url'; +import { useSelect } from '@wordpress/data'; +import { useState, useEffect } from '@wordpress/element'; +import { Icon, globe } from '@wordpress/icons'; /** * Internal dependencies */ import { ViewerSlot } from './viewer-slot'; +import { store as blockEditorStore } from '../../store'; export default function LinkPreview( { value, onEditClick } ) { + const [ richData, setRichData ] = useState( {} ); + + const { fetchRemoteUrlData } = useSelect( ( select ) => { + const { getSettings } = select( blockEditorStore ); + return { + fetchRemoteUrlData: getSettings().__experimentalFetchRemoteUrlData, + }; + }, [] ); + + useEffect( () => { + const fetchRichData = async () => { + const urlData = await fetchRemoteUrlData( value.url ); + setRichData( urlData ); + }; + + if ( value?.url?.length ) { + fetchRichData(); + } + }, [ value ] ); + const displayURL = ( value && filterURLForDisplay( safeDecodeURI( value.url ), 16 ) ) || ''; @@ -29,17 +53,23 @@ export default function LinkPreview( { value, onEditClick } ) { } ) } > - - { ( value && value.title ) || displayURL } - - { value && value.title && ( - - { displayURL } - - ) } + + + + { richData?.title || value?.title || displayURL } + + { value?.url && ( + + { displayURL } + + ) } + - + + { ( richData?.image || richData?.description ) && (
From dee6d6fb8b562f769bc7df4c935255c453990a32 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 20 May 2021 16:54:03 +0100 Subject: [PATCH 09/52] Add in description --- .../src/components/link-control/link-preview.js | 12 ++++++++++-- .../src/components/link-control/style.scss | 5 +++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index bc69dca4ecf88a..be41b9f9c35edc 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -93,9 +93,17 @@ export default function LinkPreview( { value, onEditClick } ) { { ( richData?.image || richData?.description ) && (
{ richData?.image && ( - + + ) } + { richData?.description && ( +

+ { richData.description } +

) } - { richData?.description &&

{ richData.description }

}
) }
diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index d2d8902b0426d2..1f0db77a96f3f5 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -212,6 +212,11 @@ $block-editor-link-control-number-of-actions: 1; background-color: $gray-100; border-radius: 2px; } + + .block-editor-link-control__search-item-description { + padding-top: $grid-unit-10; + margin: 0; + } } .block-editor-link-control__search-item-top { From fece66fcc1ebc689a05da43cdf9b16f8201e48e9 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 20 May 2021 17:08:40 +0100 Subject: [PATCH 10/52] Remove url data to hook and prevent setting state on unmounted --- .../components/link-control/link-preview.js | 26 ++--------- .../link-control/use-remote-url-data.js | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+), 22 deletions(-) create mode 100644 packages/block-editor/src/components/link-control/use-remote-url-data.js diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index be41b9f9c35edc..1548f213938287 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -9,36 +9,18 @@ import classnames from 'classnames'; import { __ } from '@wordpress/i18n'; import { Button, ExternalLink } from '@wordpress/components'; import { filterURLForDisplay, safeDecodeURI } from '@wordpress/url'; -import { useSelect } from '@wordpress/data'; -import { useState, useEffect } from '@wordpress/element'; + import { Icon, globe } from '@wordpress/icons'; /** * Internal dependencies */ import { ViewerSlot } from './viewer-slot'; -import { store as blockEditorStore } from '../../store'; - -export default function LinkPreview( { value, onEditClick } ) { - const [ richData, setRichData ] = useState( {} ); - const { fetchRemoteUrlData } = useSelect( ( select ) => { - const { getSettings } = select( blockEditorStore ); - return { - fetchRemoteUrlData: getSettings().__experimentalFetchRemoteUrlData, - }; - }, [] ); +import useRemoteUrlData from './use-remote-url-data'; - useEffect( () => { - const fetchRichData = async () => { - const urlData = await fetchRemoteUrlData( value.url ); - setRichData( urlData ); - }; - - if ( value?.url?.length ) { - fetchRichData(); - } - }, [ value ] ); +export default function LinkPreview( { value, onEditClick } ) { + const richData = useRemoteUrlData( value?.url ); const displayURL = ( value && filterURLForDisplay( safeDecodeURI( value.url ), 16 ) ) || diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js new file mode 100644 index 00000000000000..f12bb8a35045f1 --- /dev/null +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -0,0 +1,46 @@ +/** + * Internal dependencies + */ +import { store as blockEditorStore } from '../../store'; + +/** + * WordPress dependencies + */ +import { useSelect } from '@wordpress/data'; +import { useState, useEffect, useRef } from '@wordpress/element'; + +function useRemoteUrlData( url ) { + let { current: isMounted } = useRef( false ); + const [ richData, setRichData ] = useState( {} ); + + const { fetchRemoteUrlData } = useSelect( ( select ) => { + const { getSettings } = select( blockEditorStore ); + return { + fetchRemoteUrlData: getSettings().__experimentalFetchRemoteUrlData, + }; + }, [] ); + + useEffect( () => { + const fetchRichData = async () => { + const urlData = await fetchRemoteUrlData( url ); + if ( isMounted ) { + setRichData( urlData ); + } + }; + + if ( url?.length && isMounted ) { + fetchRichData(); + } + }, [ url ] ); + + useEffect( () => { + isMounted = true; + return () => { + isMounted = false; + }; + }, [] ); + + return richData; +} + +export default useRemoteUrlData; From 47edef2af9a0b07f3937da2fe9495e3e2abd286b Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 21 May 2021 08:54:46 +0100 Subject: [PATCH 11/52] Avoid ref destructure --- .../link-control/use-remote-url-data.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index f12bb8a35045f1..98cdeced1986c3 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -10,8 +10,8 @@ import { useSelect } from '@wordpress/data'; import { useState, useEffect, useRef } from '@wordpress/element'; function useRemoteUrlData( url ) { - let { current: isMounted } = useRef( false ); - const [ richData, setRichData ] = useState( {} ); + const isMounted = useRef( false ); + const [ richData, setRichData ] = useState( null ); const { fetchRemoteUrlData } = useSelect( ( select ) => { const { getSettings } = select( blockEditorStore ); @@ -20,26 +20,26 @@ function useRemoteUrlData( url ) { }; }, [] ); + useEffect( () => { + isMounted.current = true; + return () => { + isMounted.current = false; + }; + }, [] ); + useEffect( () => { const fetchRichData = async () => { const urlData = await fetchRemoteUrlData( url ); - if ( isMounted ) { + if ( isMounted.current ) { setRichData( urlData ); } }; - if ( url?.length && isMounted ) { + if ( url?.length && isMounted.current ) { fetchRichData(); } }, [ url ] ); - useEffect( () => { - isMounted = true; - return () => { - isMounted = false; - }; - }, [] ); - return richData; } From e6633aa1b97b0c6b5c4e7bf8bd74d5a878b2f534 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 21 May 2021 09:38:49 +0100 Subject: [PATCH 12/52] Add loading animation --- .../components/link-control/link-preview.js | 31 +++++----- .../src/components/link-control/style.scss | 57 +++++++++++++++++++ .../link-control/use-remote-url-data.js | 19 ++++++- 3 files changed, 91 insertions(+), 16 deletions(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index 1548f213938287..29bc7a5f3df46f 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -20,34 +20,37 @@ import { ViewerSlot } from './viewer-slot'; import useRemoteUrlData from './use-remote-url-data'; export default function LinkPreview( { value, onEditClick } ) { - const richData = useRemoteUrlData( value?.url ); + const { richData, isFetching } = useRemoteUrlData( value?.url ); const displayURL = ( value && filterURLForDisplay( safeDecodeURI( value.url ), 16 ) ) || ''; - return (
- { richData?.icon ? ( - - ) : ( - - ) } + + { richData?.icon ? ( + + ) : ( + + ) } + { @@ -29,7 +30,14 @@ function useRemoteUrlData( url ) { useEffect( () => { const fetchRichData = async () => { - const urlData = await fetchRemoteUrlData( url ); + isFetching.current = true; + + const urlData = await fetchRemoteUrlData( url ).catch( () => { + isFetching.current = false; + } ); + + isFetching.current = false; + if ( isMounted.current ) { setRichData( urlData ); } @@ -38,9 +46,16 @@ function useRemoteUrlData( url ) { if ( url?.length && isMounted.current ) { fetchRichData(); } + + return () => { + isFetching.current = false; + }; }, [ url ] ); - return richData; + return { + richData, + isFetching: isFetching.current, + }; } export default useRemoteUrlData; From 21fb199e958a90114a09eaf3257c174ee19fd1e0 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 21 May 2021 09:52:07 +0100 Subject: [PATCH 13/52] Clear richdata on url change --- .../src/components/link-control/use-remote-url-data.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index 86b4cb5d34c448..b3705c0de91fde 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -21,6 +21,7 @@ function useRemoteUrlData( url ) { }; }, [] ); + // Avoid fetching or state updates if not mounted. useEffect( () => { isMounted.current = true; return () => { @@ -28,6 +29,11 @@ function useRemoteUrlData( url ) { }; }, [] ); + // Clear the data if the URL changes to avoid stale data in hook consumer. + useEffect( () => { + setRichData( null ); + }, [ url ] ); + useEffect( () => { const fetchRichData = async () => { isFetching.current = true; From 592051542e5e48ae5690154b49453bfeae7582db Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 21 May 2021 09:56:53 +0100 Subject: [PATCH 14/52] Avoid rich data in search items --- .../components/link-control/search-item.js | 2 +- .../link-control/use-search-handler.js | 75 +++++-------------- 2 files changed, 18 insertions(+), 59 deletions(-) diff --git a/packages/block-editor/src/components/link-control/search-item.js b/packages/block-editor/src/components/link-control/search-item.js index 8b3e7b006348cb..804e5c95f05afa 100644 --- a/packages/block-editor/src/components/link-control/search-item.js +++ b/packages/block-editor/src/components/link-control/search-item.js @@ -39,7 +39,7 @@ export const LinkControlSearchItem = ( { diff --git a/packages/block-editor/src/components/link-control/use-search-handler.js b/packages/block-editor/src/components/link-control/use-search-handler.js index 8b33633be1f206..37fc89951b9416 100644 --- a/packages/block-editor/src/components/link-control/use-search-handler.js +++ b/packages/block-editor/src/components/link-control/use-search-handler.js @@ -19,11 +19,7 @@ import { store as blockEditorStore } from '../../store'; export const handleNoop = () => Promise.resolve( [] ); -export const handleDirectEntry = async ( - val, - fetchRemoteUrlData, - { fetchUrlInfo = true } = {} -) => { +export const handleDirectEntry = ( val ) => { let type = 'URL'; const protocol = getProtocol( val ) || ''; @@ -40,44 +36,14 @@ export const handleDirectEntry = async ( type = 'internal'; } - const defaultResponse = { - id: val, - title: val, - url: type === 'URL' ? prependHTTP( val ) : val, - type, - }; - - if ( - fetchUrlInfo && - type === 'URL' && - isURLLike( prependHTTP( val ) ) && - val.length > 3 - ) { - try { - const urlData = await fetchRemoteUrlData( val ); - - return [ - { - ...defaultResponse, - richMeta: { - ...( urlData?.title && { - title: urlData.title, - } ), - ...( urlData?.description && { - description: urlData.description, - } ), - ...( urlData?.icon && { - icon: urlData.title, - } ), - }, - }, - ]; - } catch ( error ) { - return [ defaultResponse ]; - } - } - - return [ defaultResponse ]; + return Promise.resolve( [ + { + id: val, + title: val, + url: type === 'URL' ? prependHTTP( val ) : val, + type, + }, + ] ); }; const handleEntitySearch = async ( @@ -143,18 +109,13 @@ export default function useSearchHandler( withCreateSuggestion, withURLSuggestion ) { - const { fetchSearchSuggestions, fetchRemoteUrlData } = useSelect( - ( select ) => { - const { getSettings } = select( blockEditorStore ); - return { - fetchSearchSuggestions: getSettings() - .__experimentalFetchLinkSuggestions, - fetchRemoteUrlData: getSettings() - .__experimentalFetchRemoteUrlData, - }; - }, - [] - ); + const { fetchSearchSuggestions } = useSelect( ( select ) => { + const { getSettings } = select( blockEditorStore ); + return { + fetchSearchSuggestions: getSettings() + .__experimentalFetchLinkSuggestions, + }; + }, [] ); const directEntryHandler = allowDirectEntry ? handleDirectEntry @@ -163,9 +124,7 @@ export default function useSearchHandler( return useCallback( ( val, { isInitialSuggestions } ) => { return isURLLike( val ) - ? directEntryHandler( val, fetchRemoteUrlData, { - isInitialSuggestions, - } ) + ? directEntryHandler( val, { isInitialSuggestions } ) : handleEntitySearch( val, { ...suggestionsQuery, isInitialSuggestions }, From 48f909055917b114a021d2efff319090506e3daa Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 21 May 2021 09:57:35 +0100 Subject: [PATCH 15/52] Account for preview layout variation requirements --- .../src/components/link-control/link-preview.js | 1 + .../block-editor/src/components/link-control/style.scss | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index 29bc7a5f3df46f..7266dda9df93ba 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -33,6 +33,7 @@ export default function LinkPreview( { value, onEditClick } ) { 'is-current': true, 'is-rich': richData, 'is-fetching': isFetching, + 'is-preview': true, } ) } >
diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index b4c8c404cd4b6d..d6da23c5f2ac62 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -150,7 +150,7 @@ $block-editor-link-control-number-of-actions: 1; } .block-editor-link-control__search-item-header { - display: flex; + display: block; flex-direction: row; align-items: center; margin-right: $grid-unit-20; @@ -158,6 +158,10 @@ $block-editor-link-control-number-of-actions: 1; white-space: nowrap; } + &.is-preview .block-editor-link-control__search-item-header { + display: flex; + } + .block-editor-link-control__search-item-details { display: block; } From b0d4a33a6ef857de71fb20784ec095ee265eb3f1 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 21 May 2021 10:54:45 +0100 Subject: [PATCH 16/52] Only fetch if fetchRemoteUrlData is available --- .../src/components/link-control/use-remote-url-data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index b3705c0de91fde..1418569d0cf33e 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -49,7 +49,7 @@ function useRemoteUrlData( url ) { } }; - if ( url?.length && isMounted.current ) { + if ( url?.length && isMounted.current && fetchRemoteUrlData ) { fetchRichData(); } From f89d4abec9ae8378045bbf4feeb639d8549b263a Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 21 May 2021 11:05:58 +0100 Subject: [PATCH 17/52] Fix text overflow ellipsis --- packages/block-editor/src/components/link-control/style.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index d6da23c5f2ac62..0db713793d420a 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -163,7 +163,7 @@ $block-editor-link-control-number-of-actions: 1; } .block-editor-link-control__search-item-details { - display: block; + overflow: hidden; // clip to force text ellipsis. } .block-editor-link-control__search-item-icon { From 74501fba632eecd7cfebf3ba94876b301fdcc439 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 21 May 2021 11:10:14 +0100 Subject: [PATCH 18/52] Improve icon visual scaling --- .../block-editor/src/components/link-control/style.scss | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index 0db713793d420a..58c11038db39b2 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -152,7 +152,7 @@ $block-editor-link-control-number-of-actions: 1; .block-editor-link-control__search-item-header { display: block; flex-direction: row; - align-items: center; + align-items: flex-start; margin-right: $grid-unit-20; overflow: hidden; white-space: nowrap; @@ -168,14 +168,15 @@ $block-editor-link-control-number-of-actions: 1; .block-editor-link-control__search-item-icon { position: relative; + top: 0.2em; margin-right: $grid-unit-10; - width: 32px; - height: 32px; + width: 18px; // half of 32px to improve perceived resolution. + height: 18px; // half of 32px to improve perceived resolution. svg, img { max-width: none; - width: 32px; + width: 18px; // half of 32px to improve perceived resolution. } } From 5f1688a4d54330ed44fb8ede88535fedd50d9b20 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 26 May 2021 09:50:43 +0100 Subject: [PATCH 19/52] Manage fetching as state --- .../link-control/use-remote-url-data.js | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index 1418569d0cf33e..476fa48e93a2f2 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -11,8 +11,9 @@ import { useState, useEffect, useRef } from '@wordpress/element'; function useRemoteUrlData( url ) { const isMounted = useRef( false ); - const isFetching = useRef( false ); + const [ richData, setRichData ] = useState( null ); + const [ isFetching, setIsFetching ] = useState( false ); const { fetchRemoteUrlData } = useSelect( ( select ) => { const { getSettings } = select( blockEditorStore ); @@ -30,37 +31,34 @@ function useRemoteUrlData( url ) { }, [] ); // Clear the data if the URL changes to avoid stale data in hook consumer. - useEffect( () => { - setRichData( null ); - }, [ url ] ); + // useEffect( () => { + // setRichData( null ); + // }, [ url ] ); useEffect( () => { const fetchRichData = async () => { - isFetching.current = true; + setIsFetching( true ); + setRichData( null ); const urlData = await fetchRemoteUrlData( url ).catch( () => { - isFetching.current = false; + setIsFetching( false ); } ); - isFetching.current = false; - if ( isMounted.current ) { + // console.log( 'Setting urlData', urlData ); setRichData( urlData ); + setIsFetching( false ); } }; if ( url?.length && isMounted.current && fetchRemoteUrlData ) { fetchRichData(); } - - return () => { - isFetching.current = false; - }; }, [ url ] ); return { richData, - isFetching: isFetching.current, + isFetching, }; } From 03e4ab685b867ba2ed726165776e727f064d94ab Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 9 Jun 2021 09:21:52 +0100 Subject: [PATCH 20/52] Add tests for rich data and resolve exposed bugs --- .../components/link-control/link-preview.js | 7 +- .../src/components/link-control/test/index.js | 108 +++++++++++++++++- 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index 7266dda9df93ba..cccc5155cb50f3 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -22,6 +22,9 @@ import useRemoteUrlData from './use-remote-url-data'; export default function LinkPreview( { value, onEditClick } ) { const { richData, isFetching } = useRemoteUrlData( value?.url ); + // Rich data may be an empty object so test for that. + const hasRichData = richData && Object.keys( richData ).length; + const displayURL = ( value && filterURLForDisplay( safeDecodeURI( value.url ), 16 ) ) || ''; @@ -31,8 +34,8 @@ export default function LinkPreview( { value, onEditClick } ) { aria-selected="true" className={ classnames( 'block-editor-link-control__search-item', { 'is-current': true, - 'is-rich': richData, - 'is-fetching': isFetching, + 'is-rich': hasRichData, + 'is-fetching': !! isFetching, 'is-preview': true, } ) } > diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index c4feb75eca8e9e..9a16d39885632e 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -10,6 +10,10 @@ import { default as lodash, first, last, nth, uniqueId } from 'lodash'; */ import { useState } from '@wordpress/element'; import { UP, DOWN, ENTER } from '@wordpress/keycodes'; +/** + * WordPress dependencies + */ +import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ @@ -23,11 +27,23 @@ lodash.debounce = jest.fn( ( callback ) => { } ); const mockFetchSearchSuggestions = jest.fn(); +const mockFetchRemoteUrlData = jest.fn(); -jest.mock( '@wordpress/data/src/components/use-select', () => () => ( { +jest.mock( '@wordpress/data/src/components/use-select', () => { + // This allows us to tweak the returned value on each test + const mock = jest.fn(); + return mock; +} ); +useSelect.mockImplementation( () => ( { fetchSearchSuggestions: mockFetchSearchSuggestions, + fetchRemoteUrlData: mockFetchRemoteUrlData, } ) ); +// jest.mock( '@wordpress/data/src/components/use-select', () => () => ( { +// fetchSearchSuggestions: mockFetchSearchSuggestions, +// fetchRemoteUrlData: mockFetchRemoteUrlData, +// } ) ); + jest.mock( '@wordpress/data/src/components/use-dispatch', () => ( { useDispatch: () => ( { saveEntityRecords: jest.fn() } ), } ) ); @@ -59,6 +75,7 @@ afterEach( () => { container.remove(); container = null; mockFetchSearchSuggestions.mockReset(); + mockFetchRemoteUrlData.mockReset(); } ); function getURLInput() { @@ -1713,3 +1730,92 @@ describe( 'Post types', () => { } ); } ); + +describe( 'Rich link previews', () => { + const selectedLink = { + id: '1', + title: 'https://www.wordpress.org', + url: 'https://www.wordpress.org', + type: 'URL', + }; + it( 'should display a rich preview when data is available', async () => { + mockFetchRemoteUrlData.mockImplementation( () => + Promise.resolve( { + title: + 'Blog Tool, Publishing Platform, and CMS \u2014 WordPress.org', + icon: 'https://s.w.org/favicon.ico?2', + description: + 'Open source software which you can use to easily create a beautiful website, blog, or app.', + image: 'https://s.w.org/images/home/screen-themes.png?3', + } ) + ); + + act( () => { + render( , container ); + } ); + + // mockFetchRemoteUrlData resolves on next "tick" of event loop + await act( async () => { + await eventLoopTick(); + } ); + + const linkPreview = container.querySelector( + "[aria-label='Currently selected']" + ); + + const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); + + expect( isRichLinkPreview ).toBe( true ); + expect( linkPreview ).toMatchSnapshot(); + } ); + + it( 'should not display a rich preview when data is empty', async () => { + mockFetchRemoteUrlData.mockImplementation( () => + Promise.resolve( {} ) + ); + + act( () => { + render( , container ); + } ); + + // mockFetchRemoteUrlData resolves on next "tick" of event loop + await act( async () => { + await eventLoopTick(); + } ); + + const linkPreview = container.querySelector( + "[aria-label='Currently selected']" + ); + + const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); + + expect( isRichLinkPreview ).toBe( false ); + } ); + + it( 'should display in loading state when rich data is being fetched', async () => { + const nonResolvingPromise = () => new Promise( () => {} ); + + mockFetchRemoteUrlData.mockImplementation( nonResolvingPromise ); + + act( () => { + render( , container ); + } ); + + // mockFetchRemoteUrlData resolves on next "tick" of event loop + await act( async () => { + await eventLoopTick(); + } ); + + const linkPreview = container.querySelector( + "[aria-label='Currently selected']" + ); + + const isFetchingRichPreview = linkPreview.classList.contains( + 'is-fetching' + ); + const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); + + expect( isFetchingRichPreview ).toBe( true ); + expect( isRichLinkPreview ).toBe( false ); + } ); +} ); From 9a108afaf24a4337b1740a83e7e4f4d9c9ccf165 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 26 May 2021 16:03:44 +0100 Subject: [PATCH 21/52] Fix broken async/await code and restore test running --- .../link-control/use-remote-url-data.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index 476fa48e93a2f2..ab5151d0e2aeec 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -40,13 +40,16 @@ function useRemoteUrlData( url ) { setIsFetching( true ); setRichData( null ); - const urlData = await fetchRemoteUrlData( url ).catch( () => { - setIsFetching( false ); - } ); + try { + const urlData = await fetchRemoteUrlData( url ).catch( () => { + setIsFetching( false ); + } ); - if ( isMounted.current ) { - // console.log( 'Setting urlData', urlData ); - setRichData( urlData ); + if ( isMounted.current ) { + setRichData( urlData ); + setIsFetching( false ); + } + } catch ( e ) { setIsFetching( false ); } }; From b15ad00367dc265b261157dc2876a754bd3e23dc Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 26 May 2021 16:04:34 +0100 Subject: [PATCH 22/52] Tidy code comments --- .../src/components/link-control/use-remote-url-data.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index ab5151d0e2aeec..ad2d106f952872 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -30,14 +30,11 @@ function useRemoteUrlData( url ) { }; }, [] ); - // Clear the data if the URL changes to avoid stale data in hook consumer. - // useEffect( () => { - // setRichData( null ); - // }, [ url ] ); - useEffect( () => { const fetchRichData = async () => { setIsFetching( true ); + + // Clear the data if the URL changes to avoid stale data in hook consumer. setRichData( null ); try { From b4df9d2ae6d3c944580344cd7c1c5f8d91aa24f7 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 9 Jun 2021 09:22:21 +0100 Subject: [PATCH 23/52] Updates to use visually truncated description text --- .../src/components/link-control/link-preview.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index cccc5155cb50f3..2a4abaa7cf617f 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -7,9 +7,12 @@ import classnames from 'classnames'; * WordPress dependencies */ import { __ } from '@wordpress/i18n'; -import { Button, ExternalLink } from '@wordpress/components'; +import { + Button, + ExternalLink, + __experimentalText as Text, +} from '@wordpress/components'; import { filterURLForDisplay, safeDecodeURI } from '@wordpress/url'; - import { Icon, globe } from '@wordpress/icons'; /** @@ -89,9 +92,13 @@ export default function LinkPreview( { value, onEditClick } ) { /> ) } { richData?.description && ( -

+ { richData.description } -

+ ) }
) } From 5b9736be5ebd65a9e7a2853819e328fc00400d37 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 9 Jun 2021 09:22:28 +0100 Subject: [PATCH 24/52] Clip image height, center and provide border-radius and background --- .../src/components/link-control/link-preview.js | 8 +++----- .../src/components/link-control/style.scss | 13 ++++++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index 2a4abaa7cf617f..0757b1070750fe 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -85,11 +85,9 @@ export default function LinkPreview( { value, onEditClick } ) { { ( richData?.image || richData?.description ) && (
{ richData?.image && ( - +
+ +
) } { richData?.description && ( Date: Thu, 27 May 2021 10:17:58 +0100 Subject: [PATCH 25/52] Apply correct spacing as per design provided --- .../block-editor/src/components/link-control/style.scss | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index f19061495b989f..8e8df631c584ce 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -228,7 +228,7 @@ $block-editor-link-control-number-of-actions: 1; } .block-editor-link-control__search-item-description { - padding-top: $grid-unit-10; + padding-top: 12px; margin: 0; font-size: 0.9em; @@ -239,8 +239,11 @@ $block-editor-link-control-number-of-actions: 1; width: 100%; background-color: $gray-100; justify-content: center; + height: 180px; // limit height + max-height: 180px; // limit height overflow: hidden; border-radius: 2px; + margin-top: 12px; img { display: block; // remove unwanted space below image @@ -258,7 +261,6 @@ $block-editor-link-control-number-of-actions: 1; } .block-editor-link-control__search-item-bottom { - padding-top: $grid-unit-20; transition: opacity 1.5s; min-height: 100px; width: 100%; From 8189a8096e479df8c398955baa32c30ac3e782bd Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 27 May 2021 10:20:09 +0100 Subject: [PATCH 26/52] Allow text to flow (almost) to edge of `Edit` button before ellipsis --- .../block-editor/src/components/link-control/style.scss | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index 8e8df631c584ce..0b27658852f297 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -153,13 +153,14 @@ $block-editor-link-control-number-of-actions: 1; display: block; flex-direction: row; align-items: flex-start; - margin-right: $grid-unit-20; + margin-right: $grid-unit-10; overflow: hidden; white-space: nowrap; } &.is-preview .block-editor-link-control__search-item-header { display: flex; + flex: 1; // fill available space. } .block-editor-link-control__search-item-details { @@ -185,7 +186,6 @@ $block-editor-link-control-number-of-actions: 1; .block-editor-link-control__search-item-title { overflow: hidden; text-overflow: ellipsis; - padding-right: $grid-unit-30; .components-external-link__icon { position: absolute; @@ -209,6 +209,10 @@ $block-editor-link-control-number-of-actions: 1; span { font-weight: normal; } + + svg { + display: none; // specifically requested to be removed visually as well. + } } .block-editor-link-control__search-item-info { From b3551c6dcde85c5b331ed4651082ec217ed75f19 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 27 May 2021 10:26:34 +0100 Subject: [PATCH 27/52] Constrain image height to 140px as per Figma design --- .../block-editor/src/components/link-control/style.scss | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index 0b27658852f297..4dde495f82957d 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -1,4 +1,5 @@ $block-editor-link-control-number-of-actions: 1; +$preview-image-height: 140px; .block-editor-link-control { position: relative; @@ -243,8 +244,8 @@ $block-editor-link-control-number-of-actions: 1; width: 100%; background-color: $gray-100; justify-content: center; - height: 180px; // limit height - max-height: 180px; // limit height + height: $preview-image-height; // limit height + max-height: $preview-image-height; // limit height overflow: hidden; border-radius: 2px; margin-top: 12px; @@ -252,8 +253,8 @@ $block-editor-link-control-number-of-actions: 1; img { display: block; // remove unwanted space below image max-width: 100%; - height: 180px; // limit height - max-height: 180px; // limit height + height: $preview-image-height; // limit height + max-height: $preview-image-height; // limit height } } } From 151659249654c5198b6e6a7e790e083c2678268d Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 27 May 2021 11:24:42 +0100 Subject: [PATCH 28/52] Add tests to cover missing data edge cases --- .../src/components/link-control/test/index.js | 131 +++++++++++++++++- 1 file changed, 127 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index 9a16d39885632e..888db4c96f956a 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -1734,7 +1734,7 @@ describe( 'Post types', () => { describe( 'Rich link previews', () => { const selectedLink = { id: '1', - title: 'https://www.wordpress.org', + title: 'Wordpress.org', // customize this for differentiation in assertions url: 'https://www.wordpress.org', type: 'URL', }; @@ -1769,9 +1769,14 @@ describe( 'Rich link previews', () => { expect( linkPreview ).toMatchSnapshot(); } ); - it( 'should not display a rich preview when data is empty', async () => { + it( 'should display a fallback when title is missing from rich data', async () => { mockFetchRemoteUrlData.mockImplementation( () => - Promise.resolve( {} ) + Promise.resolve( { + icon: 'https://s.w.org/favicon.ico?2', + description: + 'Open source software which you can use to easily create a beautiful website, blog, or app.', + image: 'https://s.w.org/images/home/screen-themes.png?3', + } ) ); act( () => { @@ -1788,10 +1793,128 @@ describe( 'Rich link previews', () => { ); const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); + expect( isRichLinkPreview ).toBe( true ); - expect( isRichLinkPreview ).toBe( false ); + const titlePreview = linkPreview.querySelector( + '.block-editor-link-control__search-item-title' + ); + + expect( titlePreview.textContent ).toEqual( + expect.stringContaining( selectedLink.title ) + ); + } ); + + it( 'should display a fallback when icon is missing from rich data', async () => { + mockFetchRemoteUrlData.mockImplementation( () => + Promise.resolve( { + title: + 'Blog Tool, Publishing Platform, and CMS \u2014 WordPress.org', + description: + 'Open source software which you can use to easily create a beautiful website, blog, or app.', + image: 'https://s.w.org/images/home/screen-themes.png?3', + } ) + ); + + act( () => { + render( , container ); + } ); + + // mockFetchRemoteUrlData resolves on next "tick" of event loop + await act( async () => { + await eventLoopTick(); + } ); + + const linkPreview = container.querySelector( + "[aria-label='Currently selected']" + ); + + const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); + expect( isRichLinkPreview ).toBe( true ); + + const iconPreview = linkPreview.querySelector( + `.block-editor-link-control__search-item-icon` + ); + + const fallBackIcon = iconPreview.querySelector( 'svg' ); + const richIcon = iconPreview.querySelector( 'img' ); + + expect( fallBackIcon ).toBeTruthy(); + expect( richIcon ).toBeFalsy(); } ); + it.each( [ 'image', 'description' ] )( + 'should not display an %s fallback when it is missing from the rich data', + async ( dataItem ) => { + mockFetchRemoteUrlData.mockImplementation( () => { + const data = { + title: + 'Blog Tool, Publishing Platform, and CMS \u2014 WordPress.org', + icon: 'https://s.w.org/favicon.ico?2', + description: + 'Open source software which you can use to easily create a beautiful website, blog, or app.', + image: 'https://s.w.org/images/home/screen-themes.png?3', + }; + delete data[ dataItem ]; + return Promise.resolve( data ); + } ); + + act( () => { + render( , container ); + } ); + + // mockFetchRemoteUrlData resolves on next "tick" of event loop + await act( async () => { + await eventLoopTick(); + } ); + + const linkPreview = container.querySelector( + "[aria-label='Currently selected']" + ); + + const isRichLinkPreview = linkPreview.classList.contains( + 'is-rich' + ); + expect( isRichLinkPreview ).toBe( true ); + + const missingDataItem = linkPreview.querySelector( + `.block-editor-link-control__search-item-${ dataItem }` + ); + + expect( missingDataItem ).toBeFalsy(); + } + ); + + it.each( [ + [ 'empty', {} ], + [ 'null', null ], + ] )( + 'should not display a rich preview when data is %s', + async ( _descriptor, data ) => { + mockFetchRemoteUrlData.mockImplementation( () => + Promise.resolve( data ) + ); + + act( () => { + render( , container ); + } ); + + // mockFetchRemoteUrlData resolves on next "tick" of event loop + await act( async () => { + await eventLoopTick(); + } ); + + const linkPreview = container.querySelector( + "[aria-label='Currently selected']" + ); + + const isRichLinkPreview = linkPreview.classList.contains( + 'is-rich' + ); + + expect( isRichLinkPreview ).toBe( false ); + } + ); + it( 'should display in loading state when rich data is being fetched', async () => { const nonResolvingPromise = () => new Promise( () => {} ); From 7d91abad0722a46ea426e45d00f43442673cf017 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 27 May 2021 14:41:56 +0100 Subject: [PATCH 29/52] Fix cancel pending fetch when URL changes. --- .../link-control/use-remote-url-data.js | 71 +++++++++++++++++-- 1 file changed, 65 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index ad2d106f952872..13f85062df5d55 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -7,10 +7,11 @@ import { store as blockEditorStore } from '../../store'; * WordPress dependencies */ import { useSelect } from '@wordpress/data'; -import { useState, useEffect, useRef } from '@wordpress/element'; +import { useState, useEffect, useRef, useCallback } from '@wordpress/element'; function useRemoteUrlData( url ) { const isMounted = useRef( false ); + const cancelableFetch = useRef(); const [ richData, setRichData ] = useState( null ); const [ isFetching, setIsFetching ] = useState( false ); @@ -30,6 +31,29 @@ function useRemoteUrlData( url ) { }; }, [] ); + const cancelPendingFetch = useCallback( () => { + if ( cancelableFetch.current ) { + cancelableFetch.current.cancel(); + } + } ); + + /** + * Cancel any pending requests that were made for + * stale URL values. + */ + useEffect( () => { + return cancelPendingFetch(); + }, [ url ] ); + + /** + * Cancel any pending requests on "unmount" + */ + useEffect( () => { + return () => { + cancelPendingFetch(); + }; + }, [] ); + useEffect( () => { const fetchRichData = async () => { setIsFetching( true ); @@ -38,16 +62,24 @@ function useRemoteUrlData( url ) { setRichData( null ); try { - const urlData = await fetchRemoteUrlData( url ).catch( () => { - setIsFetching( false ); - } ); + cancelableFetch.current = makeCancelable( + // Using Promise.resolve to allow createSuggestion to return a + // non-Promise based value. + fetchRemoteUrlData( url ) + ); + const urlData = await cancelableFetch.current.promise; if ( isMounted.current ) { setRichData( urlData ); setIsFetching( false ); } - } catch ( e ) { - setIsFetching( false ); + } catch ( error ) { + if ( error && error.isCanceled ) { + return; // bail if canceled to avoid setting state + } + if ( isMounted.current ) { + setIsFetching( false ); + } } }; @@ -63,3 +95,30 @@ function useRemoteUrlData( url ) { } export default useRemoteUrlData; + +/** + * Creates a wrapper around a promise which allows it to be programmatically + * cancelled. + * See: https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html + * + * @param {Promise} promise the Promise to make cancelable + */ +const makeCancelable = ( promise ) => { + let hasCanceled_ = false; + + const wrappedPromise = new Promise( ( resolve, reject ) => { + promise.then( + ( val ) => + hasCanceled_ ? reject( { isCanceled: true } ) : resolve( val ), + ( error ) => + hasCanceled_ ? reject( { isCanceled: true } ) : reject( error ) + ); + } ); + + return { + promise: wrappedPromise, + cancel() { + hasCanceled_ = true; + }, + }; +}; From 0acd56c658f632525b74deae5fffa757c4dec5ba Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 27 May 2021 16:04:30 +0100 Subject: [PATCH 30/52] Remove isMounted anti pattern in favour of explictly cancelled promise pattern See https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html --- .../link-control/use-remote-url-data.js | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index 13f85062df5d55..4399bd8a5966fe 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -10,7 +10,6 @@ import { useSelect } from '@wordpress/data'; import { useState, useEffect, useRef, useCallback } from '@wordpress/element'; function useRemoteUrlData( url ) { - const isMounted = useRef( false ); const cancelableFetch = useRef(); const [ richData, setRichData ] = useState( null ); @@ -23,26 +22,18 @@ function useRemoteUrlData( url ) { }; }, [] ); - // Avoid fetching or state updates if not mounted. - useEffect( () => { - isMounted.current = true; - return () => { - isMounted.current = false; - }; - }, [] ); - const cancelPendingFetch = useCallback( () => { if ( cancelableFetch.current ) { cancelableFetch.current.cancel(); } - } ); + }, [ cancelableFetch ] ); /** * Cancel any pending requests that were made for * stale URL values. */ useEffect( () => { - return cancelPendingFetch(); + cancelPendingFetch(); }, [ url ] ); /** @@ -69,21 +60,20 @@ function useRemoteUrlData( url ) { ); const urlData = await cancelableFetch.current.promise; - if ( isMounted.current ) { - setRichData( urlData ); - setIsFetching( false ); - } + // If the promise is cancelled then this will never run + // as we should fall into the `catch` below. + setRichData( urlData ); + setIsFetching( false ); } catch ( error ) { - if ( error && error.isCanceled ) { + if ( error?.isCanceled ) { return; // bail if canceled to avoid setting state } - if ( isMounted.current ) { - setIsFetching( false ); - } + + setIsFetching( false ); } }; - if ( url?.length && isMounted.current && fetchRemoteUrlData ) { + if ( url?.length && fetchRemoteUrlData ) { fetchRichData(); } }, [ url ] ); From ba57bb1e0e1ff2a499f05cd63475797d6cab685e Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 27 May 2021 16:06:27 +0100 Subject: [PATCH 31/52] Update comments to make reason for cancelling on URL change clearer --- .../src/components/link-control/use-remote-url-data.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index 4399bd8a5966fe..6aad13f52a4f5c 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -30,7 +30,9 @@ function useRemoteUrlData( url ) { /** * Cancel any pending requests that were made for - * stale URL values. + * stale URL values. If the component does not unmount + * we must handle cancelling the current request if + * the URL changes otherwise we will see stale data. */ useEffect( () => { cancelPendingFetch(); From cfa5775a38fc07408809a28d35916273707149f3 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 27 May 2021 16:24:48 +0100 Subject: [PATCH 32/52] Fix broken tests by disabling rich reviews in those tests which do not exercise this feature --- .../src/components/link-control/test/index.js | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index 888db4c96f956a..8eed6df7a69c86 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -27,7 +27,15 @@ lodash.debounce = jest.fn( ( callback ) => { } ); const mockFetchSearchSuggestions = jest.fn(); -const mockFetchRemoteUrlData = jest.fn(); + +/** + * The call to the real method `fetchRemoteUrlData` is wrapped in a promise in order to make it cancellable. + * Therefore if we pass any value as the mock of `fetchRemoteUrlData` then ALL of the tests will require + * addition code to handle the async nature of `fetchRemoteUrlData`. This is unecessary. Instead we default + * to an undefined value which will ensure that the code under test does not call `fetchRemoteUrlData`. Only + * when we are testing the "rich previews" to we update this value with a true mock. + */ +let mockFetchRemoteUrlData; jest.mock( '@wordpress/data/src/components/use-select', () => { // This allows us to tweak the returned value on each test @@ -39,11 +47,6 @@ useSelect.mockImplementation( () => ( { fetchRemoteUrlData: mockFetchRemoteUrlData, } ) ); -// jest.mock( '@wordpress/data/src/components/use-select', () => () => ( { -// fetchSearchSuggestions: mockFetchSearchSuggestions, -// fetchRemoteUrlData: mockFetchRemoteUrlData, -// } ) ); - jest.mock( '@wordpress/data/src/components/use-dispatch', () => ( { useDispatch: () => ( { saveEntityRecords: jest.fn() } ), } ) ); @@ -75,7 +78,7 @@ afterEach( () => { container.remove(); container = null; mockFetchSearchSuggestions.mockReset(); - mockFetchRemoteUrlData.mockReset(); + mockFetchRemoteUrlData?.mockReset(); // conditionally reset as it may NOT be a mock } ); function getURLInput() { @@ -1738,6 +1741,17 @@ describe( 'Rich link previews', () => { url: 'https://www.wordpress.org', type: 'URL', }; + + beforeAll( () => { + /** + * These tests require that we exercise the `fetchRemoteUrlData` function. + * We are therefore overwriting the mock "placeholder" with a true jest mock + * which will cause the code under test to execute the code which fetches + * rich previews. + */ + mockFetchRemoteUrlData = jest.fn(); + } ); + it( 'should display a rich preview when data is available', async () => { mockFetchRemoteUrlData.mockImplementation( () => Promise.resolve( { @@ -1941,4 +1955,9 @@ describe( 'Rich link previews', () => { expect( isFetchingRichPreview ).toBe( true ); expect( isRichLinkPreview ).toBe( false ); } ); + + afterAll( () => { + // Remove the mock to avoid edge cases in other tests. + mockFetchRemoteUrlData = undefined; + } ); } ); From fecdf866d251165fec7f05fa864d1faf8f37c9c5 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 27 May 2021 16:40:39 +0100 Subject: [PATCH 33/52] Move make cancelable util into own file --- .../link-control/make-cancelable.js | 28 ++++++++++++++++ .../link-control/use-create-page.js | 32 +++---------------- .../link-control/use-remote-url-data.js | 30 +---------------- 3 files changed, 34 insertions(+), 56 deletions(-) create mode 100644 packages/block-editor/src/components/link-control/make-cancelable.js diff --git a/packages/block-editor/src/components/link-control/make-cancelable.js b/packages/block-editor/src/components/link-control/make-cancelable.js new file mode 100644 index 00000000000000..50fa92e969fc1c --- /dev/null +++ b/packages/block-editor/src/components/link-control/make-cancelable.js @@ -0,0 +1,28 @@ +/** + * Creates a wrapper around a promise which allows it to be programmatically + * cancelled. + * See: https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html + * + * @param {Promise} promise the Promise to make cancelable + */ +const makeCancelable = ( promise ) => { + let hasCanceled_ = false; + + const wrappedPromise = new Promise( ( resolve, reject ) => { + promise.then( + ( val ) => + hasCanceled_ ? reject( { isCanceled: true } ) : resolve( val ), + ( error ) => + hasCanceled_ ? reject( { isCanceled: true } ) : reject( error ) + ); + } ); + + return { + promise: wrappedPromise, + cancel() { + hasCanceled_ = true; + }, + }; +}; + +export default makeCancelable; diff --git a/packages/block-editor/src/components/link-control/use-create-page.js b/packages/block-editor/src/components/link-control/use-create-page.js index 42e243a2625fd4..9a1018aa36de5b 100644 --- a/packages/block-editor/src/components/link-control/use-create-page.js +++ b/packages/block-editor/src/components/link-control/use-create-page.js @@ -4,6 +4,11 @@ import { __ } from '@wordpress/i18n'; import { useEffect, useState, useRef } from '@wordpress/element'; +/** + * Internal dependencies + */ +import makeCancelable from './make-cancelable'; + export default function useCreatePage( handleCreatePage ) { const cancelableCreateSuggestion = useRef(); const [ isCreatingPage, setIsCreatingPage ] = useState( false ); @@ -58,30 +63,3 @@ export default function useCreatePage( handleCreatePage ) { errorMessage, }; } - -/** - * Creates a wrapper around a promise which allows it to be programmatically - * cancelled. - * See: https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html - * - * @param {Promise} promise the Promise to make cancelable - */ -const makeCancelable = ( promise ) => { - let hasCanceled_ = false; - - const wrappedPromise = new Promise( ( resolve, reject ) => { - promise.then( - ( val ) => - hasCanceled_ ? reject( { isCanceled: true } ) : resolve( val ), - ( error ) => - hasCanceled_ ? reject( { isCanceled: true } ) : reject( error ) - ); - } ); - - return { - promise: wrappedPromise, - cancel() { - hasCanceled_ = true; - }, - }; -}; diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index 6aad13f52a4f5c..f4685227e1ad65 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -2,6 +2,7 @@ * Internal dependencies */ import { store as blockEditorStore } from '../../store'; +import makeCancelable from './make-cancelable'; /** * WordPress dependencies @@ -56,8 +57,6 @@ function useRemoteUrlData( url ) { try { cancelableFetch.current = makeCancelable( - // Using Promise.resolve to allow createSuggestion to return a - // non-Promise based value. fetchRemoteUrlData( url ) ); const urlData = await cancelableFetch.current.promise; @@ -87,30 +86,3 @@ function useRemoteUrlData( url ) { } export default useRemoteUrlData; - -/** - * Creates a wrapper around a promise which allows it to be programmatically - * cancelled. - * See: https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html - * - * @param {Promise} promise the Promise to make cancelable - */ -const makeCancelable = ( promise ) => { - let hasCanceled_ = false; - - const wrappedPromise = new Promise( ( resolve, reject ) => { - promise.then( - ( val ) => - hasCanceled_ ? reject( { isCanceled: true } ) : resolve( val ), - ( error ) => - hasCanceled_ ? reject( { isCanceled: true } ) : reject( error ) - ); - } ); - - return { - promise: wrappedPromise, - cancel() { - hasCanceled_ = true; - }, - }; -}; From 5030f96e732622b0c51d643c1c93081b05972e34 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 9 Jun 2021 09:23:06 +0100 Subject: [PATCH 34/52] Allow for passing options to fetchRemoteUrlData This enables the usage of AbortController signal --- .../src/fetch/__experimental-fetch-remote-url-data.js | 9 +++++---- .../src/components/provider/use-block-editor-settings.js | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js b/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js index 93602fe4149b18..ed2e453e0406eb 100644 --- a/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js +++ b/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js @@ -15,8 +15,8 @@ import { addQueryArgs, prependHTTP } from '@wordpress/url'; * eg: tag, favicon...etc. * * @async - * @param {string} url - * + * @param {string} url the URL to request details from. + * @param {Object?} options any options to pass to the underlying fetch. * @example * ```js * import { __experimentalFetchRemoteUrlData as fetchRemoteUrlData } from '@wordpress/core-data'; @@ -26,12 +26,12 @@ import { addQueryArgs, prependHTTP } from '@wordpress/url'; * export function initialize( id, settings ) { * * settings.__experimentalFetchRemoteUrlData = ( - * url + * url * ) => fetchRemoteUrlData( url ); * ``` * @return {Promise< WPRemoteUrlData[] >} Remote URL data. */ -const fetchRemoteUrlData = async ( url ) => { +const fetchRemoteUrlData = async ( url, options = {} ) => { const endpoint = '/__experimental/url-details'; const args = { @@ -40,6 +40,7 @@ const fetchRemoteUrlData = async ( url ) => { return apiFetch( { path: addQueryArgs( endpoint, args ), + ...options, } ); }; diff --git a/packages/editor/src/components/provider/use-block-editor-settings.js b/packages/editor/src/components/provider/use-block-editor-settings.js index 158fa8d02f5363..e1a0be0b1b08ae 100644 --- a/packages/editor/src/components/provider/use-block-editor-settings.js +++ b/packages/editor/src/components/provider/use-block-editor-settings.js @@ -102,8 +102,8 @@ function useBlockEditorSettings( settings, hasTemplate ) { __experimentalReusableBlocks: reusableBlocks, __experimentalFetchLinkSuggestions: ( search, searchOptions ) => fetchLinkSuggestions( search, searchOptions, settings ), - __experimentalFetchRemoteUrlData: ( url ) => - fetchRemoteUrlData( url ), + __experimentalFetchRemoteUrlData: ( url, options ) => + fetchRemoteUrlData( url, options ), __experimentalCanUserUseUnfilteredHTML: canUseUnfilteredHTML, __experimentalUndo: undo, outlineMode: hasTemplate, From 08d3e39acb2655383ed23ea9382b1e4c113e20f7 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Fri, 4 Jun 2021 10:48:01 +0100 Subject: [PATCH 35/52] Use AbortController to cancel requests rather than cancellable promise wrapper --- .../link-control/use-remote-url-data.js | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index f4685227e1ad65..c1d87f257599d0 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -2,16 +2,17 @@ * Internal dependencies */ import { store as blockEditorStore } from '../../store'; -import makeCancelable from './make-cancelable'; /** * WordPress dependencies */ import { useSelect } from '@wordpress/data'; -import { useState, useEffect, useRef, useCallback } from '@wordpress/element'; +import { useState, useEffect, useCallback } from '@wordpress/element'; function useRemoteUrlData( url ) { - const cancelableFetch = useRef(); + // eslint-disable-next-line no-undef + let controller; + let signal; const [ richData, setRichData ] = useState( null ); const [ isFetching, setIsFetching ] = useState( false ); @@ -24,10 +25,8 @@ function useRemoteUrlData( url ) { }, [] ); const cancelPendingFetch = useCallback( () => { - if ( cancelableFetch.current ) { - cancelableFetch.current.cancel(); - } - }, [ cancelableFetch ] ); + controller?.abort(); + }, [ controller ] ); /** * Cancel any pending requests that were made for @@ -56,17 +55,20 @@ function useRemoteUrlData( url ) { setRichData( null ); try { - cancelableFetch.current = makeCancelable( - fetchRemoteUrlData( url ) - ); - const urlData = await cancelableFetch.current.promise; + // eslint-disable-next-line no-undef + controller = new AbortController(); + signal = controller.signal; + + const urlData = await fetchRemoteUrlData( url, { + signal, + } ); // If the promise is cancelled then this will never run // as we should fall into the `catch` below. setRichData( urlData ); setIsFetching( false ); } catch ( error ) { - if ( error?.isCanceled ) { + if ( signal?.aborted ) { return; // bail if canceled to avoid setting state } From 7749b6e3de75ed3ce2081d1eb01d191583aa36a0 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Fri, 4 Jun 2021 15:09:31 +0100 Subject: [PATCH 36/52] Force remount preview when the URL changes Previously a URL might change and the component would not remount which could lead to edge cases. Addresses https://github.com/WordPress/gutenberg/pull/31464#discussion_r645436478 --- packages/block-editor/src/components/link-control/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index d2bac48450bb0d..2300da61097a8d 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -249,6 +249,7 @@ function LinkControl( { { value && ! isEditingLink && ! isCreatingPage && ( <LinkPreview + key={ value?.url } // force remount when URL changes to avoid race conditions for rich previews value={ value } onEditClick={ () => setIsEditingLink( true ) } /> From 1d694c9249043a07f8c00649bda691aa64fe90df Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Fri, 4 Jun 2021 15:11:39 +0100 Subject: [PATCH 37/52] Implement simpler hook via abortable fetch Based on https://codesandbox.io/s/dry-pond-43qy2?file=/src/link-control.js:187-201 --- .../link-control/use-remote-url-data.js | 93 +++++++++---------- 1 file changed, 44 insertions(+), 49 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index c1d87f257599d0..24ababc0ceeae8 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -7,13 +7,9 @@ import { store as blockEditorStore } from '../../store'; * WordPress dependencies */ import { useSelect } from '@wordpress/data'; -import { useState, useEffect, useCallback } from '@wordpress/element'; +import { useState, useEffect } from '@wordpress/element'; function useRemoteUrlData( url ) { - // eslint-disable-next-line no-undef - let controller; - let signal; - const [ richData, setRichData ] = useState( null ); const [ isFetching, setIsFetching ] = useState( false ); @@ -24,61 +20,60 @@ function useRemoteUrlData( url ) { }; }, [] ); - const cancelPendingFetch = useCallback( () => { - controller?.abort(); - }, [ controller ] ); - - /** - * Cancel any pending requests that were made for - * stale URL values. If the component does not unmount - * we must handle cancelling the current request if - * the URL changes otherwise we will see stale data. - */ - useEffect( () => { - cancelPendingFetch(); - }, [ url ] ); + function abortableFetch( theUrl, signal ) { + return new Promise( ( resolve, reject ) => { + // Listen once on the AbortController signal + // and reject the promise if abort is triggered. + signal.addEventListener( + 'abort', + () => { + reject(); + }, + { once: true } + ); + + return fetchRemoteUrlData( theUrl, { + signal, + } ) + .then( ( data ) => resolve( data ) ) + .catch( () => reject( 'aborted' ) ); + } ); + } - /** - * Cancel any pending requests on "unmount" - */ useEffect( () => { - return () => { - cancelPendingFetch(); - }; - }, [] ); + // eslint-disable-next-line no-undef + let controller; - useEffect( () => { - const fetchRichData = async () => { + const fetchRichData = () => { setIsFetching( true ); - // Clear the data if the URL changes to avoid stale data in hook consumer. - setRichData( null ); - - try { - // eslint-disable-next-line no-undef - controller = new AbortController(); - signal = controller.signal; - - const urlData = await fetchRemoteUrlData( url, { - signal, + // eslint-disable-next-line no-undef + controller = new AbortController(); + + abortableFetch( url, controller.signal ) + .then( ( urlData ) => { + setRichData( urlData ); + setIsFetching( false ); + } ) + .catch( ( error ) => { + // Avoid setting state on unmounted component + if ( ! 'aborted' === error ) { + setIsFetching( false ); + } } ); - - // If the promise is cancelled then this will never run - // as we should fall into the `catch` below. - setRichData( urlData ); - setIsFetching( false ); - } catch ( error ) { - if ( signal?.aborted ) { - return; // bail if canceled to avoid setting state - } - - setIsFetching( false ); - } }; + // Only make the request if we have an actual URL + // and the fetching util is available. In some editors + // there may not be such a util. if ( url?.length && fetchRemoteUrlData ) { fetchRichData(); } + + // When the URL changes the abort the current request + return () => { + controller?.abort(); + }; }, [ url ] ); return { From 621119cc735799611baa9dcf435622d66e5346d3 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Fri, 4 Jun 2021 17:15:53 +0100 Subject: [PATCH 38/52] Fix to ensure correct handling of aborted vs standard fetch errors Avoid situation where requests which 404 do not result in the fetching UI being reset. --- .../src/components/link-control/use-remote-url-data.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index 24ababc0ceeae8..c88eb547fd817d 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -27,7 +27,7 @@ function useRemoteUrlData( url ) { signal.addEventListener( 'abort', () => { - reject(); + reject( 'aborted' ); }, { once: true } ); @@ -36,7 +36,7 @@ function useRemoteUrlData( url ) { signal, } ) .then( ( data ) => resolve( data ) ) - .catch( () => reject( 'aborted' ) ); + .catch( () => reject() ); } ); } @@ -57,8 +57,9 @@ function useRemoteUrlData( url ) { } ) .catch( ( error ) => { // Avoid setting state on unmounted component - if ( ! 'aborted' === error ) { + if ( 'aborted' !== error ) { setIsFetching( false ); + setRichData( null ); } } ); }; From 1bdce6db3b78f49e082f805e39fdb45e4add0ac4 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Fri, 4 Jun 2021 17:21:34 +0100 Subject: [PATCH 39/52] Add test to cover resetting fetching state if rich data requests fails --- .../src/components/link-control/test/index.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index 8eed6df7a69c86..13c7879e888ff3 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -1956,6 +1956,34 @@ describe( 'Rich link previews', () => { expect( isRichLinkPreview ).toBe( false ); } ); + it( 'should remove fetching UI indicators and fallback to standard preview if request for rich preview results in an error', async () => { + const simulateFailedFetch = () => Promise.reject(); + + mockFetchRemoteUrlData.mockImplementation( simulateFailedFetch ); + + act( () => { + render( <LinkControl value={ selectedLink } />, container ); + } ); + + // mockFetchRemoteUrlData resolves on next "tick" of event loop + await act( async () => { + await eventLoopTick(); + } ); + + const linkPreview = container.querySelector( + "[aria-label='Currently selected']" + ); + + const isFetchingRichPreview = linkPreview.classList.contains( + 'is-fetching' + ); + + const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); + + expect( isFetchingRichPreview ).toBe( false ); + expect( isRichLinkPreview ).toBe( false ); + } ); + afterAll( () => { // Remove the mock to avoid edge cases in other tests. mockFetchRemoteUrlData = undefined; From dad3a702f450247829ff18b596120acd2d3d058c Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Mon, 7 Jun 2021 11:06:35 +0100 Subject: [PATCH 40/52] Simplify hook implementation --- .../link-control/use-remote-url-data.js | 55 +++++-------------- 1 file changed, 15 insertions(+), 40 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index c88eb547fd817d..d069ee6c94c32b 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -20,61 +20,36 @@ function useRemoteUrlData( url ) { }; }, [] ); - function abortableFetch( theUrl, signal ) { - return new Promise( ( resolve, reject ) => { - // Listen once on the AbortController signal - // and reject the promise if abort is triggered. - signal.addEventListener( - 'abort', - () => { - reject( 'aborted' ); - }, - { once: true } - ); - - return fetchRemoteUrlData( theUrl, { - signal, - } ) - .then( ( data ) => resolve( data ) ) - .catch( () => reject() ); - } ); - } - useEffect( () => { - // eslint-disable-next-line no-undef - let controller; - - const fetchRichData = () => { + // Only make the request if we have an actual URL + // and the fetching util is available. In some editors + // there may not be such a util. + if ( url?.length && fetchRemoteUrlData ) { setIsFetching( true ); // eslint-disable-next-line no-undef - controller = new AbortController(); + const controller = new AbortController(); + const signal = controller.signal; - abortableFetch( url, controller.signal ) + fetchRemoteUrlData( url, { + signal, + } ) .then( ( urlData ) => { setRichData( urlData ); setIsFetching( false ); } ) - .catch( ( error ) => { + .catch( () => { // Avoid setting state on unmounted component - if ( 'aborted' !== error ) { + if ( ! signal?.aborted ) { setIsFetching( false ); setRichData( null ); } } ); - }; - - // Only make the request if we have an actual URL - // and the fetching util is available. In some editors - // there may not be such a util. - if ( url?.length && fetchRemoteUrlData ) { - fetchRichData(); + // Cleanup: when the URL changes the abort the current request + return () => { + controller.abort(); + }; } - - // When the URL changes the abort the current request - return () => { - controller?.abort(); - }; }, [ url ] ); return { From d7268835cd5b34ebe00357ed79454a959a4c5dbf Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Mon, 7 Jun 2021 11:21:55 +0100 Subject: [PATCH 41/52] Guard for legacy browsers --- .../components/link-control/use-remote-url-data.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index d069ee6c94c32b..f146fd0406ed43 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -24,11 +24,15 @@ function useRemoteUrlData( url ) { // Only make the request if we have an actual URL // and the fetching util is available. In some editors // there may not be such a util. - if ( url?.length && fetchRemoteUrlData ) { + if ( + url?.length && + fetchRemoteUrlData && + typeof AbortController !== 'undefined' + ) { setIsFetching( true ); - // eslint-disable-next-line no-undef - const controller = new AbortController(); + const controller = new window.AbortController(); + const signal = controller.signal; fetchRemoteUrlData( url, { @@ -40,7 +44,7 @@ function useRemoteUrlData( url ) { } ) .catch( () => { // Avoid setting state on unmounted component - if ( ! signal?.aborted ) { + if ( ! signal.aborted ) { setIsFetching( false ); setRichData( null ); } From 429a42a1c7fac52985a4868ca0acd1c3cfd68166 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Mon, 7 Jun 2021 11:37:37 +0100 Subject: [PATCH 42/52] Refactor to useReducer --- .../link-control/use-remote-url-data.js | 52 ++++++++++++++----- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/packages/block-editor/src/components/link-control/use-remote-url-data.js b/packages/block-editor/src/components/link-control/use-remote-url-data.js index f146fd0406ed43..e57c716f84d777 100644 --- a/packages/block-editor/src/components/link-control/use-remote-url-data.js +++ b/packages/block-editor/src/components/link-control/use-remote-url-data.js @@ -7,11 +7,37 @@ import { store as blockEditorStore } from '../../store'; * WordPress dependencies */ import { useSelect } from '@wordpress/data'; -import { useState, useEffect } from '@wordpress/element'; +import { useEffect, useReducer } from '@wordpress/element'; + +function reducer( state, action ) { + switch ( action.type ) { + case 'RESOLVED': + return { + ...state, + isFetching: false, + richData: action.richData, + }; + case 'ERROR': + return { + ...state, + isFetching: false, + richData: null, + }; + case 'LOADING': + return { + ...state, + isFetching: true, + }; + default: + throw new Error( `Unexpected action type ${ action.type }` ); + } +} function useRemoteUrlData( url ) { - const [ richData, setRichData ] = useState( null ); - const [ isFetching, setIsFetching ] = useState( false ); + const [ state, dispatch ] = useReducer( reducer, { + richData: null, + isFetching: false, + } ); const { fetchRemoteUrlData } = useSelect( ( select ) => { const { getSettings } = select( blockEditorStore ); @@ -29,7 +55,9 @@ function useRemoteUrlData( url ) { fetchRemoteUrlData && typeof AbortController !== 'undefined' ) { - setIsFetching( true ); + dispatch( { + type: 'LOADING', + } ); const controller = new window.AbortController(); @@ -39,14 +67,17 @@ function useRemoteUrlData( url ) { signal, } ) .then( ( urlData ) => { - setRichData( urlData ); - setIsFetching( false ); + dispatch( { + type: 'RESOLVED', + richData: urlData, + } ); } ) .catch( () => { // Avoid setting state on unmounted component if ( ! signal.aborted ) { - setIsFetching( false ); - setRichData( null ); + dispatch( { + type: 'ERROR', + } ); } } ); // Cleanup: when the URL changes the abort the current request @@ -56,10 +87,7 @@ function useRemoteUrlData( url ) { } }, [ url ] ); - return { - richData, - isFetching, - }; + return state; } export default useRemoteUrlData; From 654b0d4478f3798abce04a606025d85529b7b873 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Mon, 7 Jun 2021 12:08:06 +0100 Subject: [PATCH 43/52] Disable rich previews by default and enable only in inline text by default --- .../src/components/link-control/index.js | 2 + .../components/link-control/link-preview.js | 11 ++- .../test/__snapshots__/index.js.snap | 84 +++++++++++++++++++ .../src/components/link-control/test/index.js | 67 +++++++++++++-- packages/format-library/src/link/inline.js | 1 + 5 files changed, 156 insertions(+), 9 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 2300da61097a8d..7936e0859371f0 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -115,6 +115,7 @@ function LinkControl( { suggestionsQuery = {}, noURLSuggestion = false, createSuggestionButtonText, + richPreviews = false, } ) { if ( withCreateSuggestion === undefined && createSuggestion ) { withCreateSuggestion = true; @@ -252,6 +253,7 @@ function LinkControl( { key={ value?.url } // force remount when URL changes to avoid race conditions for rich previews value={ value } onEditClick={ () => setIsEditingLink( true ) } + richPreviews={ richPreviews } /> ) } diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index 0757b1070750fe..8a59618425dbc2 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -22,8 +22,15 @@ import { ViewerSlot } from './viewer-slot'; import useRemoteUrlData from './use-remote-url-data'; -export default function LinkPreview( { value, onEditClick } ) { - const { richData, isFetching } = useRemoteUrlData( value?.url ); +export default function LinkPreview( { + value, + onEditClick, + richPreviews = false, +} ) { + // Avoid fetching if rich previews are not desired. + const maybeRemoteURL = richPreviews ? value?.url : null; + + const { richData, isFetching } = useRemoteUrlData( maybeRemoteURL ); // Rich data may be an empty object so test for that. const hasRichData = richData && Object.keys( richData ).length; diff --git a/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap b/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap index 417d6996e6a247..c7f38246b4542f 100644 --- a/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap +++ b/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap @@ -60,3 +60,87 @@ exports[`Rich link previews should not fetch or display rich previews by default </button> </div> `; + +exports[`Rich link previews should not fetch or display rich previews by default 1`] = ` +.emotion-0 { + width: 1.4em; + height: 1.4em; + margin: -0.2em 0.1em 0; + vertical-align: middle; + fill: currentColor; +} + +<div + aria-label="Currently selected" + aria-selected="true" + class="block-editor-link-control__search-item is-current is-preview" +> + <div + class="block-editor-link-control__search-item-top" + > + <span + class="block-editor-link-control__search-item-header" + > + <span + class="block-editor-link-control__search-item-icon" + > + <svg + aria-hidden="true" + focusable="false" + height="24" + role="img" + viewBox="-2 -2 24 24" + width="24" + xmlns="http://www.w3.org/2000/svg" + > + <path + d="M9 0C4.03 0 0 4.03 0 9s4.03 9 9 9 9-4.03 9-9-4.03-9-9-9zM1.11 9.68h2.51c.04.91.167 1.814.38 2.7H1.84c-.403-.85-.65-1.764-.73-2.7zm8.57-5.4V1.19c.964.366 1.756 1.08 2.22 2 .205.347.386.708.54 1.08l-2.76.01zm3.22 1.35c.232.883.37 1.788.41 2.7H9.68v-2.7h3.22zM8.32 1.19v3.09H5.56c.154-.372.335-.733.54-1.08.462-.924 1.255-1.64 2.22-2.01zm0 4.44v2.7H4.7c.04-.912.178-1.817.41-2.7h3.21zm-4.7 2.69H1.11c.08-.936.327-1.85.73-2.7H4c-.213.886-.34 1.79-.38 2.7zM4.7 9.68h3.62v2.7H5.11c-.232-.883-.37-1.788-.41-2.7zm3.63 4v3.09c-.964-.366-1.756-1.08-2.22-2-.205-.347-.386-.708-.54-1.08l2.76-.01zm1.35 3.09v-3.04h2.76c-.154.372-.335.733-.54 1.08-.464.92-1.256 1.634-2.22 2v-.04zm0-4.44v-2.7h3.62c-.04.912-.178 1.817-.41 2.7H9.68zm4.71-2.7h2.51c-.08.936-.327 1.85-.73 2.7H14c.21-.87.337-1.757.38-2.65l.01-.05zm0-1.35c-.046-.894-.176-1.78-.39-2.65h2.16c.403.85.65 1.764.73 2.7l-2.5-.05zm1-4H13.6c-.324-.91-.793-1.76-1.39-2.52 1.244.56 2.325 1.426 3.14 2.52h.04zm-9.6-2.52c-.597.76-1.066 1.61-1.39 2.52H2.65c.815-1.094 1.896-1.96 3.14-2.52zm-3.15 12H4.4c.324.91.793 1.76 1.39 2.52-1.248-.567-2.33-1.445-3.14-2.55l-.01.03zm9.56 2.52c.597-.76 1.066-1.61 1.39-2.52h1.76c-.82 1.08-1.9 1.933-3.14 2.48l-.01.04z" + /> + </svg> + </span> + <span + class="block-editor-link-control__search-item-details" + > + <a + class="components-external-link block-editor-link-control__search-item-title" + href="https://www.wordpress.org" + rel="external noreferrer noopener" + target="_blank" + > + Wordpress.org + <span + class="components-visually-hidden" + > + (opens in a new tab) + </span> + <svg + aria-hidden="true" + class="components-external-link__icon emotion-0 emotion-1" + focusable="false" + height="24" + role="img" + viewBox="0 0 24 24" + width="24" + xmlns="http://www.w3.org/2000/svg" + > + <path + d="M18.2 17c0 .7-.6 1.2-1.2 1.2H7c-.7 0-1.2-.6-1.2-1.2V7c0-.7.6-1.2 1.2-1.2h3.2V4.2H7C5.5 4.2 4.2 5.5 4.2 7v10c0 1.5 1.2 2.8 2.8 2.8h10c1.5 0 2.8-1.2 2.8-2.8v-3.6h-1.5V17zM14.9 3v1.5h3.7l-6.4 6.4 1.1 1.1 6.4-6.4v3.7h1.5V3h-6.3z" + /> + </svg> + </a> + <span + class="block-editor-link-control__search-item-info" + > + wordpress.org + </span> + </span> + </span> + <button + class="components-button block-editor-link-control__search-item-action is-secondary" + type="button" + > + Edit + </button> + </div> +</div> +`; diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index 13c7879e888ff3..5d348031c069bd 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -1752,7 +1752,7 @@ describe( 'Rich link previews', () => { mockFetchRemoteUrlData = jest.fn(); } ); - it( 'should display a rich preview when data is available', async () => { + it( 'should not fetch or display rich previews by default', async () => { mockFetchRemoteUrlData.mockImplementation( () => Promise.resolve( { title: @@ -1779,6 +1779,41 @@ describe( 'Rich link previews', () => { const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); + expect( mockFetchRemoteUrlData ).not.toHaveBeenCalled(); + expect( isRichLinkPreview ).toBe( false ); + expect( linkPreview ).toMatchSnapshot(); + } ); + + it( 'should display a rich preview when data is available', async () => { + mockFetchRemoteUrlData.mockImplementation( () => + Promise.resolve( { + title: + 'Blog Tool, Publishing Platform, and CMS \u2014 WordPress.org', + icon: 'https://s.w.org/favicon.ico?2', + description: + 'Open source software which you can use to easily create a beautiful website, blog, or app.', + image: 'https://s.w.org/images/home/screen-themes.png?3', + } ) + ); + + act( () => { + render( + <LinkControl value={ selectedLink } richPreviews />, + container + ); + } ); + + // mockFetchRemoteUrlData resolves on next "tick" of event loop + await act( async () => { + await eventLoopTick(); + } ); + + const linkPreview = container.querySelector( + "[aria-label='Currently selected']" + ); + + const isRichLinkPreview = linkPreview.classList.contains( 'is-rich' ); + expect( isRichLinkPreview ).toBe( true ); expect( linkPreview ).toMatchSnapshot(); } ); @@ -1794,7 +1829,10 @@ describe( 'Rich link previews', () => { ); act( () => { - render( <LinkControl value={ selectedLink } />, container ); + render( + <LinkControl value={ selectedLink } richPreviews />, + container + ); } ); // mockFetchRemoteUrlData resolves on next "tick" of event loop @@ -1830,7 +1868,10 @@ describe( 'Rich link previews', () => { ); act( () => { - render( <LinkControl value={ selectedLink } />, container ); + render( + <LinkControl value={ selectedLink } richPreviews />, + container + ); } ); // mockFetchRemoteUrlData resolves on next "tick" of event loop @@ -1873,7 +1914,10 @@ describe( 'Rich link previews', () => { } ); act( () => { - render( <LinkControl value={ selectedLink } />, container ); + render( + <LinkControl value={ selectedLink } richPreviews />, + container + ); } ); // mockFetchRemoteUrlData resolves on next "tick" of event loop @@ -1909,7 +1953,10 @@ describe( 'Rich link previews', () => { ); act( () => { - render( <LinkControl value={ selectedLink } />, container ); + render( + <LinkControl value={ selectedLink } richPreviews />, + container + ); } ); // mockFetchRemoteUrlData resolves on next "tick" of event loop @@ -1935,7 +1982,10 @@ describe( 'Rich link previews', () => { mockFetchRemoteUrlData.mockImplementation( nonResolvingPromise ); act( () => { - render( <LinkControl value={ selectedLink } />, container ); + render( + <LinkControl value={ selectedLink } richPreviews />, + container + ); } ); // mockFetchRemoteUrlData resolves on next "tick" of event loop @@ -1962,7 +2012,10 @@ describe( 'Rich link previews', () => { mockFetchRemoteUrlData.mockImplementation( simulateFailedFetch ); act( () => { - render( <LinkControl value={ selectedLink } />, container ); + render( + <LinkControl value={ selectedLink } richPreviews />, + container + ); } ); // mockFetchRemoteUrlData resolves on next "tick" of event loop diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 4c78c9d1475a99..31c10f902c55e2 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -140,6 +140,7 @@ function InlineLinkUI( { value={ linkValue } onChange={ onChangeLink } forceIsEditingLink={ addingLink } + richPreviews /> </Popover> ); From b722bd71beaa05117a76a277f7655df960f269e4 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Mon, 7 Jun 2021 16:21:59 +0100 Subject: [PATCH 44/52] Update packages/editor/src/components/provider/use-block-editor-settings.js Co-authored-by: Kai Hao <kevin830726@gmail.com> --- .../src/components/provider/use-block-editor-settings.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/editor/src/components/provider/use-block-editor-settings.js b/packages/editor/src/components/provider/use-block-editor-settings.js index e1a0be0b1b08ae..4ad83ca37b1967 100644 --- a/packages/editor/src/components/provider/use-block-editor-settings.js +++ b/packages/editor/src/components/provider/use-block-editor-settings.js @@ -102,8 +102,7 @@ function useBlockEditorSettings( settings, hasTemplate ) { __experimentalReusableBlocks: reusableBlocks, __experimentalFetchLinkSuggestions: ( search, searchOptions ) => fetchLinkSuggestions( search, searchOptions, settings ), - __experimentalFetchRemoteUrlData: ( url, options ) => - fetchRemoteUrlData( url, options ), + __experimentalFetchRemoteUrlData: fetchRemoteUrlData, __experimentalCanUserUseUnfilteredHTML: canUseUnfilteredHTML, __experimentalUndo: undo, outlineMode: hasTemplate, From b236b71cc70f164fc0f8fd854fddc0751adfca04 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Mon, 7 Jun 2021 16:38:57 +0100 Subject: [PATCH 45/52] Update to hasRichPreviews Address feedback from https://github.com/WordPress/gutenberg/pull/31464#discussion_r646582909 --- .../src/components/link-control/index.js | 4 ++-- .../src/components/link-control/link-preview.js | 4 ++-- .../src/components/link-control/test/index.js | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 7936e0859371f0..85b5e45f2d95c9 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -115,7 +115,7 @@ function LinkControl( { suggestionsQuery = {}, noURLSuggestion = false, createSuggestionButtonText, - richPreviews = false, + hasRichPreviews = false, } ) { if ( withCreateSuggestion === undefined && createSuggestion ) { withCreateSuggestion = true; @@ -253,7 +253,7 @@ function LinkControl( { key={ value?.url } // force remount when URL changes to avoid race conditions for rich previews value={ value } onEditClick={ () => setIsEditingLink( true ) } - richPreviews={ richPreviews } + hasRichPreviews={ hasRichPreviews } /> ) } diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index 8a59618425dbc2..82f16421c8a7b6 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -25,10 +25,10 @@ import useRemoteUrlData from './use-remote-url-data'; export default function LinkPreview( { value, onEditClick, - richPreviews = false, + hasRichPreviews = false, } ) { // Avoid fetching if rich previews are not desired. - const maybeRemoteURL = richPreviews ? value?.url : null; + const maybeRemoteURL = hasRichPreviews ? value?.url : null; const { richData, isFetching } = useRemoteUrlData( maybeRemoteURL ); diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index 5d348031c069bd..0913c597a3efdd 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -1798,7 +1798,7 @@ describe( 'Rich link previews', () => { act( () => { render( - <LinkControl value={ selectedLink } richPreviews />, + <LinkControl value={ selectedLink } hasRichPreviews />, container ); } ); @@ -1830,7 +1830,7 @@ describe( 'Rich link previews', () => { act( () => { render( - <LinkControl value={ selectedLink } richPreviews />, + <LinkControl value={ selectedLink } hasRichPreviews />, container ); } ); @@ -1869,7 +1869,7 @@ describe( 'Rich link previews', () => { act( () => { render( - <LinkControl value={ selectedLink } richPreviews />, + <LinkControl value={ selectedLink } hasRichPreviews />, container ); } ); @@ -1915,7 +1915,7 @@ describe( 'Rich link previews', () => { act( () => { render( - <LinkControl value={ selectedLink } richPreviews />, + <LinkControl value={ selectedLink } hasRichPreviews />, container ); } ); @@ -1954,7 +1954,7 @@ describe( 'Rich link previews', () => { act( () => { render( - <LinkControl value={ selectedLink } richPreviews />, + <LinkControl value={ selectedLink } hasRichPreviews />, container ); } ); @@ -1983,7 +1983,7 @@ describe( 'Rich link previews', () => { act( () => { render( - <LinkControl value={ selectedLink } richPreviews />, + <LinkControl value={ selectedLink } hasRichPreviews />, container ); } ); @@ -2013,7 +2013,7 @@ describe( 'Rich link previews', () => { act( () => { render( - <LinkControl value={ selectedLink } richPreviews />, + <LinkControl value={ selectedLink } hasRichPreviews />, container ); } ); From e450f66127cffc1ffe9ba6a894ffa47cd7cee763 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Mon, 7 Jun 2021 18:17:27 +0100 Subject: [PATCH 46/52] Use correct prop to enable rich previews in rich text --- packages/format-library/src/link/inline.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 31c10f902c55e2..d015267a8e38f7 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -140,7 +140,7 @@ function InlineLinkUI( { value={ linkValue } onChange={ onChangeLink } forceIsEditingLink={ addingLink } - richPreviews + hasRichPreviews /> </Popover> ); From 4e48952d370bb196503e87271863143999488437 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Tue, 8 Jun 2021 10:44:40 +0100 Subject: [PATCH 47/52] Fix e2e test to allow unlinking when rich data obscures block toolbar --- packages/e2e-tests/specs/editor/various/links.test.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/links.test.js b/packages/e2e-tests/specs/editor/various/links.test.js index a33132a28ac69d..2b422916562753 100644 --- a/packages/e2e-tests/specs/editor/various/links.test.js +++ b/packages/e2e-tests/specs/editor/various/links.test.js @@ -233,7 +233,13 @@ describe( 'Links', () => { await createAndReselectLink(); // Click on the Unlink button - await page.click( 'button[aria-label="Unlink"]' ); + // await page.click( 'button[aria-label="Unlink"]' ); + + // Unlick via shortcut + // we do this to avoid an layout edge case whereby + // the rich link preview popover will obscure the block toolbar + // under very specific circumstances and screensizes. + await pressKeyWithModifier( 'primaryShift', 'K' ); // The link should have been removed expect( await getEditedPostContent() ).toMatchSnapshot(); From de2824e55206cd0b217f49b56626cca3c9d41450 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Tue, 8 Jun 2021 15:39:08 +0100 Subject: [PATCH 48/52] Add permenant missing data placeholders with loading animation --- .../components/link-control/link-preview.js | 35 +++++++--- .../src/components/link-control/style.scss | 70 ++++++++++++++----- 2 files changed, 78 insertions(+), 27 deletions(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index 82f16421c8a7b6..fca3635bfb6d0b 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -89,24 +89,37 @@ export default function LinkPreview( { </Button> <ViewerSlot fillProps={ value } /> </div> - { ( richData?.image || richData?.description ) && ( - <div className="block-editor-link-control__search-item-bottom"> + + <div className="block-editor-link-control__search-item-bottom"> + <div + aria-hidden={ ! richData?.image } + className={ classnames( + 'block-editor-link-control__search-item-image', + { + 'is-placeholder': ! richData?.image, + } + ) } + > { richData?.image && ( - <div className="block-editor-link-control__search-item-image"> - <img src={ richData?.image } alt="" /> - </div> + <img src={ richData?.image } alt="" /> + ) } + </div> + <div + aria-hidden={ ! richData?.description } + className={ classnames( + 'block-editor-link-control__search-item-description', + { + 'is-placeholder': ! richData?.description, + } ) } + > { richData?.description && ( - <Text - className="block-editor-link-control__search-item-description" - truncate - numberOfLines="2" - > + <Text truncate numberOfLines="2"> { richData.description } </Text> ) } </div> - ) } + </div> </div> ); } diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index 4dde495f82957d..b1b6958dbe5f8c 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -1,6 +1,18 @@ $block-editor-link-control-number-of-actions: 1; $preview-image-height: 140px; +@keyframes loadingpulse { + 0% { + opacity: 1; + } + 50% { + opacity: 0; + } + 100% { + opacity: 1; + } +} + .block-editor-link-control { position: relative; min-width: $modal-min-width; @@ -235,8 +247,29 @@ $preview-image-height: 140px; .block-editor-link-control__search-item-description { padding-top: 12px; margin: 0; - font-size: 0.9em; + &.is-placeholder { + margin-top: 12px; + padding-top: 0; + height: 28px; + display: flex; + flex-direction: column; + justify-content: space-around; + + &::before, + &::after { + display: block; + content: ""; + height: 0.7em; + width: 100%; + background-color: $gray-100; + border-radius: 3px; + } + } + + .components-text { + font-size: 0.9em; + } } .block-editor-link-control__search-item-image { @@ -250,6 +283,11 @@ $preview-image-height: 140px; border-radius: 2px; margin-top: 12px; + &.is-placeholder { + background-color: $gray-100; + border-radius: 3px; + } + img { display: block; // remove unwanted space below image max-width: 100%; @@ -271,10 +309,21 @@ $preview-image-height: 140px; width: 100%; } + .block-editor-link-control__search-item.is-fetching { - .block-editor-link-control__search-item-bottom { - opacity: 0; + .block-editor-link-control__search-item-description { + &::before, + &::after { + animation: loadingpulse 1s linear infinite; + animation-delay: 0.5s; // avoid animating for fast network responses + } + + } + + .block-editor-link-control__search-item-image { + animation: loadingpulse 1s linear infinite; + animation-delay: 0.5s; // avoid animating for fast network responses } .block-editor-link-control__search-item-icon { @@ -286,7 +335,7 @@ $preview-image-height: 140px; &::before { content: ""; display: block; - background-color: #999; + background-color: $gray-100; position: absolute; top: 0; left: 0; @@ -294,18 +343,7 @@ $preview-image-height: 140px; bottom: 0; border-radius: 100%; animation: loadingpulse 1s linear infinite; - } - } - - @keyframes loadingpulse { - 0% { - opacity: 1; - } - 50% { - opacity: 0; - } - 100% { - opacity: 1; + animation-delay: 0.5s; // avoid animating for fast network responses } } } From 0d55791ab5e7fc66ae779708ceb3e20afa9b23e2 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Wed, 9 Jun 2021 09:23:21 +0100 Subject: [PATCH 49/52] Fix tests --- .../components/link-control/link-preview.js | 60 ++++++++++--------- .../src/components/link-control/test/index.js | 6 +- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index fca3635bfb6d0b..17152cf8b5b5bb 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -90,36 +90,38 @@ export default function LinkPreview( { <ViewerSlot fillProps={ value } /> </div> - <div className="block-editor-link-control__search-item-bottom"> - <div - aria-hidden={ ! richData?.image } - className={ classnames( - 'block-editor-link-control__search-item-image', - { - 'is-placeholder': ! richData?.image, - } - ) } - > - { richData?.image && ( - <img src={ richData?.image } alt="" /> - ) } - </div> - <div - aria-hidden={ ! richData?.description } - className={ classnames( - 'block-editor-link-control__search-item-description', - { - 'is-placeholder': ! richData?.description, - } - ) } - > - { richData?.description && ( - <Text truncate numberOfLines="2"> - { richData.description } - </Text> - ) } + { ( hasRichData || isFetching ) && ( + <div className="block-editor-link-control__search-item-bottom"> + <div + aria-hidden={ ! richData?.image } + className={ classnames( + 'block-editor-link-control__search-item-image', + { + 'is-placeholder': ! richData?.image, + } + ) } + > + { richData?.image && ( + <img src={ richData?.image } alt="" /> + ) } + </div> + <div + aria-hidden={ ! richData?.description } + className={ classnames( + 'block-editor-link-control__search-item-description', + { + 'is-placeholder': ! richData?.description, + } + ) } + > + { richData?.description && ( + <Text truncate numberOfLines="2"> + { richData.description } + </Text> + ) } + </div> </div> - </div> + ) } </div> ); } diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index 0913c597a3efdd..3eed46f032e845 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -1898,7 +1898,7 @@ describe( 'Rich link previews', () => { } ); it.each( [ 'image', 'description' ] )( - 'should not display an %s fallback when it is missing from the rich data', + 'should display a fallback placeholder when %s it is missing from the rich data', async ( dataItem ) => { mockFetchRemoteUrlData.mockImplementation( () => { const data = { @@ -1935,10 +1935,10 @@ describe( 'Rich link previews', () => { expect( isRichLinkPreview ).toBe( true ); const missingDataItem = linkPreview.querySelector( - `.block-editor-link-control__search-item-${ dataItem }` + `.block-editor-link-control__search-item-${ dataItem }.is-placeholder` ); - expect( missingDataItem ).toBeFalsy(); + expect( missingDataItem ).toBeTruthy(); } ); From ebb93468fe138f10b6af024e63bd089db81ae838 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Wed, 9 Jun 2021 11:21:28 +0100 Subject: [PATCH 50/52] Update snapshots --- .../test/__snapshots__/index.js.snap | 125 +++++++++++++----- 1 file changed, 89 insertions(+), 36 deletions(-) diff --git a/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap b/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap index c7f38246b4542f..5687cb67c05932 100644 --- a/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap +++ b/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap @@ -2,7 +2,19 @@ exports[`Basic rendering should render 1`] = `"<div tabindex=\\"-1\\" class=\\"block-editor-link-control\\"><div class=\\"block-editor-link-control__search-input-wrapper\\"><form><div class=\\"components-base-control block-editor-url-input block-editor-link-control__search-input css-wdf2ti-Wrapper e1puf3u0\\"><div class=\\"components-base-control__field css-11vcxb9-StyledField e1puf3u1\\"><input required=\\"\\" class=\\"block-editor-url-input__input\\" type=\\"text\\" placeholder=\\"Search or type url\\" role=\\"combobox\\" aria-label=\\"URL\\" aria-expanded=\\"false\\" aria-autocomplete=\\"list\\" aria-owns=\\"block-editor-url-input-suggestions-0\\" value=\\"\\"></div></div><div class=\\"block-editor-link-control__search-actions\\"><button type=\\"submit\\" class=\\"components-button block-editor-link-control__search-submit has-icon\\" aria-label=\\"Submit\\"><svg width=\\"24\\" height=\\"24\\" xmlns=\\"http://www.w3.org/2000/svg\\" viewBox=\\"-2 -2 24 24\\" role=\\"img\\" aria-hidden=\\"true\\" focusable=\\"false\\"><path d=\\"M6.734 16.106l2.176-2.38-1.093-1.028-3.846 4.158 3.846 4.157 1.093-1.027-2.176-2.38h2.811c1.125 0 2.25.03 3.374 0 1.428-.001 3.362-.25 4.963-1.277 1.66-1.065 2.868-2.906 2.868-5.859 0-2.479-1.327-4.896-3.65-5.93-1.82-.813-3.044-.8-4.806-.788l-.567.002v1.5c.184 0 .368 0 .553-.002 1.82-.007 2.704-.014 4.21.657 1.854.827 2.76 2.657 2.76 4.561 0 2.472-.973 3.824-2.178 4.596-1.258.807-2.864 1.04-4.163 1.04h-.02c-1.115.03-2.229 0-3.344 0H6.734z\\"></path></svg></button></div></form></div><fieldset class=\\"block-editor-link-control__settings\\"><legend class=\\"components-visually-hidden\\">Currently selected link settings</legend><div class=\\"components-base-control components-toggle-control block-editor-link-control__setting css-wdf2ti-Wrapper e1puf3u0\\"><div class=\\"components-base-control__field css-11vcxb9-StyledField e1puf3u1\\"><span class=\\"components-form-toggle\\"><input class=\\"components-form-toggle__input\\" id=\\"inspector-toggle-control-0\\" type=\\"checkbox\\"><span class=\\"components-form-toggle__track\\"></span><span class=\\"components-form-toggle__thumb\\"></span></span><label for=\\"inspector-toggle-control-0\\" class=\\"components-toggle-control__label\\">Open in new tab</label></div></div></fieldset></div>"`; -exports[`Rich link previews should not fetch or display rich previews by default 1`] = ` +exports[`Rich link previews should display a rich preview when data is available 1`] = ` +.emotion-2 { + -webkit-box-orient: vertical; + -webkit-line-clamp: 2; + display: -webkit-box; + overflow: hidden; + color: #000; + line-height: 1.2; + margin: 0; + font-size: calc((13 / 13) * 13px); + font-weight: normal; +} + .emotion-0 { width: 1.4em; height: 1.4em; @@ -14,50 +26,91 @@ exports[`Rich link previews should not fetch or display rich previews by default <div aria-label="Currently selected" aria-selected="true" - class="block-editor-link-control__search-item is-current" + class="block-editor-link-control__search-item is-current is-rich is-preview" > - <span - class="block-editor-link-control__search-item-header" + <div + class="block-editor-link-control__search-item-top" > - <a - class="components-external-link block-editor-link-control__search-item-title" - href="https://www.wordpress.org" - rel="external noreferrer noopener" - target="_blank" + <span + class="block-editor-link-control__search-item-header" > - Wordpress.org <span - class="components-visually-hidden" + class="block-editor-link-control__search-item-icon is-image" > - (opens in a new tab) + <img + alt="" + src="https://s.w.org/favicon.ico?2" + /> </span> - <svg - aria-hidden="true" - class="components-external-link__icon emotion-0 emotion-1" - focusable="false" - height="24" - role="img" - viewBox="0 0 24 24" - width="24" - xmlns="http://www.w3.org/2000/svg" + <span + class="block-editor-link-control__search-item-details" > - <path - d="M18.2 17c0 .7-.6 1.2-1.2 1.2H7c-.7 0-1.2-.6-1.2-1.2V7c0-.7.6-1.2 1.2-1.2h3.2V4.2H7C5.5 4.2 4.2 5.5 4.2 7v10c0 1.5 1.2 2.8 2.8 2.8h10c1.5 0 2.8-1.2 2.8-2.8v-3.6h-1.5V17zM14.9 3v1.5h3.7l-6.4 6.4 1.1 1.1 6.4-6.4v3.7h1.5V3h-6.3z" - /> - </svg> - </a> - <span - class="block-editor-link-control__search-item-info" - > - wordpress.org + <a + class="components-external-link block-editor-link-control__search-item-title" + href="https://www.wordpress.org" + rel="external noreferrer noopener" + target="_blank" + > + Blog Tool, Publishing Platform, and CMS — WordPress.org + <span + class="components-visually-hidden" + > + (opens in a new tab) + </span> + <svg + aria-hidden="true" + class="components-external-link__icon emotion-0 emotion-1" + focusable="false" + height="24" + role="img" + viewBox="0 0 24 24" + width="24" + xmlns="http://www.w3.org/2000/svg" + > + <path + d="M18.2 17c0 .7-.6 1.2-1.2 1.2H7c-.7 0-1.2-.6-1.2-1.2V7c0-.7.6-1.2 1.2-1.2h3.2V4.2H7C5.5 4.2 4.2 5.5 4.2 7v10c0 1.5 1.2 2.8 2.8 2.8h10c1.5 0 2.8-1.2 2.8-2.8v-3.6h-1.5V17zM14.9 3v1.5h3.7l-6.4 6.4 1.1 1.1 6.4-6.4v3.7h1.5V3h-6.3z" + /> + </svg> + </a> + <span + class="block-editor-link-control__search-item-info" + > + wordpress.org + </span> + </span> </span> - </span> - <button - class="components-button block-editor-link-control__search-item-action is-secondary" - type="button" + <button + class="components-button block-editor-link-control__search-item-action is-secondary" + type="button" + > + Edit + </button> + </div> + <div + class="block-editor-link-control__search-item-bottom" > - Edit - </button> + <div + aria-hidden="false" + class="block-editor-link-control__search-item-image" + > + <img + alt="" + src="https://s.w.org/images/home/screen-themes.png?3" + /> + </div> + <div + aria-hidden="false" + class="block-editor-link-control__search-item-description" + > + <span + class="components-truncate components-text emotion-2 emotion-3 emotion-4" + data-wp-c16t="true" + data-wp-component="Text" + > + Open source software which you can use to easily create a beautiful website, blog, or app. + </span> + </div> + </div> </div> `; From 939f7be5dbb8b2dd5e6f04fbec4bd7037601db38 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Wed, 9 Jun 2021 14:57:56 +0100 Subject: [PATCH 51/52] Update doc block Co-authored-by: Kai Hao <kevin830726@gmail.com> --- .../core-data/src/fetch/__experimental-fetch-remote-url-data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js b/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js index ed2e453e0406eb..c9210c009bc74d 100644 --- a/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js +++ b/packages/core-data/src/fetch/__experimental-fetch-remote-url-data.js @@ -15,7 +15,7 @@ import { addQueryArgs, prependHTTP } from '@wordpress/url'; * eg: <title> tag, favicon...etc. * * @async - * @param {string} url the URL to request details from. + * @param {string} url the URL to request details from. * @param {Object?} options any options to pass to the underlying fetch. * @example * ```js From bf22b24d349a294609156664bb3e2eb11cf76d75 Mon Sep 17 00:00:00 2001 From: Dave Smith <getdavemail@gmail.com> Date: Wed, 9 Jun 2021 16:20:00 +0100 Subject: [PATCH 52/52] Restore makecancellable to a local util --- .../link-control/make-cancelable.js | 28 ---------------- .../link-control/use-create-page.js | 32 ++++++++++++++++--- 2 files changed, 27 insertions(+), 33 deletions(-) delete mode 100644 packages/block-editor/src/components/link-control/make-cancelable.js diff --git a/packages/block-editor/src/components/link-control/make-cancelable.js b/packages/block-editor/src/components/link-control/make-cancelable.js deleted file mode 100644 index 50fa92e969fc1c..00000000000000 --- a/packages/block-editor/src/components/link-control/make-cancelable.js +++ /dev/null @@ -1,28 +0,0 @@ -/** - * Creates a wrapper around a promise which allows it to be programmatically - * cancelled. - * See: https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html - * - * @param {Promise} promise the Promise to make cancelable - */ -const makeCancelable = ( promise ) => { - let hasCanceled_ = false; - - const wrappedPromise = new Promise( ( resolve, reject ) => { - promise.then( - ( val ) => - hasCanceled_ ? reject( { isCanceled: true } ) : resolve( val ), - ( error ) => - hasCanceled_ ? reject( { isCanceled: true } ) : reject( error ) - ); - } ); - - return { - promise: wrappedPromise, - cancel() { - hasCanceled_ = true; - }, - }; -}; - -export default makeCancelable; diff --git a/packages/block-editor/src/components/link-control/use-create-page.js b/packages/block-editor/src/components/link-control/use-create-page.js index 9a1018aa36de5b..42e243a2625fd4 100644 --- a/packages/block-editor/src/components/link-control/use-create-page.js +++ b/packages/block-editor/src/components/link-control/use-create-page.js @@ -4,11 +4,6 @@ import { __ } from '@wordpress/i18n'; import { useEffect, useState, useRef } from '@wordpress/element'; -/** - * Internal dependencies - */ -import makeCancelable from './make-cancelable'; - export default function useCreatePage( handleCreatePage ) { const cancelableCreateSuggestion = useRef(); const [ isCreatingPage, setIsCreatingPage ] = useState( false ); @@ -63,3 +58,30 @@ export default function useCreatePage( handleCreatePage ) { errorMessage, }; } + +/** + * Creates a wrapper around a promise which allows it to be programmatically + * cancelled. + * See: https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html + * + * @param {Promise} promise the Promise to make cancelable + */ +const makeCancelable = ( promise ) => { + let hasCanceled_ = false; + + const wrappedPromise = new Promise( ( resolve, reject ) => { + promise.then( + ( val ) => + hasCanceled_ ? reject( { isCanceled: true } ) : resolve( val ), + ( error ) => + hasCanceled_ ? reject( { isCanceled: true } ) : reject( error ) + ); + } ); + + return { + promise: wrappedPromise, + cancel() { + hasCanceled_ = true; + }, + }; +};