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

Support a more flexible useItems API for the autocompleters API #22853

Merged
merged 4 commits into from
Jun 22, 2020

Conversation

youknowriad
Copy link
Contributor

Right now, the autocompleters API is a bit limitted and relies only on global functions to fetch the options. It also doesn't allow us to reuse logic easily: for instance the Inserter use hooks to fetch the available items to insert but these hooks can't be reused inside the autocompleter itself which make it very hard to support things like: block variations, block patterns... without duplicating a lot of logic.

This PR updates the Autocomplete component to rely on a useItems hook in the autocompleter definition. If the hook has not been provided, a default one relying on the existing autocompeters API is used.

That component is a bit complex, so potential regressions are not excluded.

As a follow-up I'll be updating the block autocompleter to support patterns and block variations.

Testing instructions

  • Make sure the user and block autocompleters work as expected.

@youknowriad youknowriad added [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Feature] UI Components Impacts or related to the UI component system labels Jun 3, 2020
@youknowriad youknowriad requested review from gziolo and ellatrix June 3, 2020 09:33
@youknowriad youknowriad self-assigned this Jun 3, 2020
@github-actions
Copy link

github-actions bot commented Jun 3, 2020

Size Change: -165 B (0%)

Total Size: 1.12 MB

Filename Size Change
build/block-editor/style-rtl.css 10.7 kB -44 B (0%)
build/block-editor/style.css 10.6 kB -47 B (0%)
build/components/index.js 195 kB -169 B (0%)
build/edit-post/style-rtl.css 5.48 kB -21 B (0%)
build/edit-post/style.css 5.47 kB -23 B (0%)
build/editor/editor-styles-rtl.css 537 B +69 B (12%) ⚠️
build/editor/editor-styles.css 539 B +70 B (12%) ⚠️
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.27 kB 0 B
build/block-directory/style-rtl.css 937 B 0 B
build/block-directory/style.css 937 B 0 B
build/block-editor/index.js 107 kB 0 B
build/block-library/editor-rtl.css 7.59 kB 0 B
build/block-library/editor.css 7.59 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 8.02 kB 0 B
build/block-library/style.css 8.02 kB 0 B
build/block-library/theme-rtl.css 730 B 0 B
build/block-library/theme.css 732 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.2 kB 0 B
build/components/style-rtl.css 15.8 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 9.61 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.17 kB 0 B
build/edit-navigation/index.js 9.64 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.02 kB 0 B
build/edit-site/style.css 3.02 kB 0 B
build/edit-widgets/index.js 9.33 kB 0 B
build/edit-widgets/style-rtl.css 2.42 kB 0 B
build/edit-widgets/style.css 2.42 kB 0 B
build/editor/index.js 44.8 kB 0 B
build/editor/style-rtl.css 3.81 kB 0 B
build/editor/style.css 3.81 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.13 kB 0 B
build/list-reusable-blocks/style-rtl.css 450 B 0 B
build/list-reusable-blocks/style.css 451 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 663 B 0 B
build/nux/style.css 660 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@ellatrix
Copy link
Member

I'm fine with this, if it helps. Would be nice to see usage of the hook

@ellatrix ellatrix closed this Jun 12, 2020
@ellatrix ellatrix reopened this Jun 12, 2020
@gziolo
Copy link
Member

gziolo commented Jun 17, 2020

E2e tests that use the slash inserter fail on Travis, I think it's a bad sign 😅

@gziolo
Copy link
Member

gziolo commented Jun 17, 2020

I tested it locally and it works perfectly fine in all the cases I tested:

  • slash inserter, including reusable blocks
  • mentions in blocks that use RichText (Quote, Paragraph, List)

@youknowriad youknowriad force-pushed the update/autocompleter-hook branch from d37c1ce to 73dbcd3 Compare June 19, 2020 12:45
@youknowriad youknowriad force-pushed the update/autocompleter-hook branch from feb1020 to 5754c4b Compare June 19, 2020 19:01
@youknowriad youknowriad force-pushed the update/autocompleter-hook branch from 5754c4b to 2cca698 Compare June 22, 2020 09:16
@youknowriad
Copy link
Contributor Author

The test changes are actually intermittent failures on master too. they are though harder to trigger on master (I was able to do it something though)

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Based on my previous testing 👍

@youknowriad youknowriad merged commit 3156d0b into master Jun 22, 2020
@youknowriad youknowriad deleted the update/autocompleter-hook branch June 22, 2020 13:42
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jun 22, 2020
Comment on lines +145 to +147
const useItems = autocompleter.useItems
? autocompleter.useItems
: ( filterValue ) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we confident now to document useItems, or is there a reason to wait?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some potential things we may want to add, like a way to show alternative UIs (not just a flat list) but I do feel confident about useItems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Feature] UI Components Impacts or related to the UI component system [Type] New API New API to be used by plugin developers or package users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants