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

Autocomplete links to user posts #2896

Merged
merged 50 commits into from Nov 3, 2017
Merged
Changes from 15 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
2667d2d
Autocomplete links to user posts
EphoxJames Oct 6, 2017
96ed19e
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 9, 2017
5db79cd
add/2793: Changed autocomplete to take in "completer" definitions
EphoxJames Oct 9, 2017
fe4aa22
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 10, 2017
2386c1f
add/2793: Improved inline documentation
EphoxJames Oct 10, 2017
d8d91b4
add/2793: Removed old autocomplete components
EphoxJames Oct 11, 2017
053a478
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 11, 2017
fc53c3b
add/2793: Updating tests [WIP]
EphoxJames Oct 11, 2017
46875ad
add/2793: Updated the rest of the existing tests
EphoxJames Oct 13, 2017
ad8d036
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 13, 2017
f5eb752
add/2793: Removed blank line that linter complained about
EphoxJames Oct 15, 2017
ad93a3e
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 15, 2017
29b0a36
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 17, 2017
a23a608
add/2793: Allow onSelect to optionally return selection replacement
EphoxJames Oct 17, 2017
e2988b4
add/2793: Made linter happy again
EphoxJames Oct 17, 2017
4c0c793
add/2793: extract out findMatch; load options on demand; handle tab
EphoxJames Oct 18, 2017
ca6c469
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 18, 2017
83d697e
add/2793: Hide the popover on left and right arrow keys
EphoxJames Oct 18, 2017
22baa78
add/2793: Moved options into state and reload them on each new open
EphoxJames Oct 18, 2017
3c5350a
add/2793: Removed unused allOptions variable
EphoxJames Oct 18, 2017
94110cd
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 18, 2017
5220299
add/2793: changed onClickOutside to onClose
EphoxJames Oct 19, 2017
158c9d0
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 20, 2017
8791dd3
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 23, 2017
79712b8
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 24, 2017
56c0889
add/2793: Updated tests to work with updated enzyme
EphoxJames Oct 24, 2017
58f703e
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 25, 2017
ed9bd85
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 30, 2017
f48d7e1
Fixed linting error.
EphoxJames Oct 30, 2017
8ae2e80
add/2793: Try to make autocomplete popover accessable
EphoxJames Oct 30, 2017
cb5aa58
add/2793: Updated autocomplete tests to handle FACC structure
EphoxJames Oct 30, 2017
f6234ae
add/2793: delegate handling of nil aria attributes to Editable
EphoxJames Oct 30, 2017
7089532
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Oct 31, 2017
4f437e3
add/2793: Disabled linter warning about findDOMNode as it is required
EphoxJames Oct 31, 2017
2207026
add/2793: Converted autocomplete FACC parameters into object
EphoxJames Nov 2, 2017
2c75ae1
add/2793: Moved test that selection is within container into assertion
EphoxJames Nov 2, 2017
aa420d2
add/2793: Converted `Array#map` and `Array#filter` to lodash equalivents
EphoxJames Nov 2, 2017
41e10da
add/2793: Reorder calculations to avoid doing unneeded work
EphoxJames Nov 2, 2017
5a25e32
add/2793: Break out of loop early to avoid excess indentation
EphoxJames Nov 2, 2017
c06e95e
add/2793: Removed unused 'range' parameter of allowContext
EphoxJames Nov 2, 2017
dccc61e
add/2793: Removed unneeded if-statment
EphoxJames Nov 2, 2017
3d7ffd5
add/2793: Avoid some indentation with an early return
EphoxJames Nov 2, 2017
bdae604
add/2793: Replace escapeStringRegexp with escapeRegExp from lodash
EphoxJames Nov 2, 2017
a25f6f7
add/2793: Removed unneeded guard statement
EphoxJames Nov 2, 2017
78221f5
add/2793: Avoid spurious react reconcile by unwrapping function
EphoxJames Nov 2, 2017
4e995dc
add/2793: Moved bounding client rect calculation into else branch
EphoxJames Nov 2, 2017
20fa7fb
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Nov 2, 2017
bbae623
add/2793: Only take padding into account when using the anchor
EphoxJames Nov 2, 2017
b9871c6
Add/2793: Allow popover position to be overriden by prop getAnchorRect
EphoxJames Nov 3, 2017
8fc7023
Merge branch 'master' into add/2793-user-autocompletion
EphoxJames Nov 3, 2017
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -0,0 +1,128 @@
/**
* Internal dependencies
*/
import './style.scss';
import { createBlock, getBlockTypes } from '../api';
import BlockIcon from '../block-icon';

/**
* @typedef {Object} CompleterOption
* @property {Array.<Component>} label list of react components to render.
* @property {Array.<String>} keywords list of key words to search.
* @property {*} value the value that will be passed to onSelect.
*/

/**
* @callback FnGetOptions
* @returns {Promise.<Array.<CompleterOption>>} A promise that resolves to the list of completer options.
*/

/**
* @callback FnAllowNode
* @param {Node} textNode check if the completer can handle this text node.
* @returns {boolean} true if the completer can handle this text node.
*/

/**
* @callback FnAllowContext
* @param {Range} before the range before the auto complete trigger and query.
* @param {Range} range the range of the autocomplete trigger and query.
* @param {Range} after the range after the autocomplete trigger and query.
* @returns {boolean} true if the completer can handle these ranges.
*/

/**
* @callback FnOnSelect
* @param {*} value the value of the completer option.
* @param {Range} range the nodes included in the autocomplete trigger and query.
* @param {String} query the text value of the autocomplete query.
* @returns {?Component} optional html to replace the range.
*/

/**
* @typedef {Object} Completer
* @property {?String} className A class to apply to the popup menu.
* @property {String} triggerPrefix the prefix that will display the menu.
* @property {FnGetOptions} getOptions get the block options in a resolved promise.
* @property {?FnAllowNode} allowNode filter the allowed text nodes in the autocomplete.
* @property {?FnAllowContext} allowContext filter the context under which the autocomplete activates.
* @property {FnOnSelect} onSelect
*/

/**
* Returns an "completer" definition for selecting from available blocks to replace the current one.
* The definition can be understood by the Autocomplete component.
*
* @param {Function} onReplace Callback to replace the current block.
* @returns {Completer} Completer object used by the Autocomplete component.
*/
export function blockAutocompleter( { onReplace } ) {
const options = getBlockTypes().map( ( blockType ) => {
const { name, title, icon, keywords = [] } = blockType;
return {
value: name,
label: [
<BlockIcon key="icon" icon={ icon } />,
title,
],
keywords: [ ...keywords, title ],
};
} );

const getOptions = () => Promise.resolve( options );

const allowContext = ( before, range, after ) => {
return ! ( /\S/.test( before.toString() ) || /\S/.test( after.toString() ) );
};

const onSelect = ( blockName ) => {
onReplace( createBlock( blockName ) );
};

return {
className: 'blocks-block-autocomplete',
triggerPrefix: '/',
getOptions,
allowContext,

This comment has been minimized.

Copy link
@youknowriad

youknowriad Oct 13, 2017

Contributor

Can't understand the difference between triggerPrefix and allowContext, could these be merged into one function?

This comment has been minimized.

Copy link
@EphoxJames

EphoxJames Oct 15, 2017

Author Contributor

TLDR: not without making it an annoying API to use.

triggerPrefix is the string that preceeds the query. For example "@" in the case of the user mentions, "/" in the case of block selection and "#" in the case of hash-tags (which aren't implemented yet). In theory you could just search back to the last space or boundary and pass the whole range to the completer definition to check but that that means that ALL completers would have to convert the range into text and check the leading substring. This puts more work on the implementer of the completer which is exactly where we don't want it.

allowContext was added to solve the problem that sometimes the completer has to know what preceeds it and follows it. The majority of the time this doesn't matter but the original implementation of the autocomplete block would require that there was nothing but whitespace before and afterwards (which makes sense because it replaces the whole block). This field is completely optional and most completers won't use it.

onSelect,
};
}
/**
* Returns a "completer" definition for inserting links to the posts of a user.
* The definition can be understood by the Autocomplete component.
*
* @returns {Completer} Completer object used by the Autocomplete component.
*/
export function userAutocompleter() {
const getOptions = () => {
return ( new wp.api.collections.Users() ).fetch().then( ( users ) => {

This comment has been minimized.

Copy link
@aduth

aduth Oct 17, 2017

Member

We might consider the implications for users who don't have permissions to list other users on a site. Currently this doesn't appear to break anything, but will only ever return administrator users if issued by a non-administrator. Probably fine, with the other option being that we don't enable the autocompleter for these accounts.

This comment has been minimized.

Copy link
@EphoxJames

EphoxJames Oct 17, 2017

Author Contributor

If you know of a better source for the information I will happily switch to it.

return users.map( ( user ) => {
return {
value: user,
label: [
<img key="avatar" alt="" src={ user.avatar_urls[ 24 ] } />,

This comment has been minimized.

Copy link
@aduth

aduth Oct 17, 2017

Member

We should assign a meaningful alt here, maybe { __( 'User avatar' ) }

This comment has been minimized.

Copy link
@EphoxJames

EphoxJames Oct 17, 2017

Author Contributor

The problem with that is that screen readers will (for each item) read out "User avatar" before telling them the actually useful information of the user-name. Unless we can give an actually useful description it's probably more usable if they are just treated as decoration.

This comment has been minimized.

Copy link
@EphoxJames

EphoxJames Oct 18, 2017

Author Contributor

@afercia do you think we should put descriptive text on the avatar in the users menu or treat it as decorative?

This comment has been minimized.

Copy link
@afercia

afercia Oct 20, 2017

Contributor

@EphoxJames empty alt are fine in this case. The relevant information is already in plain text close to the image, so these images are decorative.
Although not exactly the same thing, we've faced a similar case in the WP credits page. See https://core.trac.wordpress.org/changeset/36406 and https://core.trac.wordpress.org/ticket/34953.

<span key="name" className="name">{ user.name }</span>,
<span key="slug" className="slug">{ user.slug }</span>,
],
keywords: [ user.slug, user.name ],
};
} );
} );
};

const allowNode = ( textNode ) => {
return textNode.parentElement.closest( 'a' ) === null;
};

const onSelect = ( user ) => {
return <a href={ user.link }>{ '@' + user.name }</a>;
};

return {
className: 'blocks-user-autocomplete',
triggerPrefix: '@',
getOptions,
allowNode,
onSelect,
};
}
@@ -0,0 +1,12 @@
.blocks-block-autocomplete .dashicon {
margin-right: 8px;
}

.blocks-user-autocomplete img {
margin-right: 8px;
}

.blocks-user-autocomplete .slug {
margin-left: 8px;
color: $dark-gray-100;
}

This file was deleted.

This file was deleted.

This file was deleted.

@@ -8,17 +8,17 @@ import classnames from 'classnames';
*/
import { __ } from '@wordpress/i18n';
import { concatChildren } from '@wordpress/element';
import { PanelBody } from '@wordpress/components';
import { Autocomplete, PanelBody } from '@wordpress/components';

/**
* Internal dependencies
*/
import './style.scss';
import { registerBlockType, createBlock, source, setDefaultBlockName } from '../../api';
import { blockAutocompleter, userAutocompleter } from '../../autocompleters';
import AlignmentToolbar from '../../alignment-toolbar';
import BlockAlignmentToolbar from '../../block-alignment-toolbar';
import BlockControls from '../../block-controls';
import BlockAutocomplete from '../../block-autocomplete';
import Editable from '../../editable';
import InspectorControls from '../../inspector-controls';
import ToggleControl from '../../inspector-controls/toggle-control';
@@ -152,7 +152,10 @@ registerBlockType( 'core/paragraph', {
</PanelBody>
</InspectorControls>
),
<BlockAutocomplete key="editable" onReplace={ onReplace }>
<Autocomplete key="editable" completers={ [
blockAutocompleter( { onReplace } ),
userAutocompleter(),
] }>

This comment has been minimized.

Copy link
@youknowriad

youknowriad Oct 13, 2017

Contributor

Nice 👍

This comment has been minimized.

Copy link
@youknowriad

youknowriad Oct 13, 2017

Contributor

Thinking the "patterns" we have right now in Editable could be written like this cc @iseulde

<Editable
tagName="p"
className={ classnames( 'wp-block-paragraph', className, {
@@ -183,8 +186,8 @@ registerBlockType( 'core/paragraph', {
onMerge={ mergeBlocks }
onReplace={ onReplace }
placeholder={ placeholder || __( 'New Paragraph' ) }
/>
</BlockAutocomplete>,
/>
</Autocomplete>,
];
},

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.