-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Declare and override autocompleters via filter #4609
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
Autocomplete | ||
============ | ||
|
||
This is an Autocomplete component for use in block UI. It is based on `Autocomplete` from `@wordpress/components` and takes the same props. In addition, it passes its autocompleters through a `blocks.Autocomplete.completers` filter to give developers an opportunity to override or extend them. | ||
|
||
The autocompleter interface is documented with the original `Autocomplete` component in `@wordpress/components`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { clone } from 'lodash'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { applyFilters, hasFilter } from '@wordpress/hooks'; | ||
import { Component } from '@wordpress/element'; | ||
import { Autocomplete as OriginalAutocomplete } from '@wordpress/components'; | ||
|
||
/* | ||
* Use one array instance for fallback rather than inline array literals | ||
* because the latter may cause rerender due to failed prop equality checks. | ||
*/ | ||
const completersFallback = []; | ||
|
||
/** | ||
* Wrap the default Autocomplete component with one that | ||
* supports a filter hook for customizing its list of autocompleters. | ||
* | ||
* Since there may be many Autocomplete instances at one time, this component | ||
* applies the filter on demand, when the component is first focused after | ||
* receiving a new list of completers. | ||
* | ||
* This function is exported for unit test. | ||
* | ||
* @param {Function} Autocomplete Original component. | ||
* @return {Function} Wrapped component | ||
*/ | ||
export function withFilteredAutocompleters( Autocomplete ) { | ||
return class FilteredAutocomplete extends Component { | ||
constructor() { | ||
super(); | ||
|
||
this.state = { completers: completersFallback }; | ||
|
||
this.saveParentRef = this.saveParentRef.bind( this ); | ||
this.onFocus = this.onFocus.bind( this ); | ||
} | ||
|
||
componentDidUpdate() { | ||
const hasFocus = this.parentNode.contains( document.activeElement ); | ||
|
||
/* | ||
* It's possible for props to be updated when the component has focus, | ||
* so here, we ensure new completers are immediately applied while we | ||
* have the focus. | ||
* | ||
* NOTE: This may trigger another render but only when the component has focus. | ||
*/ | ||
if ( hasFocus && this.hasStaleCompleters() ) { | ||
this.updateCompletersState(); | ||
} | ||
} | ||
|
||
onFocus() { | ||
if ( this.hasStaleCompleters() ) { | ||
this.updateCompletersState(); | ||
} | ||
} | ||
|
||
hasStaleCompleters() { | ||
return ( | ||
! ( 'lastFilteredCompletersProp' in this.state ) || | ||
this.state.lastFilteredCompletersProp !== this.props.completers | ||
); | ||
} | ||
|
||
updateCompletersState() { | ||
let { completers: nextCompleters } = this.props; | ||
const lastFilteredCompletersProp = nextCompleters; | ||
|
||
if ( hasFilter( 'blocks.Autocomplete.completers' ) ) { | ||
nextCompleters = applyFilters( | ||
'blocks.Autocomplete.completers', | ||
// Provide copies so filters may directly modify them. | ||
nextCompleters && nextCompleters.map( clone ) | ||
); | ||
} | ||
|
||
this.setState( { | ||
lastFilteredCompletersProp, | ||
completers: nextCompleters || completersFallback, | ||
} ); | ||
} | ||
|
||
saveParentRef( parentNode ) { | ||
this.parentNode = parentNode; | ||
} | ||
|
||
render() { | ||
const { completers } = this.state; | ||
const autocompleteProps = { | ||
...this.props, | ||
completers, | ||
}; | ||
|
||
return ( | ||
<div onFocus={ this.onFocus } ref={ this.saveParentRef }> | ||
<Autocomplete onFocus={ this.onFocus } { ...autocompleteProps } /> | ||
</div> | ||
); | ||
} | ||
}; | ||
} | ||
|
||
export default withFilteredAutocompleters( OriginalAutocomplete ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { mount, shallow } from 'enzyme'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { addFilter, removeFilter } from '@wordpress/hooks'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { withFilteredAutocompleters } from '..'; | ||
|
||
function TestComponent() { | ||
// Use a naturally focusable element because we will test with focus. | ||
return <input />; | ||
} | ||
const FilteredComponent = withFilteredAutocompleters( TestComponent ); | ||
|
||
describe( 'Autocomplete', () => { | ||
let wrapper = null; | ||
|
||
afterEach( () => { | ||
removeFilter( 'blocks.Autocomplete.completers', 'test/autocompleters-hook' ); | ||
|
||
if ( wrapper ) { | ||
wrapper.unmount(); | ||
wrapper = null; | ||
} | ||
} ); | ||
|
||
it( 'filters supplied completers when next focused', () => { | ||
const completersFilter = jest.fn(); | ||
addFilter( | ||
'blocks.Autocomplete.completers', | ||
'test/autocompleters-hook', | ||
completersFilter | ||
); | ||
|
||
const expectedCompleters = [ {}, {}, {} ]; | ||
wrapper = mount( <FilteredComponent completers={ expectedCompleters } /> ); | ||
|
||
expect( completersFilter ).not.toHaveBeenCalled(); | ||
wrapper.find( 'input' ).simulate( 'focus' ); | ||
expect( completersFilter ).toHaveBeenCalledWith( expectedCompleters ); | ||
} ); | ||
|
||
it( 'filters completers supplied when already focused', () => { | ||
wrapper = mount( <FilteredComponent completers={ [] } /> ); | ||
|
||
wrapper.find( 'input' ).getDOMNode().focus(); | ||
expect( wrapper.getDOMNode().contains( document.activeElement ) ).toBeTruthy(); | ||
|
||
const completersFilter = jest.fn(); | ||
addFilter( | ||
'blocks.Autocomplete.completers', | ||
'test/autocompleters-hook', | ||
completersFilter | ||
); | ||
|
||
const expectedCompleters = [ {}, {}, {} ]; | ||
|
||
expect( completersFilter ).not.toHaveBeenCalled(); | ||
wrapper.setProps( { completers: expectedCompleters } ); | ||
expect( completersFilter ).toHaveBeenCalledWith( expectedCompleters ); | ||
} ); | ||
|
||
it( 'provides copies of completers to filter', () => { | ||
const completersFilter = jest.fn(); | ||
addFilter( | ||
'blocks.Autocomplete.completers', | ||
'test/autocompleters-hook', | ||
completersFilter | ||
); | ||
|
||
const specifiedCompleters = [ {}, {}, {} ]; | ||
wrapper = mount( <FilteredComponent completers={ specifiedCompleters } /> ); | ||
|
||
expect( completersFilter ).not.toHaveBeenCalled(); | ||
wrapper.find( 'input' ).simulate( 'focus' ); | ||
expect( completersFilter ).toHaveBeenCalledTimes( 1 ); | ||
|
||
const [ actualCompleters ] = completersFilter.mock.calls[ 0 ]; | ||
expect( actualCompleters ).not.toBe( specifiedCompleters ); | ||
expect( actualCompleters ).toEqual( specifiedCompleters ); | ||
} ); | ||
|
||
it( 'supplies filtered completers to inner component', () => { | ||
const expectedFilteredCompleters = [ {}, {} ]; | ||
const completersFilter = jest.fn( () => expectedFilteredCompleters ); | ||
addFilter( | ||
'blocks.Autocomplete.completers', | ||
'test/autocompleters-hook', | ||
completersFilter | ||
); | ||
|
||
wrapper = mount( <FilteredComponent /> ); | ||
|
||
wrapper.find( 'input' ).simulate( 'focus' ); | ||
|
||
const filteredComponentWrapper = wrapper.childAt( 0 ); | ||
const innerComponentWrapper = filteredComponentWrapper.childAt( 0 ); | ||
expect( innerComponentWrapper.name() ).toBe( 'TestComponent' ); | ||
expect( innerComponentWrapper.prop( 'completers' ) ).toEqual( expectedFilteredCompleters ); | ||
} ); | ||
|
||
it( 'passes props to inner component', () => { | ||
const expectedProps = { | ||
expected1: 1, | ||
expected2: 'two', | ||
expected3: '🌳', | ||
}; | ||
|
||
wrapper = shallow( <FilteredComponent { ...expectedProps } /> ); | ||
|
||
const innerComponentWrapper = wrapper.childAt( 0 ); | ||
expect( innerComponentWrapper.name() ).toBe( 'TestComponent' ); | ||
expect( innerComponentWrapper.props() ).toMatchObject( expectedProps ); | ||
} ); | ||
} ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Autocompleters | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a feeling that this should be documented next to the original There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tend to agree with you. I don't know why the completer interface should be documented within The blocks completer probably belongs within I feel better keeping the user completer within There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Where is a good place to provide such notes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, this was my exactly my point. It's a general interface that should work with every custom
I think it is enough to include |
||
============== | ||
|
||
The Autocompleter interface is documented [here](../../components/autocomplete/README.md) with the `Autocomplete` component in `@wordpress/components`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { sortBy, once } from 'lodash'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import './style.scss'; | ||
import { createBlock, getBlockTypes } from '../api'; | ||
import BlockIcon from '../block-icon'; | ||
|
||
/** | ||
* A blocks repeater for replacing the current block with a selected block type. | ||
* | ||
* @type {Completer} | ||
*/ | ||
export default { | ||
name: 'blocks', | ||
className: 'blocks-autocompleters__block', | ||
triggerPrefix: '/', | ||
options: once( function options() { | ||
return Promise.resolve( | ||
// Prioritize common category in block type options | ||
sortBy( | ||
getBlockTypes(), | ||
( { category } ) => 'common' !== category | ||
) | ||
); | ||
} ), | ||
getOptionKeywords( blockSettings ) { | ||
const { title, keywords = [] } = blockSettings; | ||
return [ ...keywords, title ]; | ||
}, | ||
getOptionLabel( blockSettings ) { | ||
const { icon, title } = blockSettings; | ||
return [ | ||
<BlockIcon key="icon" icon={ icon } />, | ||
title, | ||
]; | ||
}, | ||
allowContext( before, after ) { | ||
return ! ( /\S/.test( before.toString() ) || /\S/.test( after.toString() ) ); | ||
}, | ||
getOptionCompletion( blockData ) { | ||
return { | ||
action: 'replace', | ||
value: createBlock( blockData.name ), | ||
}; | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why we need this additional check in here? It seems like having
onFocus
handler should be enough.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is technically possible for the
completers
prop to change while the component contains the focus. This check exists for that case, sinceonFocus
will already have been called. I don't think that is likely to occur, but I'd rather just cover the case. Sound OK?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope it’s good 😃It makes sense, might make sense to leave an inline comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's mostly about not assuming when props will be updated. Sure, I can add a comment. :)