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

Add post author selector enhanced with ComboboxControl #23237

Merged
merged 84 commits into from
Sep 28, 2020

Conversation

adamsilverstein
Copy link
Member

@adamsilverstein adamsilverstein commented Jun 17, 2020

Description

Add accessible search with debounced api requests for the post author dropdown.

Built on the ComboBox being added in #19657 which relies on Downshift.

Supersedes #7385.

How has this been tested?

  • Tested with a dozen users, by adding them with wp-cli: wp user generate --role=editor --count=12

Regular dropdown shows.

  • Tested with 10100 users by adding them with wp-cli: wp user generate --role=editor --count=10100

Enhanced selector shows. Typing will begin searching in the selector.

Note: to clear out generated users, I used wp user delete $(wp user list --role=editor --field=ID) --reassign=2

Screenshots

Screen Recording 2020-06-19 at 11 27 AM

Types of changes

  • Only load the first 100 users from the API at load.
  • Load the post author object by id at load.
  • Use a debounced search to query the API for matching users while the user types.
  • Question: should we start searching once the user types 2 characters?
  • Use progressive enhancement to add the enhanced selector when there are more than 50 users to choose from.
  • I haven't added any styles, I am assuming the underlying component will include these.
  • I added a loading results state to show a spinner while fetching authors, should this be tracked in the store vs the component?

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@adamsilverstein adamsilverstein changed the base branch from master to add/combobox-control-component June 17, 2020 19:59
@adamsilverstein adamsilverstein changed the title Add/7385 author combobox 2 Add post author selector enhanced with Downshift Jun 19, 2020
@github-actions
Copy link

github-actions bot commented Jun 19, 2020

Size Change: -179 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/block-directory/index.js 8.53 kB +1 B
build/block-editor/index.js 128 kB +66 B (0%)
build/block-library/index.js 135 kB +42 B (0%)
build/blocks/index.js 47.5 kB +1 B
build/components/index.js 168 kB -55 B (0%)
build/components/style-rtl.css 15.4 kB +24 B (0%)
build/components/style.css 15.4 kB +24 B (0%)
build/core-data/index.js 12 kB +27 B (0%)
build/data-controls/index.js 1.27 kB -1 B
build/data/index.js 8.43 kB -1 B
build/edit-post/index.js 306 kB +1 B
build/edit-site/index.js 20.5 kB +2 B (0%)
build/edit-widgets/index.js 17.7 kB -122 B (0%)
build/edit-widgets/style-rtl.css 2.73 kB -96 B (3%)
build/edit-widgets/style.css 2.73 kB -97 B (3%)
build/editor/index.js 45.4 kB +8 B (0%)
build/rich-text/index.js 13.7 kB -2 B (0%)
build/server-side-render/index.js 2.6 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.52 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.57 kB 0 B
build/block-library/editor.css 8.58 kB 0 B
build/block-library/style-rtl.css 7.61 kB 0 B
build/block-library/style.css 7.6 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.78 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/compose/index.js 9.42 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/index.js 10.7 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/style-rtl.css 6.25 kB 0 B
build/edit-post/style.css 6.24 kB 0 B
build/edit-site/style-rtl.css 3.54 kB 0 B
build/edit-site/style.css 3.54 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/style-rtl.css 3.83 kB 0 B
build/editor/style.css 3.82 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.49 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 1.74 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.55 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.34 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

packages/components/src/combobox-control/index.js Outdated Show resolved Hide resolved
packages/core-data/src/selectors.js Outdated Show resolved Hide resolved
packages/editor/src/components/post-author/index.js Outdated Show resolved Hide resolved
@youknowriad
Copy link
Contributor

Noticed a weird glitch here (I'm testing on safari), When I click on one of the select options (and before releasing the button), the panel chevron icons on the sidebar move to the right.

@adamsilverstein
Copy link
Member Author

Noticed a weird glitch here (I'm testing on safari), When I click on one of the select options (and before releasing the button), the panel chevron icons on the sidebar move to the right.

Does it also affect the font size dropdown selector where the combobox is already used?

@youknowriad
Copy link
Contributor

No, it only affects the author control, that said the font size one is not a ComboboxControl, it's a CustomSelectControl.

@adamsilverstein
Copy link
Member Author

No, it only affects the author control, that said the font size one is not a ComboboxControl, it's a CustomSelectControl.

Oh, I see now. I thought previously that the selector used ComboboxControl, maybe that changed?

@afercia afercia added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). and removed Needs Accessibility Feedback Need input from accessibility labels Sep 15, 2020
@afercia
Copy link
Contributor

afercia commented Sep 15, 2020

For a first accessibility feedback, please see this comment on the related PR for the Page parent combobox: #25267 (comment)

@adamsilverstein
Copy link
Member Author

@afercia I took a pass at starting implementing the author selector using the same component the tag autocomplete selector uses: FormTokenField in #25420

It sort of works and illustrates some of the fundamental UX differences here:

  • The tag selector never shows a dropdown of selections, Downshift shows a default list of items (maybe this is ok - for <= 50 users we can show a standard select element)
  • The tag selector is designed to contain multiple tags, the page/author selectors are designed to choose a single item. I hack that by hiding the existing tag, with mixed results.
  • The tag selector is designed to allow you to "clear" all of the tags and have a blank field. This should be possible for pages (no parent) but not authors.
  • one other limitation - values in the selector are flat, not objects, so we can't store both the author name and id. instead we combine the two and extract them using a regex when selected.

Given the fact that this component already has excellent accessibility, does it seem worth trying to extend the underlying component to better fit our use case?

@afercia
Copy link
Contributor

afercia commented Sep 17, 2020

@adamsilverstein thanks for further exploring this issue!

Well if you ask me, I see a value in abstracting the Gutenberg combobox (auto-completer) implementation and make it more reusable, not only for internal usage but also for usage by plugins. I think I mentioned this a few times in other issues 🙂 As per the technical side, I'm afraid I don't have the skills to be of help with coding.

The tag selector never shows a dropdown of selections, Downshift shows a default list of items (maybe this is ok - for <= 50 users we can show a standard select element)

Using a native <select> element for a small amount of users seems a very good idea. I'm not sure a search / filtering / suggestions UI would make sense for, say, 5 users even from a visual perspective 🙂

@youknowriad youknowriad changed the title Add post author selector enhanced with Downshift Add post author selector enhanced with ComboboxControl Sep 28, 2020
@youknowriad
Copy link
Contributor

I updated this to use the new ComboboxControl.
I had to update the component a bit cause its behavior didn't match the real use-case expectations but I believe it's good now.

@youknowriad
Copy link
Contributor

I think we can explore isLoading prop on a separate PR to get it right.

@youknowriad youknowriad merged commit 78dde89 into master Sep 28, 2020
@youknowriad youknowriad deleted the add/7385-author-combobox-2 branch September 28, 2020 13:14
@github-actions github-actions bot added this to the Gutenberg 9.1 milestone Sep 28, 2020
@adamsilverstein
Copy link
Member Author

🎉

Thank you @youknowriad for working on this. I'll give this final version a test to see if I notice any issues. I have been working on these selectors for a long time (years?!) so it is really exciting to see this get fixed!

@adamsilverstein
Copy link
Member Author

Confirming this works well in my test, excited to see this land!

@adamsilverstein
Copy link
Member Author

@adamsilverstein thanks for further exploring this issue!

Well if you ask me, I see a value in abstracting the Gutenberg combobox (auto-completer) implementation and make it more reusable, not only for internal usage but also for usage by plugins. I think I mentioned this a few times in other issues 🙂 As per the technical side, I'm afraid I don't have the skills to be of help with coding.

The tag selector never shows a dropdown of selections, Downshift shows a default list of items (maybe this is ok - for <= 50 users we can show a standard select element)

Using a native <select> element for a small amount of users seems a very good idea. I'm not sure a search / filtering / suggestions UI would make sense for, say, 5 users even from a visual perspective 🙂

@afercia We addressed this in #26426 for the post author selector (and I plan to do the same for the page parent selector soon). Thanks again for all your guidance on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants