From ecd3d313068d803a3d947135738b0314e40661b6 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 19 Jul 2018 09:20:30 -0400 Subject: [PATCH 1/3] Components: Mimic URLInput request abort on fetch --- editor/components/url-input/index.js | 113 ++++++++++++++------------- 1 file changed, 59 insertions(+), 54 deletions(-) diff --git a/editor/components/url-input/index.js b/editor/components/url-input/index.js index 2de470aa436e6..b1595816ce12c 100644 --- a/editor/components/url-input/index.js +++ b/editor/components/url-input/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { throttle } from 'lodash'; +import { includes, throttle } from 'lodash'; import classnames from 'classnames'; import scrollIntoView from 'dom-scroll-into-view'; import { stringify } from 'querystringify'; @@ -25,11 +25,15 @@ const stopEventPropagation = ( event ) => event.stopPropagation(); class UrlInput extends Component { constructor() { super( ...arguments ); + this.onChange = this.onChange.bind( this ); this.onKeyDown = this.onKeyDown.bind( this ); this.bindListNode = this.bindListNode.bind( this ); this.updateSuggestions = throttle( this.updateSuggestions.bind( this ), 200 ); + this.suggestionNodes = []; + this.suggestionsRequests = []; + this.state = { posts: [], showSuggestions: false, @@ -37,6 +41,26 @@ class UrlInput extends Component { }; } + componentDidUpdate() { + const { showSuggestions, selectedSuggestion } = this.state; + // only have to worry about scrolling selected suggestion into view + // when already expanded + if ( showSuggestions && selectedSuggestion !== null && ! this.scrollingIntoView ) { + this.scrollingIntoView = true; + scrollIntoView( this.suggestionNodes[ selectedSuggestion ], this.listNode, { + onlyScrollIfNeeded: true, + } ); + + setTimeout( () => { + this.scrollingIntoView = false; + }, 100 ); + } + } + + componentWillUnmount() { + this.suggestionsRequests = []; + } + bindListNode( ref ) { this.listNode = ref; } @@ -48,9 +72,7 @@ class UrlInput extends Component { } updateSuggestions( value ) { - if ( this.suggestionsRequest ) { - this.suggestionsRequest.abort(); - } + this.suggestionsRequests = []; // Show the suggestions after typing at least 2 characters // and also for URLs @@ -69,7 +91,8 @@ class UrlInput extends Component { selectedSuggestion: null, loading: true, } ); - this.suggestionsRequest = apiFetch( { + + const request = apiFetch( { path: `/wp/v2/posts?${ stringify( { search: value, per_page: 20, @@ -77,33 +100,37 @@ class UrlInput extends Component { } ) }`, } ); - this.suggestionsRequest - .then( - ( posts ) => { - this.setState( { - posts, - loading: false, - } ); - - if ( !! posts.length ) { - this.props.debouncedSpeak( sprintf( _n( - '%d result found, use up and down arrow keys to navigate.', - '%d results found, use up and down arrow keys to navigate.', - posts.length - ), posts.length ), 'assertive' ); - } else { - this.props.debouncedSpeak( __( 'No results.' ), 'assertive' ); - } - }, - ( xhr ) => { - if ( xhr.statusText === 'abort' ) { - return; - } - this.setState( { - loading: false, - } ); - } - ); + request.then( ( posts ) => { + // A fetch Promise doesn't have an abort option. It's mimicked by + // detecting the request reference in an array which is reset on + // subsequent requests or unmounting. + if ( ! includes( this.suggestionsRequests, request ) ) { + return; + } + + this.setState( { + posts, + loading: false, + } ); + + if ( !! posts.length ) { + this.props.debouncedSpeak( sprintf( _n( + '%d result found, use up and down arrow keys to navigate.', + '%d results found, use up and down arrow keys to navigate.', + posts.length + ), posts.length ), 'assertive' ); + } else { + this.props.debouncedSpeak( __( 'No results.' ), 'assertive' ); + } + } ).catch( () => { + if ( includes( this.suggestionsRequests, request ) ) { + this.setState( { + loading: false, + } ); + } + } ); + + this.suggestionsRequests.push( request ); } onChange( event ) { @@ -157,28 +184,6 @@ class UrlInput extends Component { } ); } - componentWillUnmount() { - if ( this.suggestionsRequest ) { - this.suggestionsRequest.abort(); - } - } - - componentDidUpdate() { - const { showSuggestions, selectedSuggestion } = this.state; - // only have to worry about scrolling selected suggestion into view - // when already expanded - if ( showSuggestions && selectedSuggestion !== null && ! this.scrollingIntoView ) { - this.scrollingIntoView = true; - scrollIntoView( this.suggestionNodes[ selectedSuggestion ], this.listNode, { - onlyScrollIfNeeded: true, - } ); - - setTimeout( () => { - this.scrollingIntoView = false; - }, 100 ); - } - } - render() { const { value = '', autoFocus = true, instanceId } = this.props; const { showSuggestions, posts, selectedSuggestion, loading } = this.state; From d53650ab107e4a582bbb542d5f0a6acec210e62b Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 19 Jul 2018 09:49:55 -0400 Subject: [PATCH 2/3] Components: Compare URLInput request by reference --- editor/components/url-input/index.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/editor/components/url-input/index.js b/editor/components/url-input/index.js index b1595816ce12c..1693a204aa54f 100644 --- a/editor/components/url-input/index.js +++ b/editor/components/url-input/index.js @@ -32,7 +32,6 @@ class UrlInput extends Component { this.updateSuggestions = throttle( this.updateSuggestions.bind( this ), 200 ); this.suggestionNodes = []; - this.suggestionsRequests = []; this.state = { posts: [], @@ -58,7 +57,7 @@ class UrlInput extends Component { } componentWillUnmount() { - this.suggestionsRequests = []; + delete this.suggestionsRequest; } bindListNode( ref ) { @@ -72,8 +71,6 @@ class UrlInput extends Component { } updateSuggestions( value ) { - this.suggestionsRequests = []; - // Show the suggestions after typing at least 2 characters // and also for URLs if ( value.length < 2 || /^https?:/.test( value ) ) { @@ -102,9 +99,9 @@ class UrlInput extends Component { request.then( ( posts ) => { // A fetch Promise doesn't have an abort option. It's mimicked by - // detecting the request reference in an array which is reset on - // subsequent requests or unmounting. - if ( ! includes( this.suggestionsRequests, request ) ) { + // comparing the request reference in on the instance, which is + // reset or deleted on subsequent requests or unmounting. + if ( this.suggestionsRequest !== request ) { return; } @@ -123,14 +120,14 @@ class UrlInput extends Component { this.props.debouncedSpeak( __( 'No results.' ), 'assertive' ); } } ).catch( () => { - if ( includes( this.suggestionsRequests, request ) ) { + if ( this.suggestionsRequest === request ) { this.setState( { loading: false, } ); } } ); - this.suggestionsRequests.push( request ); + this.suggestionsRequest = request; } onChange( event ) { From 67b062d5c76d787013fef45e7eaf4120b4c89197 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 19 Jul 2018 11:08:58 -0400 Subject: [PATCH 3/3] Components: Remove unused includes import --- editor/components/url-input/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/components/url-input/index.js b/editor/components/url-input/index.js index 1693a204aa54f..a2e49a8877f1f 100644 --- a/editor/components/url-input/index.js +++ b/editor/components/url-input/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { includes, throttle } from 'lodash'; +import { throttle } from 'lodash'; import classnames from 'classnames'; import scrollIntoView from 'dom-scroll-into-view'; import { stringify } from 'querystringify';