Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nav Block - show recent pages as default suggestions when creating Nav Links #19458

Merged
merged 29 commits into from
Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4d693f3
Adds ability to manual trigger search data fetch
getdave Jan 7, 2020
a124c1b
Update to prop to experimental and rename to initialSuggestions
getdave Jan 7, 2020
a94b1c3
Query for and display 5 most recently modified Pages
getdave Jan 7, 2020
56b8ff8
Update initial suggestions to be display whenever the input is empty …
getdave Jan 8, 2020
a4e205c
Fix to ensure initial suggestions are shown when pages request fulfills
getdave Jan 8, 2020
5384967
Updates to display 3 most recently modified pages instead of 5
getdave Jan 8, 2020
eeacd74
Fix to allow keyboard arrow to move focus into suggestions when present
getdave Jan 8, 2020
2bf0d68
Fix bug whereby not providing initialSuggestions to LinkControl disab…
getdave Jan 9, 2020
3923795
Adds tests to cover displaying initial suggestions
getdave Jan 9, 2020
8b23050
Removes redundant const comment
getdave Jan 9, 2020
7d34ed5
Test fix to e2e tests
getdave Jan 9, 2020
c37497e
Try looser mock request matching to fix e2e test failures
getdave Jan 10, 2020
cf55b73
Try fix Travis e2e test failure by clearing request mocks
getdave Jan 10, 2020
573972f
Fix not awaiting mock setup
getdave Jan 10, 2020
ee6c5b8
Fix inadvertant revert of e2e test fix
getdave Jan 10, 2020
c96409c
Refactor to remove seperate initialSuggestions query
getdave Jan 13, 2020
5eef3dd
Refactor to use empty value as condition for initialSuggestions request
getdave Jan 14, 2020
f500fe4
Fix “suggestions” typo
getdave Jan 14, 2020
0208bc6
Update to reuse existing edit constant
getdave Jan 14, 2020
2244e33
Fix initial suggestions showing when input has value
getdave Jan 14, 2020
a31f651
Fix potential infinite render using flag to conditionalise updating s…
getdave Jan 14, 2020
3b8b6ea
Add test to cover inifinite re-render loop.
getdave Jan 14, 2020
1ca624b
Fixes typo in WordPress ORG link in e2e tests
getdave Jan 14, 2020
b6d6c26
Fix e2e test failure due to console warning in LinkControl
getdave Jan 14, 2020
abe616c
Revise snapshots
getdave Jan 14, 2020
259938a
Restore missing `fetchSearchSuggestions` prop following rebase
getdave Jan 15, 2020
385d04f
Fix lint indentation error
getdave Jan 15, 2020
5dc5716
Rename prop
getdave Jan 15, 2020
8c648a3
Fix test broken due to prop rename
getdave Jan 15, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions packages/block-editor/src/components/link-control/index.js
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 } : {} ),
getdave marked this conversation as resolved.
Show resolved Hide resolved
} ),
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
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 ) {

}
Comment on lines +30 to +36
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were these changes intended to have been included? Looks like dead code to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code. Being removed elsewhere.

};

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
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
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
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