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

Post author dropdown: enable search #5921

Closed
wants to merge 10 commits into from
Closed

Conversation

adamsilverstein
Copy link
Member

Description

Change the post author component into a searchable react-select component.

Fixes #4622

  • Leverage react-select to create a dynamic select/dropdown for the author picker component: http://jedwatson.github.io/react-select/. I didn’t find an existing component like this in Gutenberg, did I miss one? This component seems solid, open to other suggestions.
  • initially, shows the first 100 users and dropdown works like existing dropdown - current user is the initial post author.
  • additionally, you can type into the field which begins a (debounced/cached) search, filling the list with available matches
  • when the post loads, checks if the post author is in the dropdown (one of the default, first 100 users), if not, calls the api to add that user to the dropdown (maybe this could be localized/done in php?)

How Has This Been Tested?

  • Tested creating new posts.
  • Tested basic keyboard functionality, could use a more complete a11y review.
  • Tested editing existing posts.
  • Tested on system with hundreds of users (leveraging wp user generate to generate test users)

Screenshots (jpeg or gifs if applicable):

This could use some design feedback. Currently uses the default CSS styles provided by react-select, with the control set to fill the full width

Types of changes

Switches the post author selector from a regular dropdown choosing from the to a searchable react component to select the author from all possible users.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@adamsilverstein adamsilverstein added Needs Design Feedback Needs general design feedback. Needs Testing Needs further testing to be confirmed. labels Apr 1, 2018
@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Apr 2, 2018
@afercia
Copy link
Contributor

afercia commented Apr 2, 2018

From an accessibility perspective, react-select is not so different from Select2. Unfortunately, it is not accessible. We've recently pinged the maintainers to ask fi they have plans to improve assistive technologies support, see JedWatson/react-select#2456. No reply, so far.

I'd rather suggest to consider to leverage the autocompleter component in Gutenberg.

@adamsilverstein
Copy link
Member Author

@afercia thanks for the feedback, that is disappointing and I was concerned about that. Have you come across any react select libraries that aim to be accessible?

I looked around the codebase for suitable similar components and thought autocompleter wouldn't quite fit, however I'll give it a try!

@mtias
Copy link
Member

mtias commented Apr 2, 2018

I'd rather suggest to consider to leverage the autocompleter component in Gutenberg.

I tend to agree here, we should try to use the same UI in both places and make sure all our polish efforts go to it (the @matcher interface should be essentially the same and exposed to 3rd party devs as a "UserSelect" component or similar). Also, react-select seems to bring a non insignificant code weight for just that UI element?

@afercia
Copy link
Contributor

afercia commented Apr 3, 2018

Yep, the problem though is the current UI is based on an input field, while this PR intent is to try with a select element. @adamsilverstein unfortunately I'm not aware of any "select-replacement" that is fully accessible.

@danielbachhuber
Copy link
Member

@adamsilverstein Can we close this in favor of #6180?

@adamsilverstein
Copy link
Member Author

@adamsilverstein Can we close this in favor of #6180?

@danielbachhuber I think these are pretty different!

this pr seeks to transform the user selector into an ajax-ified autocompleter. the only loads happen initially and in response to user actions. your linked issue attempts to load all users asynchronously which I don't think is as scalable or as usable as a search based selector. We can improve on core here, we'v long wanted to change this anyway but have been stymied by the lack of an accessible selector.

@danielbachhuber
Copy link
Member

We can improve on core here, we'v long wanted to change this anyway but have been stymied by the lack of an accessible selector.

Do we have an accessible selector now?

@adamsilverstein
Copy link
Member Author

Do we have an accessible selector now?

We have the autocomplete component which provides similar functionality.

@danielbachhuber
Copy link
Member

We have the autocomplete component which provides similar functionality.

Just to clarify, it resolves all prior core concerns with Select2?

@adamsilverstein
Copy link
Member Author

Just to clarify, it resolves all prior core concerns with Select2?

Great question, perhaps @afercia can confirm.

@afercia
Copy link
Contributor

afercia commented Apr 18, 2018

The autocompleters as implemented in Gutenberg are pretty accessible. Preferred version: the combobox with an input field and list of suggestions, for example: the Tags one.

However, if the intent is to dynamically add sets of new results to the list of suggestions, things change drastically. That's something really not well supported by assistive technologies and one of the main reasons Select2 and similar implementations are not accessible.

@adamsilverstein
Copy link
Member Author

if the intent is to dynamically add sets of new results to the list of suggestions

it is precisely that, and we need to do something similar on user autocomplete tags, etc. The alternative is to load all users/tags which could number in the tens of thousands. Loading all those results options into memory is a costly operation.

The autocompleters as implemented in Gutenberg are pretty accessible. Preferred version: the combobox with an input field and list of suggestions, for example: the Tags one.

I'm going with using the autocompleter feature here - its already quite powerful and close to what i imagine and then we can add any enhancements you can identify that would make the component more accessible, that way every part of the project will benefit.

@adamsilverstein
Copy link
Member Author

However, if the intent is to dynamically add sets of new results to the list of suggestions, things change drastically.

@afercia can you look at how the @ user (autocomplete) mentions work now - those have the searching already, and the list changes dynamically. Can you identify accessibility shortcomings or improvements we can make with the interactions there? if so, maybe open a new issue to address those specifically.

@danielbachhuber
Copy link
Member

To reduce complexity, let's follow existing WordPress behavior: autocomplete for tags, and load all users and categories.

@adamsilverstein
Copy link
Member Author

Existing WordPress behavior is not scalable and has been a pain point for some time. Why not fix it as part of the edit screen revamp? Using the autocomplete component will reduce the complexity.

@danielbachhuber
Copy link
Member

Existing WordPress behavior is not scalable and has been a pain point for some time.

For what percentage of WordPress sites is this existing behavior not scaleable? How many WordPress sites have thousands of authors?

Why not fix it as part of the edit screen revamp?

It's not clear to me that "fixing" it is a known amount of effort. We're at the point with Gutenberg where we need to wrap things up; using an existing UX pattern is a much more straightforward path.

Ultimately, whether or not to use autocomplete / autoselect UI (similar to tags) for authors is a design decision too. Given you can only select one author per post, it doesn't seem like the sensible approach.

cc @karmatosed for design input. I believe this is the current suggestion:

image

Also, @afercia's comment leads me to believe the implementation on this pull request is not accessible, because it's similar in UX to Select2.

@afercia
Copy link
Contributor

afercia commented Apr 21, 2018

First, I'd like to mention some previous discussions about this kind of auto-complete widgets with suggestions (in ARIA terms they're called "combobox") as we're basically repeating what has been discussed at length previously 🙂

Better select, multi-select, and autocomplete/suggestion inputs in the admin
https://core.trac.wordpress.org/ticket/31696
This is the ticket where Select2 was initially suggested. See @helen's framing of the issue.

Select2 a11y testing and evaluation
https://make.wordpress.org/accessibility/2015/09/07/accessibility-usertest-select2/

Historically, the critical point in the WP admin is the users select: the wp_dropdown_users() scalability and related performance issues. On multisite installations with hundreds or thousands of users, that's a problem. jQuery UI Autocomplete used there tries to mitigate the problem but it's not ideal.

dynamically add sets of new results

Let's call this "infinite scrolling" for the sake of simplicity. For example, the suggestions initially show 10 results. Scrolling the list, 10 additional results are added to the list so the total is now 20. Scrolling again, 10 more results are added so the total is now 30, and so on.

This kind of behavior is very problematic for assistive technologies because they can't recalculate the number of items in the list and correctly communicate that to users. If the initial results on first rendering are 10, browsers expose that information to assistive technologies, so they will announce "10" and, for each item, "1 of 10", "2 of 10", etc.
Updating the list adding new sets of results "on the fly" doesn't guarantee browsers are able to correctly expose the new information to assistive technologies. When I've tested this "infinite scrolling" on similar widgets, screen readers got totally confused and the widget was basically unusable.

Yes, in ARIA there are specific properties to handle this case:

aria-setsize
https://www.w3.org/TR/wai-aria-1.1/#aria-setsize

aria-posinset
https://www.w3.org/TR/wai-aria-1.1/#aria-posinset

So an "infinite scrolling" widget should use these properties but I really don't know anything about support for them (I doubt there's good support) and how screen readers behave, specifically if they're able to communicate n of n correctly after a new set of results is added.

Other potential issues I see with "infinite scrolling":

  • keyboard: the list of suggestions can be navigated with arrow keys: when a new set of results gets added, it is very likely arrow navigation will break
  • when a new set of results gets added, that should be communicated visually (e.g. "loading") and to screen reader users too with an audible message

For example:

  • I get 10 initial results
  • using the down arrow key, I can browse the list
  • when on the last item, the screen reader will announce "item xyz 10 of 10"
  • aside: at this point, whether I'm blind or not, how can I know that pressing down arrow or scrolling I get additional results?
  • when on the 10th item, I press the down arrow key
  • I should then get a message "new results added"
  • pressing the down arrow key, navigation should continue seamlessly and once on the 11th item, the screen reader should announce "item xyz 11 of 20"
  • and so on and on

This is way I've previously commented that with "infinite scrolling" things change drastically 🙂

I'd also like to mention this Trac ticket:

Reconsider the usage of infinite scrolling across the admin
https://core.trac.wordpress.org/ticket/40330

Where several concerns about infinite scrolling were expressed. Worth considering also the design feedback from @melchoyce when she suggests to not use infinite scrolling and use a button to load more results instead. However, I'd tend to think the real question is: is infinite scrolling the best we can do here?

The current autocompleters are pretty different because they don't add new sets of results. They rebuild the whole list so the information about the set size and the items position is correctly exposed to AT.

the @ user (autocomplete) mentions

It works pretty well. As far as I see, the only difference with the Tags autocompleter is that the edit field where users type to get suggestions is the block editable itself. Functionally, that's not so different from having a specific field for the search, like the Tags have.

Overall I'd recommend:

  • try not to use infinite scrolling
  • display an initial set of results: to define which users this initial set should show, the first 10? 100? in which order, alphabetical? most recent?
  • inform users that as long as they keep typing, new, more refined results will be shown

@karmatosed
Copy link
Member

I'm looping back some design feedback from Slack as this was discussed today.

@afercia those are some great links, really worth catching back up on even if people have read before.

I would second that drop-down scaling is 'lacking' to say the best. That in mind where possible autocompleting when it works well is a great solution. We have to be mindful of how this translate to accessibility though of course.

I do think infinite scroll can work well when done right, that issue is that's not often the case. Load more buttons are a solid solution for the problem of too many also and avoid the issues infinite scroll can bring. I do come back to though, having better ways to surface like autocomplete avoids us having to have that button in first place.

Overall I'd recommend:

try not to use infinite scrolling
display an initial set of results: to define which users this initial set should show, the first 10? 100? in which order, alphabetical? most recent?
inform users that as long as they keep typing, new, more refined results will be shown

I would recommend the above also and say first 10 in alphabetical. Potentially as someone used the interface I'd learn to changing that to most recent, but not totally sure there.

# Conflicts:
#	editor/components/post-author/index.js
@danielbachhuber
Copy link
Member

Thanks again for your work on this, @adamsilverstein. I want you to know that I appreciate the time you've spent exploring this option. If we can work through the accessibility and UX issues, I would love to explore an autocomplete implementation in the future. The decision to move forward with an unbounded query isn't personal; it's in the interest of needing something that works now so we can get closer to release.

Closing in favor of #6627

@karmatosed
Copy link
Member

karmatosed commented May 11, 2018

@chrisvanpatten that's an interesting prototype. I do feel that solution has slightly confused level of interactions. Do you have a working example of this as that would indicate it's something people understand as a pattern, right now I feel it may not be. That's not saying new things can't be explored.

Another issue is readability which links in the with levels problem. It's hard to read what is both going on and if there are more. In light of this I think maybe either iterating or exploring other options is a good approach, over moving this to an issue.

@adamsilverstein
Copy link
Member Author

adamsilverstein commented May 21, 2018

The latest code in this branch now includes author selection via the Autocomplete component. You can search for and select and author, and saving works as expected.

@afercia can you check this out to asses the accessibility of this approach? (also, did you get a chance to look at this select based option? https://github.com/alsoscotland/react-super-select)

Still a few things missing from the flow and design here:

  • show an initial selection before the user starts typing
  • better handling when a user selects an author to prevent additional typing
  • visual enhancements to the autocompleter to make it look/behav e more like a select element

@afercia
Copy link
Contributor

afercia commented May 24, 2018

Just to summarize the concerns expressed on the various GH issues, I'd like to link to some of the considerations made here and there.

In a previous comment, I've tried to illustrate what happens when dynamically adding sets of new results to the list of suggestions, and the difference between autocompleters and lists of suggestions with "infinite scroll".

Further considerations on "infinite scroll" are also available on #6180 (comment)
I'd like to highlight:

I see infinite scrolling as something that developers like more than users 🙂 It's a pattern that was vastly experimented in the past but today a quick search for infinite scrolling usability gives tons of articles highlighting its weak points. The fact is, infinite scrolling is not for all users, some will love it others will hate it. There are alternatives.

There's also a core Trac ticket about Infinite scroll, with interesting considerations also from a design / UX perspective:
Reconsider the usage of infinite scrolling across the admin
https://core.trac.wordpress.org/ticket/40330

For completeness, here's the Select2 accessibility testing and evaluation the a11y team posted on September 2015: https://make.wordpress.org/accessibility/2015/09/07/accessibility-usertest-select2/

Since then, some new select replacement like SelectWoo and react-select improved some of the accessibility issues but they're still not fully accessible.

I've looked a bit at the react-super-select "basic" example http://alsoscotland.github.io/react-super-select/react-super-select-examples.html#basic_example and, although it doesn't fully meet the ARIA patterns, it doesn't differ so much from the Gutenberg autocompleters. From an UX perspective, the main difference is that it presents an initial set of results.

In my opinion, the Gutenberg autocompleters work better. However, in terms of accessibility they're a trade-off and they're not guaranteed to work for all users. I think we can reasonably accept this trade-off but we should also be aware that some users might not be able to use the autocompleters. We should make sure they can at least manually enter a value. Just as en example, Safari+VoiceOver don't work very well with the ARIA combobox pattern, see the related core Trac ticket:
Fix autocomplete search suggestions support for Safari + VoiceOver
https://core.trac.wordpress.org/ticket/38637

I'd also like to mention @azaozz suggestion about this kind of searches, see https://core.trac.wordpress.org/ticket/37233#comment:21

Thinking that ideally it should trigger after 2 ASCII chars or one high UTF-8 char. We can standardize this for all similar cases in core, there are at least 6-7 other places.

@afercia
Copy link
Contributor

afercia commented May 24, 2018

@adamsilverstein looking a bit at the current state of this PR, I'm not sure why it's using a RichText component. This UI is basically an input field where users can type and get suggestions, so it should use a native input field. It also misses an ARIA role combobox. The implementation should match as much as possible the Tag suggestions.

Instead, the autocompleters used on the blocks are a different matter. The blocks editable areas need to use RichText and need to be announced as textbox; I'd admit the ARIA attributes used there don't fully meet the specification.

This is not the case for the authors selection which should be announced as a combobox. I'd suggest to have a look at the Tags autocompleter, which uses an <input type="text" role="combobox" etc...

@adamsilverstein
Copy link
Member Author

adamsilverstein commented May 24, 2018

@afercia - Thanks for the feedback!

looking a bit at the current state of this PR, I'm not sure why it's using a RichText component.

This was mostly a convenience as this PR is a proof of concept and autocompleters work with RichText out of the box. I'll take another pass using an input element, following the tag picker pattern or building a shared component.

The implementation should match as much as possible the Tag suggestions.

Good suggestion, I'll restart from there.

Two main differences for the author picker:

  • you can only select one author
  • you cannot add new authors

From an UX perspective, the main difference is that it presents an initial set of results.

This is an important distinction. Ideally, the author picker should immediately present a list of options when focused. I may explore using react-super-select as well to see if that offers any advantages.

@afercia
Copy link
Contributor

afercia commented May 24, 2018

Thanks @adamsilverstein . I was also wondering about another issue, not strictly related to the autocompleter. One of the features of a native select is that it shows users a list of existing users 🙂

Now, using an input field should also allow users to manually enter an username / email, especially if they don't want or can't use the autocompleter. When doing so, what happens when users enter a non-existing username / email? Currently, I can enter anything in the input field and there's no feedback at all. Seems to me a non-existing username doesn't get saved (that's correct!), but then worth considering some kind of message or warning to inform users.

Right now multisite > Edit Site > "Add existing user" uses jQuery autocomplete but it's also possible to manually enter any value and submit. When there's no match, a proper message is displayed:

screen shot 2018-05-24 at 12 04 26

@adamsilverstein
Copy link
Member Author

yea, the error message would be a minimum. ideally, i would imaging the control working so that if you try ty type a non existing name, the field doesn't let you - it reverts back to the existing state/selection. You would have to explicitly select something from the list to set the field value. (sound like a select field?)

Your point hints at a bigger issue i'm still not sure autocomplete can really replace a select component which is part of why i still want to try a select element based solution such as react-super-select if it can meet our accessibility requirements.

@afercia
Copy link
Contributor

afercia commented May 24, 2018

You would have to explicitly select something from the list to set the field value

My point was to allow users who don't want or can't use the autocompleter list to manually enter a value 🙂

@afercia
Copy link
Contributor

afercia commented May 24, 2018

I'd also like to link to a very interesting post by the UK gov digital service: https://accessibility.blog.gov.uk/2018/05/15/what-we-learned-from-getting-our-autocomplete-tested-for-accessibility/

Worth noting the digital service autocompleter is a really good one from an accessibility perspective, but as the post highlights there are users who struggle to use it. Testing with real users is always highly recommended, as well trying to avoid assumptions that it will work because, as developers, we think it works well 🙂

@adamsilverstein
Copy link
Member Author

adamsilverstein commented May 25, 2018

Very interesting, thanks! The article also links to their autocompleter which is open source and MIT licensed - https://github.com/alphagov/accessible-autocomplete

@afercia
Copy link
Contributor

afercia commented May 25, 2018

Yep I know, they've also promptly helped in giving some feedback on #5468 (comment) ❤️

@adamsilverstein
Copy link
Member Author

Yep I know, they've also promptly helped in giving some feedback on #5468 (comment) ❤️

That is good to hear, I'll work on an approach using their library and see how that works.

@adamsilverstein
Copy link
Member Author

Closing in favor of #7385

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 Testing Needs further testing to be confirmed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants