Skip to content

Autocomplete: Skip work for unselected blocks via isEnabled flag #78304

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

Closed
ellatrix wants to merge 1 commit into from
Closed

Autocomplete: Skip work for unselected blocks via isEnabled flag #78304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions packages/block-editor/src/components/autocomplete/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ import blockAutocompleter from '../../autocompleters/block';
*/
const EMPTY_ARRAY = [];

function useCompleters( { completers = EMPTY_ARRAY } ) {
function useCompleters( { completers = EMPTY_ARRAY, isEnabled = true } ) {
const { name } = useBlockEditContext();
return useMemo( () => {
if ( ! isEnabled ) {
return EMPTY_ARRAY;
}
let filteredCompleters = [ ...completers ];

if (
Expand All @@ -51,7 +54,7 @@ function useCompleters( { completers = EMPTY_ARRAY } ) {
}

return filteredCompleters;
}, [ completers, name ] );
}, [ completers, name, isEnabled ] );
}

export function useBlockEditorAutocompleteProps( props ) {
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/rich-text/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ export function RichTextWrapper(
completers: autocompleters,
record: value,
onChange,
isEnabled: isSelected,

@tyxla tyxla May 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there no way to trigger autocomplete programmatically for an unselected block?

} );

useMarkPersistent( { html: adjustedValue, value } );
Expand Down
38 changes: 26 additions & 12 deletions packages/components/src/autocomplete/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export function useAutocomplete( {
onReplace,
completers,
contentRef,
isEnabled = true,
}: UseAutocompleteProps ) {
const instanceId = useInstanceId( AUTOCOMPLETE_HOOK_REFERENCE );
const [ state, dispatch ] = useReducer( autocompleteReducer, initialState );
Expand Down Expand Up @@ -241,13 +242,19 @@ export function useAutocomplete( {
// but this is a preemptive performance improvement, since the autocompleter
// is a potential bottleneck for the editor type metric.
const textContent = useMemo( () => {
if ( ! isEnabled ) {
return '';
}
if ( isCollapsed( record ) ) {
return getTextContent( slice( record, 0 ) );
}
return '';
}, [ record ] );
}, [ record, isEnabled ] );

useEffect( () => {
if ( ! isEnabled ) {
return;
}
Comment on lines 250 to +257

@ciampo ciampo May 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we dispatch RESET and clear lastCompletionRef.current when isEnabled flip from true to false? ie. making the disabled state a true "clean slate"

useEffect( () => {
	if ( ! isEnabled ) {
		if ( autocompleter ) {
			dispatch( { type: 'RESET' } );
		}
		lastCompletionRef.current = null;
		// Keep prevRecordTextRef in sync so the cursor-only guard below
		// works correctly on the next re-enable (no false "text change").
		prevRecordTextRef.current = record.text;
		return;
	}
	const isTextChange = record.text !== prevRecordTextRef.current;
	prevRecordTextRef.current = record.text;
	// ... rest unchanged
}, [ textContent, isEnabled ] );

@Mamaduka Mamaduka May 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the check here isn't needed, the text and completers become empty, and then the effect should clean itself up.

P.S. Sorry, I'm at WordCamp and can't do a proper review at the moment.

@tyxla tyxla May 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with @ciampo that it's worth flushing prevRecordTextRef, lastCompletionRef, and dispatching RESET in the disabling case - otherwise an isEnabled: false to true flip with a record.text change (undo/redo, programmatic edit) could leave stale refs and could fire an unexpected MATCH upon selecting.

const isTextChange = record.text !== prevRecordTextRef.current;
prevRecordTextRef.current = record.text;

Expand Down Expand Up @@ -302,7 +309,7 @@ export function useAutocomplete( {
dispatch( { type: 'MATCH', completer, query } );
// We want to avoid introducing unexpected side effects.
// See https://github.com/WordPress/gutenberg/pull/41820
}, [ textContent ] );
}, [ textContent, isEnabled ] );

const { key: selectedKey = '' } = filteredOptions[ selectedIndex ] || {};
const { className } = autocompleter || {};
Expand Down Expand Up @@ -376,6 +383,7 @@ export function useLastDifferentValue(
}

export function useAutocompleteProps( options: UseAutocompleteProps ) {
const { isEnabled = true } = options;
const ref = useRef< HTMLElement >( null );
const onKeyDownRef =
useRef< ( event: KeyboardEvent ) => void >( undefined );
Expand All @@ -389,19 +397,25 @@ export function useAutocompleteProps( options: UseAutocompleteProps ) {

const mergedRefs = useMergeRefs( [
ref,
useRefEffect( ( element: HTMLElement ) => {
function _onKeyDown( event: KeyboardEvent ) {
onKeyDownRef.current?.( event );
}
element.addEventListener( 'keydown', _onKeyDown );
return () => {
element.removeEventListener( 'keydown', _onKeyDown );
};
}, [] ),
useRefEffect(
( element: HTMLElement ) => {
if ( ! isEnabled ) {
return;
}
function _onKeyDown( event: KeyboardEvent ) {
onKeyDownRef.current?.( event );
}
element.addEventListener( 'keydown', _onKeyDown );
return () => {
element.removeEventListener( 'keydown', _onKeyDown );
};
},
[ isEnabled ]
),
] );

// We only want to show the popover if the user has typed something.
const didUserInput = record.text !== previousRecord?.text;
const didUserInput = isEnabled && record.text !== previousRecord?.text;

if ( ! didUserInput ) {
return { ref: mergedRefs };
Expand Down
7 changes: 7 additions & 0 deletions packages/components/src/autocomplete/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,13 @@ export type UseAutocompleteProps = {
* `Autocomplete`'s `Popover`.
*/
contentRef: ContentRef;
/**
* Whether the autocompleter should be active. Defaults to `true`. When
* `false`, the heavy bits — match scanning, keydown listener attachment,
* popover state — are skipped while the hook still returns a stable ref so
* the host can attach it unconditionally.
*/
isEnabled?: boolean;
};

export type AutocompleteState = {
Expand Down
Loading