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

Making progressive enhancement to selects behave more like a select #467

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mauricioac
Copy link

First of all, thank you all that helped develop this great library.

We, at Bit Zesty, maintain a couple of government services, specifically the GOV.UK Trade Tariff and the Queens Awards for Enterprise.

We were updating these apps to use the new Design System with the frontend library and had to replace the library we've been using; Select2.

We came across this library, but we weren't quite looking for an autocomplete library, but a select enhancement library. We saw it had some support, so we extended it so that the behaviour of keys and the overall behaviour matches the browser's native select.

In this PR, we propose merging back into mainstream the following changes:
(for all the items below, it's only when progressively enhancing a select):

  • On menu open, display all options
  • Make dropdown open with Space and Enter keys
  • When select has no selection and is opened, focus on the first option
  • When select has a selected option and is open, to focus on the selection
  • Keys up and down wrap around the options list

Some of these changes also address issue #240

We also made a further change (which we are happy to make as a separate PR if needed) to enable programmatic control over the selection, specifically, we exposed a clearSelection() method so that we can easily clear the selection from a button in a simple way.

This solves issue #334

We added a suite of tests to cover the changes and the programmatic control API.

Below I'm attaching some GIFs to more easily demonstrate the changes made.

I appreciate any feedback and happy to make changes if needed.


ps: on the left is the current behaviour, and on the right, is the updated.

gif1

gif2

gif3

Maurício André Cinelli and others added 2 commits October 1, 2020 16:59
- Adding sensible defaults
- Modifying the autoselect library to make keyboard interactions to support enhanced select dropdowns
- Exposing a public API, with 1 function now to clear selection, which can be expanded to more functions later
- Expanding tests to cover new functionality
@edwardhorsford
Copy link
Contributor

@mauricioac would you be able to deploy somewhere so people can try this?

@mauricioac
Copy link
Author

Hi @edwardhorsford . Thanks for taking a look.

I made a deployment on heroku:

http://accessible-autocomplete-pr-467.herokuapp.com/examples/

@edwardhorsford
Copy link
Contributor

@mauricioac I no longer work at GDS so can't help with getting this merged I'm afraid. But I'm interested for my own projects that might take advantage of it.

Something I'm not clear of: is this new default behaviour, is it new behaviour for showAllValues option, or is it a new config?

@mauricioac
Copy link
Author

@edwardhorsford it would be a default behaviour regardless of showAllValues, but only when doing progressive enhancement of a select.

The reationale was that showAllValues worked only when there was no previous selection. When the select had a previous selection, when opening the dropdown, only the selected value would show up.
And this behaviour makes sense for the autocomplete.
We thought this behaviour did not sound right for selects, so we changed by default.
I will be honest that I did not really think of putting under a flag or something, so if needed, I'll scope it under an option.

The problem of scoping under the existing showAllValues is that it would still have to behave differently when in progressive enhancement vs autocomplete, since from my understanding, these two cases have different expected behaviours.

@edwardhorsford
Copy link
Contributor

@mauricioac I've had a play with it.

I agree for the showAllValues I'd prefer when you focus it with an existing value it should show all values again - this was raised here.

I may see if my team (DfE) can adopt your fork in places - assuming some of the below bugs get resolved.

Some comments after playing with your version.

  • Clicking in to a pre-filled value shows the full list (good) - but a second click shows just the item - I'm unsure about this behaviour. My reckon is it should not do anything further / should collapse the list. As it is it seems to be a weird inbetween state. Gif:
    accessible-autocomplete-two-clicks
  • I think the arrow should remain in place throughout to distinguish the 'buttoney' area vs the results.
  • Using just keyboard, there seem to be some bugs where it will remain in this state after you focus the next input:

Screenshot 2020-10-07 at 13 11 04

* It seems to be showing a placeholder in some cases - unsure what's happening here:

Screenshot 2020-10-07 at 13 17 31

* I'm unsure about focusing the first item by default.

Since this is changing behaviour rather significantly, have you done any testing with assistive technologies? if so, what have you found? which technologies have you tested with?

Sidebar: I think this is a case where we should also support disabled so you can have true empty / select a foo first entries that can't then be selected.

@mauricioac
Copy link
Author

Clicking in to a pre-filled value shows the full list (good) - but a second click shows just the item - I'm unsure about this behaviour. My reckon is it should not do anything further / should collapse the list. As it is it seems to be a weird inbetween state. Gif:

I think it's because the second click is going to focus on the input, and that would filter out the results, which then reduces the list. Still need to think what should be done in this case and how it could be fixed.

I think the arrow should remain in place throughout to distinguish the 'buttoney' area vs the results.

I agree. Will get this one fixed

Using just keyboard, there seem to be some bugs where it will remain in this state after you focus the next input:

I did not test abandoning the select while typing before. Really interesting case though...

  • It seems to be showing a placeholder in some cases - unsure what's happening here:

On our apps we did not use placeholder, so I did not test it. I'll try to see what this is about and if I can fix it.
I've seen an issue open here on github about the placeholder getting all messed up, but maybe I can fix it along.

I'm unsure about focusing the first item by default.
Since this is changing behaviour rather significantly, have you done any testing with assistive technologies? if so, what have > you found? which technologies have you tested with?

I have tested with chromevox, since it's what I have on my linux machine. I did not spot any issues when I last tested the services. However, I noticed now that with the placeholder, the second time the dropdown is opened with keyboard arrows the focus goes to the first real option, the one after the placeholder. I'll take a look at this as well and see if I can fix this issue.

I'm not sure if this is the behaviour you're describing as buggy. If yes, I totally agree. Because it really should be focusing on the placeholder, not the option after it. If this is not the case you're describing, could you please expand on it?

Sidebar: I think this is a case where we should also support disabled so you can have true empty / select a foo first entries that can't then be selected.

Hm.... very interesting. I'll give this a test and see how default browser select behaves and how hard it will be to replicate this behaviour.

@edwardhorsford Thank you for giving this branch a test and finding these issues. I'm gonna see if I can reproduce and fix them.

@edwardhorsford
Copy link
Contributor

@mauricioac no problem. Having spent a lot of time working on this with @tvararu I remember how tricky it is. There's loads and loads of edge cases to think about. I'm motivated to help if I can though because I use showAllValues frequently and it doesn't work well for changing existing answers.

I suspect for this to be merged by GDS (which I don't work for anymore) it would need significant testing in various AT. Possibly they could help with some of that - as each AT wouldn't take long. Chromevox is not commonly used by real users. When we first built and tested this component we found tonnes of inconsistency / issues we had to work around. In particular I expect this will need to do different things with focus, communicating that focus, and communicating what happens when the menu opens.

On our apps we did not use placeholder, so I did not test it. I'll try to see what this is about and if I can fix it.
I've seen an issue open here on github about the placeholder getting all messed up, but maybe I can fix it along.

Normally we'd advise not having one - but I don't think it should break like this. What you're seeing is the hint overlaid on top, which should only happen when something is typed in and it matches / a result is selected but not confirmed. I suspect you've collapse the menu, but some bit of the autocomplete code still thinks it's open.

I'm not sure if this is the behaviour you're describing as buggy.
I think I was referring to getting it in to a state where the menu remains open but narrowed to one option - even after you've focused something else.

Another sidebar: I've never been happy with the inability to clear answers in the autocomplete. Possibly less relevant as selects can't be cleared either - but if it's emulating a select more closely then I think it would be good to explore an unset default state that you can return to.

@mauricioac
Copy link
Author

@edwardhorsford got a chance to work on the issues you mentioned. Unfortunately I did not mess around disabled options, as that would require changing the indexes used for tabbing, and when using lengths, as those would have to count the disabled ones, but not allow them to be selected.

I did manage to fix a lot of the issues you mentioned, even a comment on your last response was a bug I fixed:

Another sidebar: I've never been happy with the inability to clear answers in the autocomplete. Possibly less relevant as selects can't be cleared either - but if it's emulating a select more closely then I think it would be good to explore an unset default state that you can return to.

it was clearing to the defaultValue which could mean a saved answer when editing. The clear behaviour now really clears the select, which was the intention from the beginning.

I will see if I can get a VM up and running and install a bunch of ATs in it, and try to get someone to test using voice over on mac.

Thanks for all the help so far in finding issues with my implementation.

@frankieroberto
Copy link

As @edwardhorsford points out, one of the issues when showing all after clicking when there's an existing selection is when do you go back into "filter" mode? When making any changes to the selected text?

I also wonder whether the "currently selected" option needs some kind of visual indicator when showing all values? Currently it highlights in blue initially, but if you hover over any other option this indication disappears, even though the previous option is still selected.

An alternative option to explore might be an enhanced select which behaves more like a regular select - ie always shows all options, but perhaps highlights matching ones instead.

Another alternative option might be to, once an option has been selected, remove the autocomplete completely and just display the selected option in text, alongside a "change" button which re-reveals the autocomplete and resets it to blank, with a cancel option to return to the existing value.

@stuaxo
Copy link

stuaxo commented May 19, 2021

Is this still alive ?

Maybe it would make sense to split this into separate PRs so things can be merged one by one ?

@dracos
Copy link
Contributor

dracos commented Oct 12, 2022

Thanks for this, I have taken the fix for #240 from this (would be good if that could be extracted out to deal with that issue on its own).

dracos added a commit to mysociety/fixmystreet that referenced this pull request Oct 13, 2022
Take a fix from a wider PR
alphagov/accessible-autocomplete#467 that fixes
alphagov/accessible-autocomplete#240 so that
after an option is chosen, opening the menu still shows all the values,
rather than only the chosen option.
@matthewford
Copy link

matthewford commented May 16, 2023

Is this still alive ?

Maybe it would make sense to split this into separate PRs so things can be merged one by one ?

Well, if there is interest we can revive this; we still use it in the services we maintain.

jeremyZX added a commit to jeremyZX/accessible-autocomplete that referenced this pull request Aug 23, 2023
@romaricpascal romaricpascal added this to the After next milestone Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants