Skip to content

Commit

Permalink
Nav Block - show recent pages as default suggestions when creating Na…
Browse files Browse the repository at this point in the history 
…v Links (#19458)

* Adds ability to manual trigger search data fetch

* Update to prop to experimental and rename to initialSuggestions

* Query for and display 5 most recently modified Pages

* Update initial suggestions to be display whenever the input is empty and there are no current suggestions

This is needed to ensure that the async fulfillment of the initialSuggestions prop causes the component to re-render and display the initial suggestions. Without this the initial suggestions would only be available if the prop is fufilled on the initial component mount.

* Fix to ensure initial suggestions are shown when pages request fulfills

Addresses issues noted in #19458 (comment)

* Updates to display 3 most recently modified pages instead of 5

Addresses #19458 (comment)

* Fix to allow keyboard arrow to move focus into suggestions when present

Previously it was not possible to use the down arrow key to move the focus into the search suggestions when initialSuggestions were displayed. This was due to requirement for there to be a value in the `input` which is now no longer valid now that initial suggestions can be displayed.

Note that removing the derived setting of selectedSuggestion state is valid as selectedSuggestions are always reset on each new data fetch anyway.

Resolves issue noted in #19458 (comment)

* Fix bug whereby not providing initialSuggestions to LinkControl disabled the ability to search at all

Test failures led to uncovering of a bug whereby the URLInput component’s `updateSuggestions` method would exit early it initialSuggestions wasn’t provided. As initialSuggestions is an optional prop this means that all fetching of search suggestions was disabled if the initialSuggestions wasn’t provided.

Fixed this bug and all tests by improving the conditionals.

* Adds tests to cover displaying initial suggestions

* Removes redundant const comment

* Test fix to e2e tests

* Try looser mock request matching to fix e2e test failures

* Try fix Travis e2e test failure by clearing request mocks

* Fix not awaiting mock setup

* Fix inadvertant revert of e2e test fix

* Refactor to remove seperate initialSuggestions query

As per the GH thread below, this refactors the code to avoid the need to introduce a new fetch API around `__experimentalInitialSuggestions`. Now instead we simply reuse the existing fetchSuggestions handler to get the initial results. The only different being we introduce an arguments object to queries to restrict the number of results displayed for initial Suggestions.

See #19458 (comment)

* Refactor to use empty value as condition for initialSuggestions request

Addresses #19458 (comment)

* Fix “suggestions” typo

* Update to reuse existing edit constant

* Fix initial suggestions showing when input has value

* Fix potential infinite render using flag to conditionalise updating suggestions on update or mount

* Add test to cover inifinite re-render loop.

* Fixes typo in WordPress ORG link in e2e tests

* Fix e2e test failure due to console warning in LinkControl

This is a temp fix and I’ve raised an Issue to solve the core issue which will then make this fix redundant.

#19634

* Revise snapshots

* Restore missing `fetchSearchSuggestions` prop following rebase

* Fix lint indentation error

* Rename prop

* Fix test broken due to prop rename
  • Loading branch information
@getdave
getdave authored Jan 15, 2020
1 parent a7db562 commit b448074
Show file tree
Hide file tree
Showing 10 changed files with 185 additions and 28 deletions.
17 changes: 10 additions & 7 deletions packages/block-editor/src/components/link-control/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ import LinkControlSearchItem from './search-item';
import LinkControlSearchInput from './search-input';

const MODE_EDIT = 'edit';
// const MODE_SHOW = 'show';

export function LinkControl( {
value,
settings,
onChange = noop,
showInitialSuggestions,
fetchSearchSuggestions,
} ) {
const instanceId = useInstanceId( LinkControl );
Expand Down Expand Up @@ -66,7 +66,7 @@ export function LinkControl( {

// Populate input searcher whether
// the current link has a title.
if ( value && value.title && mode === 'edit' ) {
if ( value && value.title && MODE_EDIT === mode ) {
setInputValue( value.title );
}
};
Expand Down Expand Up @@ -102,9 +102,11 @@ export function LinkControl( {
);
};

const handleEntitySearch = async ( val ) => {
const handleEntitySearch = async ( val, args ) => {
const results = await Promise.all( [
fetchSearchSuggestions( val ),
fetchSearchSuggestions( val, {
...( args.isInitialSuggestions ? { perPage: 3 } : {} ),
} ),
handleDirectEntry( val ),
] );

Expand All @@ -113,19 +115,19 @@ export function LinkControl( {
// If it's potentially a URL search then concat on a URL search suggestion
// just for good measure. That way once the actual results run out we always
// have a URL option to fallback on.
return couldBeURL ? results[ 0 ].concat( results[ 1 ] ) : results[ 0 ];
return couldBeURL && ! args.isInitialSuggestions ? results[ 0 ].concat( results[ 1 ] ) : results[ 0 ];
};

// Effects
const getSearchHandler = useCallback( ( val ) => {
const getSearchHandler = useCallback( ( val, args ) => {
const protocol = getProtocol( val ) || '';
const isMailto = protocol.includes( 'mailto' );
const isInternal = startsWith( val, '#' );
const isTel = protocol.includes( 'tel' );

const handleManualEntry = isInternal || isMailto || isTel || isURL( val ) || ( val && val.includes( 'www.' ) );

return ( handleManualEntry ) ? handleDirectEntry( val ) : handleEntitySearch( val );
return ( handleManualEntry ) ? handleDirectEntry( val, args ) : handleEntitySearch( val, args );
}, [ handleDirectEntry, fetchSearchSuggestions ] );

// Render Components
Expand Down Expand Up @@ -200,6 +202,7 @@ export function LinkControl( {
renderSuggestions={ renderSearchResults }
fetchSuggestions={ getSearchHandler }
onReset={ resetInput }
showInitialSuggestions={ showInitialSuggestions }
/>
) }

Expand Down
8 changes: 8 additions & 0 deletions packages/block-editor/src/components/link-control/search-input.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ const handleLinkControlOnKeyDown = ( event ) => {
};

const handleLinkControlOnKeyPress = ( event ) => {
const { keyCode } = event;

event.stopPropagation();

if ( keyCode === ENTER ) {

}
};

const LinkControlSearchInput = ( {
Expand All @@ -37,6 +43,7 @@ const LinkControlSearchInput = ( {
renderSuggestions,
fetchSuggestions,
onReset,
showInitialSuggestions,
} ) => {
const selectItemHandler = ( selection, suggestion ) => {
onChange( selection );
Expand Down Expand Up @@ -68,6 +75,7 @@ const LinkControlSearchInput = ( {
__experimentalRenderSuggestions={ renderSuggestions }
__experimentalFetchLinkSuggestions={ fetchSuggestions }
__experimentalHandleURLSuggestions={ true }
__experimentalShowInitialSuggestions={ showInitialSuggestions }
/>

<Button
Expand Down
13 changes: 8 additions & 5 deletions packages/block-editor/src/components/link-control/test/fixtures/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { uniqueId } from 'lodash';
import { uniqueId, take } from 'lodash';

export const fauxEntitySuggestions = [
{
Expand Down Expand Up @@ -34,8 +34,11 @@ export const fauxEntitySuggestions = [
},
];

// export const fetchFauxEntitySuggestions = async () => fauxEntitySuggestions;

export const fetchFauxEntitySuggestions = () => {
return Promise.resolve( fauxEntitySuggestions );
/* eslint-disable no-unused-vars */
export const fetchFauxEntitySuggestions = ( val = '', {
perPage = null,
} = {} ) => {
const suggestions = perPage ? take( fauxEntitySuggestions, perPage ) : fauxEntitySuggestions;
return Promise.resolve( suggestions );
};
/* eslint-enable no-unused-vars */
99 changes: 97 additions & 2 deletions packages/block-editor/src/components/link-control/test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,103 @@ describe( 'Manual link entry', () => {
} );
} );

describe( 'Default search suggestions', () => {
it( 'should display a list of initial search suggestions when there is no search value or suggestions', async () => {
const searchSuggestionsSpy = jest.fn( fetchFauxEntitySuggestions );
const expectedResultsLength = 3; // set within `LinkControl`

act( () => {
render(
<LinkControl
fetchSearchSuggestions={ searchSuggestionsSpy }
showInitialSuggestions={ true }
/>, container
);
} );

await eventLoopTick();

// Search Input UI
const searchInput = container.querySelector( 'input[aria-label="URL"]' );

// TODO: select these by aria relationship to autocomplete rather than arbitary selector.
const initialSearchResultElements = container.querySelectorAll( '[role="listbox"] [role="option"]' );

// Verify input has no value has default suggestions should only show
// when this does not have a value
expect( searchInput.value ).toBe( '' );

// Ensure only called once as a guard against potential infinite
// re-render loop within `componentDidUpdate` calling `updateSuggestions`
// which has calls to `setState` within it.
expect( searchSuggestionsSpy ).toHaveBeenCalledTimes( 1 );

// Verify the search results already display the initial suggestions
expect( initialSearchResultElements ).toHaveLength( expectedResultsLength );
} );

it( 'should not display initial suggestions when input value is present', async () => {
const searchSuggestionsSpy = jest.fn( fetchFauxEntitySuggestions );
let searchResultElements;
//
// Render with an initial value an ensure that no initial suggestions
// are shown.
//
act( () => {
render(
<LinkControl
fetchSearchSuggestions={ searchSuggestionsSpy }
showInitialSuggestions={ true }
value={ fauxEntitySuggestions[ 0 ] }
/>, container
);
} );

await eventLoopTick();

expect( searchSuggestionsSpy ).not.toHaveBeenCalled();

//
// Click the "Edit/Change" button and check initial suggestions are not
// shown.
//
const currentLinkUI = container.querySelector( '.block-editor-link-control__search-item.is-current' );
const currentLinkBtn = currentLinkUI.querySelector( 'button' );

act( () => {
Simulate.click( currentLinkBtn );
} );

await eventLoopTick();

searchResultElements = container.querySelectorAll( '[role="listbox"] [role="option"]' );

expect( searchResultElements ).toHaveLength( 0 );

expect( searchSuggestionsSpy ).not.toHaveBeenCalled();

//
// Reset the search to empty and check the initial suggestions are now shown.
//
const resetUI = container.querySelector( '[aria-label="Reset"]' );

act( () => {
Simulate.click( resetUI );
} );

await eventLoopTick();

searchResultElements = container.querySelectorAll( '[role="listbox"] [role="option"]' );

expect( searchResultElements ).toHaveLength( 3 );

// Ensure only called once as a guard against potential infinite
// re-render loop within `componentDidUpdate` calling `updateSuggestions`
// which has calls to `setState` within it.
expect( searchSuggestionsSpy ).toHaveBeenCalledTimes( 1 );
} );
} );

describe( 'Selecting links', () => {
it( 'should display a selected link corresponding to the provided "currentLink" prop', () => {
const selectedLink = first( fauxEntitySuggestions );
Expand Down Expand Up @@ -556,8 +653,6 @@ describe( 'Addition Settings UI', () => {
);
} );

// console.log( container.innerHTML );

const newTabSettingLabel = Array.from( container.querySelectorAll( 'label' ) ).find( ( label ) => label.innerHTML && label.innerHTML.includes( expectedSettingText ) );
expect( newTabSettingLabel ).not.toBeUndefined(); // find() returns "undefined" if not found

Expand Down
52 changes: 42 additions & 10 deletions packages/block-editor/src/components/url-input/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class URLInput extends Component {

this.suggestionNodes = [];

this.isUpdatingSuggestions = false;

this.state = {
suggestions: [],
showSuggestions: false,
Expand All @@ -45,6 +47,7 @@ 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 ) {
Expand All @@ -58,6 +61,16 @@ class URLInput extends Component {
this.scrollingIntoView = false;
}, 100 );
}

if ( this.shouldShowInitialSuggestions() ) {
this.updateSuggestions();
}
}

componentDidMount() {
if ( this.shouldShowInitialSuggestions() ) {
this.updateSuggestions();
}
}

componentWillUnmount() {
Expand All @@ -70,18 +83,29 @@ class URLInput extends Component {
};
}

updateSuggestions( value ) {
shouldShowInitialSuggestions() {
const { suggestions } = this.state;
const { __experimentalShowInitialSuggestions = false, value } = this.props;
return ! this.isUpdatingSuggestions && __experimentalShowInitialSuggestions && ! ( value && value.length ) && ! ( suggestions && suggestions.length );
}

updateSuggestions( value = '' ) {
const {
__experimentalFetchLinkSuggestions: fetchLinkSuggestions,
__experimentalHandleURLSuggestions: handleURLSuggestions,
} = this.props;

if ( ! fetchLinkSuggestions ) {
return;
}

// Show the suggestions after typing at least 2 characters
// and also for URLs
if ( value.length < 2 || ( ! handleURLSuggestions && isURL( value ) ) ) {
const isInitialSuggestions = ! ( value && value.length );

// Allow a suggestions request if:
// - there are at least 2 characters in the search input (except manual searches where
// search input length is not required to trigger a fetch)
// - this is a direct entry (eg: a URL)
if ( ! isInitialSuggestions && ( value.length < 2 || ( ! handleURLSuggestions && isURL( value ) ) ) ) {
this.setState( {
showSuggestions: false,
selectedSuggestion: null,
Expand All @@ -91,13 +115,17 @@ class URLInput extends Component {
return;
}

this.isUpdatingSuggestions = true;

this.setState( {
showSuggestions: true,
selectedSuggestion: null,
loading: true,
} );

const request = fetchLinkSuggestions( value );
const request = fetchLinkSuggestions( value, {
isInitialSuggestions,
} );

request.then( ( suggestions ) => {
// A fetch Promise doesn't have an abort option. It's mimicked by
Expand All @@ -121,19 +149,24 @@ class URLInput extends Component {
} else {
this.props.debouncedSpeak( __( 'No results.' ), 'assertive' );
}
this.isUpdatingSuggestions = false;
} ).catch( () => {
if ( this.suggestionsRequest === request ) {
this.setState( {
loading: false,
} );
this.isUpdatingSuggestions = false;
}
} );

// Note that this assignment is handled *before* the async search request
// as a Promise always resolves on the next tick of the event loop.
this.suggestionsRequest = request;
}

onChange( event ) {
const inputValue = event.target.value;

this.props.onChange( inputValue );
if ( ! this.props.disableSuggestions ) {
this.updateSuggestions( inputValue );
Expand All @@ -146,8 +179,7 @@ class URLInput extends Component {
// If the suggestions are not shown or loading, we shouldn't handle the arrow keys
// We shouldn't preventDefault to allow block arrow keys navigation
if (
( ! showSuggestions || ! suggestions.length || loading ) &&
this.props.value
( ! showSuggestions || ! suggestions.length || loading )
) {
// In the Windows version of Firefox the up and down arrows don't move the caret
// within an input field like they do for Mac Firefox/Chrome/Safari. This causes
Expand Down Expand Up @@ -237,12 +269,12 @@ class URLInput extends Component {
this.inputRef.current.focus();
}

static getDerivedStateFromProps( { value, disableSuggestions }, { showSuggestions, selectedSuggestion } ) {
static getDerivedStateFromProps( { value, disableSuggestions, __experimentalShowInitialSuggestions = false }, { showSuggestions } ) {
let shouldShowSuggestions = showSuggestions;

const hasValue = value && value.length;

if ( ! hasValue ) {
if ( ! __experimentalShowInitialSuggestions && ! hasValue ) {
shouldShowSuggestions = false;
}

Expand All @@ -251,7 +283,6 @@ class URLInput extends Component {
}

return {
selectedSuggestion: hasValue ? selectedSuggestion : null,
showSuggestions: shouldShowSuggestions,
};
}
Expand All @@ -275,6 +306,7 @@ class URLInput extends Component {
selectedSuggestion,
loading,
} = this.state;

const id = `url-input-control-${ instanceId }`;

const suggestionsListboxId = `block-editor-url-input-suggestions-${ instanceId }`;
Expand Down
Loading

0 comments on commit b448074

Please sign in to comment.