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.

Sign up for GitHub

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

URLInput: keep the search results label in sync with the results list #45806

Merged
merged 9 commits into from
Nov 28, 2022
Merged

URLInput: keep the search results label in sync with the results list #45806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0cea868
URLInput: reorganize the renderSuggestion condition
jsnajdr Nov 16, 2022
17ef92d
URLInput: simplify shouldShowInitialSuggestions which always runs wit…
jsnajdr Nov 16, 2022
a7b13b2
URLInput: don't updateSuggestions on change, it's done by cDU lifecycle
jsnajdr Nov 16, 2022
4401486
URLInput: cancel suggestionsRequest when updated value means no request
jsnajdr Nov 16, 2022
24dd6bb
URLInput: set empty suggestions when updated value means no request
jsnajdr Nov 16, 2022
fb63445
URLInput: move isUpdatingSuggestions to state
jsnajdr Nov 16, 2022
3ff9d20
URLInput: put the current input value to state, in sync with search r…
jsnajdr Nov 16, 2022
3656e40
LinkControl: don't send currentInputValue to URLInput, it already doe…
jsnajdr Nov 16, 2022
4459a77
URLInput: mention the currentInputValue param for renderSuggestions i…
jsnajdr Nov 28, 2022
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
4 changes: 4 additions & 0 deletions packages/block-editor/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Enhancement

- `URLInput`: the `renderSuggestions` callback prop now receives `currentInputValue` as a new parameter ([45806](https://github.com/WordPress/gutenberg/pull/45806)).

## 10.5.0 (2022-11-16)

### Enhancement
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/link-control/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,10 +322,10 @@ The following properties are provided by URLInput:
- suggestions
- selectedSuggestion
- suggestionsListProps
- currentInputValue

The following extra properties are provided by LinkControlSearchInput:

- currentInputValue
- createSuggestionButtonText
- handleSuggestionClick
- instanceId
Expand Down
1 change: 0 additions & 1 deletion packages/block-editor/src/components/link-control/search-input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ const LinkControlSearchInput = forwardRef(
...props,
instanceId,
withCreateSuggestion,
currentInputValue: value,
Copy link
Member

Choose a reason for hiding this comment

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

Sounds like something that needs to be mentioned in the CHANGELOG?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I now added info to block-editor about the URLInput.renderSuggestions having a new currentInputValue parameter.

createSuggestionButtonText,
suggestionsQuery,
handleSuggestionClick: ( suggestion ) => {
Expand Down
130 changes: 57 additions & 73 deletions packages/block-editor/src/components/url-input/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,14 @@ class URLInput extends Component {

this.suggestionNodes = [];

this.isUpdatingSuggestions = false;
this.suggestionsRequest = null;
Copy link
Member

Choose a reason for hiding this comment

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

These component instance properties are always a recipe for weird bugs since React doesn't usually observe changes in them. Maybe we should ideally get rid of them all?

Copy link
Member Author

Choose a reason for hiding this comment

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

this.suggestionNodes is used to store refs, it can't be in state and needs to stay ref-like.

this.suggestionsRequest is used to prevent state updates from a request if another request has been issued in the meantime. It can't be in state because we need to set it to null on unmount, so that responses arriving after unmount are ignored.


this.state = {
suggestions: [],
showSuggestions: false,
isUpdatingSuggestions: false,
suggestionsValue: null,
selectedSuggestion: null,

suggestionsListboxId: '',
suggestionOptionIdPrefix: '',
};
Expand Down Expand Up @@ -103,7 +104,7 @@ class URLInput extends Component {
if (
prevProps.value !== value &&
! this.props.disableSuggestions &&
! this.isUpdatingSuggestions
! this.state.isUpdatingSuggestions
) {
if ( value?.length ) {
// If the new value is not empty we need to update with suggestions for it.
Expand All @@ -123,7 +124,7 @@ class URLInput extends Component {

componentWillUnmount() {
this.suggestionsRequest?.cancel?.();
delete this.suggestionsRequest;
this.suggestionsRequest = null;
}

bindSuggestionNode( index ) {
Expand All @@ -133,14 +134,10 @@ class URLInput extends Component {
}

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

Expand Down Expand Up @@ -170,18 +167,22 @@ class URLInput extends Component {
! isInitialSuggestions &&
( value.length < 2 || ( ! handleURLSuggestions && isURL( value ) ) )
) {
this.suggestionsRequest?.cancel?.();
this.suggestionsRequest = null;

this.setState( {
suggestions: [],
showSuggestions: false,
suggestionsValue: value,
selectedSuggestion: null,
loading: false,
} );

return;
}

this.isUpdatingSuggestions = true;

this.setState( {
isUpdatingSuggestions: true,
selectedSuggestion: null,
loading: true,
} );
Expand All @@ -201,6 +202,8 @@ class URLInput extends Component {

this.setState( {
suggestions,
isUpdatingSuggestions: false,
suggestionsValue: value,
loading: false,
showSuggestions: !! suggestions.length,
} );
Expand All @@ -224,15 +227,16 @@ class URLInput extends Component {
'assertive'
);
}
this.isUpdatingSuggestions = false;
} )
.catch( () => {
if ( this.suggestionsRequest === request ) {
this.setState( {
loading: false,
} );
this.isUpdatingSuggestions = false;
if ( this.suggestionsRequest !== request ) {
return;
}

this.setState( {
isUpdatingSuggestions: false,
loading: false,
} );
} );

// Note that this assignment is handled *before* the async search request
Expand All @@ -241,12 +245,7 @@ class URLInput extends Component {
}

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

this.props.onChange( inputValue );
if ( ! this.props.disableSuggestions ) {
this.updateSuggestions( inputValue );
}
this.props.onChange( event.target.value );
}

onFocus() {
Expand All @@ -258,7 +257,7 @@ class URLInput extends Component {
if (
value &&
! disableSuggestions &&
! this.isUpdatingSuggestions &&
! this.state.isUpdatingSuggestions &&
! ( suggestions && suggestions.length )
) {
// Ensure the suggestions are updated with the current input value.
Expand Down Expand Up @@ -490,19 +489,22 @@ class URLInput extends Component {
const {
className,
__experimentalRenderSuggestions: renderSuggestions,
value = '',
__experimentalShowInitialSuggestions = false,
} = this.props;

const {
showSuggestions,
suggestions,
suggestionsValue,
selectedSuggestion,
suggestionsListboxId,
suggestionOptionIdPrefix,
loading,
} = this.state;

if ( ! showSuggestions || suggestions.length === 0 ) {
return null;
}

const suggestionsListProps = {
id: suggestionsListboxId,
ref: this.autocompleteRef,
Expand All @@ -519,64 +521,46 @@ class URLInput extends Component {
};
};

if (
isFunction( renderSuggestions ) &&
showSuggestions &&
!! suggestions.length
) {
if ( isFunction( renderSuggestions ) ) {
return renderSuggestions( {
suggestions,
selectedSuggestion,
suggestionsListProps,
buildSuggestionItemProps,
isLoading: loading,
handleSuggestionClick: this.handleOnClick,
isInitialSuggestions:
__experimentalShowInitialSuggestions &&
! ( value && value.length ),
isInitialSuggestions: ! suggestionsValue?.length,
currentInputValue: suggestionsValue,
} );
}

if (
! isFunction( renderSuggestions ) &&
showSuggestions &&
!! suggestions.length
) {
return (
<Popover placement="bottom" focusOnMount={ false }>
<div
{ ...suggestionsListProps }
className={ classnames(
'block-editor-url-input__suggestions',
`${ className }__suggestions`
) }
>
{ suggestions.map( ( suggestion, index ) => (
<Button
{ ...buildSuggestionItemProps(
suggestion,
index
) }
key={ suggestion.id }
className={ classnames(
'block-editor-url-input__suggestion',
{
'is-selected':
index === selectedSuggestion,
}
) }
onClick={ () =>
this.handleOnClick( suggestion )
return (
<Popover placement="bottom" focusOnMount={ false }>
<div
{ ...suggestionsListProps }
className={ classnames(
'block-editor-url-input__suggestions',
`${ className }__suggestions`
) }
>
{ suggestions.map( ( suggestion, index ) => (
<Button
{ ...buildSuggestionItemProps( suggestion, index ) }
key={ suggestion.id }
className={ classnames(
'block-editor-url-input__suggestion',
{
'is-selected': index === selectedSuggestion,
}
>
{ suggestion.title }
</Button>
) ) }
</div>
</Popover>
);
}
return null;
) }
onClick={ () => this.handleOnClick( suggestion ) }
>
{ suggestion.title }
</Button>
) ) }
</div>
</Popover>
);
}
}

Expand Down