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

PLATUI-3353: proof of concept - how could we integrate accessible autocomplete fixes from adam's patch #419

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

oscarduignan
Copy link
Contributor

@oscarduignan oscarduignan commented Dec 4, 2024

proposal is that rather than copying adam's patches, because we're in charge of initialising the autocomplete elements, we can do it more simply/safely

the issues that need fixing

  • doesn't get error styles
  • when configured to "show all options" doesn't have a visible dropdown arrow to make them look like a select
  • doesn't get hints and error messages associated with via aria-describedby, so not announced when using screen reader
  • previously selected answer is retained when you enter an invalid option or didn't actually choose the matching option (if not using auto select)

adam's patch checks if there is an autocomplete on the page, and then polls until it's been setup by the accessible autocomplete javascript - we shouldn't need to do this because we are in charge of initialising the autocompletes

we can simplify the applying error styles by moving it into our css, where adam's patch is updating the dom to set a govuk error class on the autocomplete after the field changes

the aria-described by fix we can run directly after we call enhanceSelectElement

the issue with previouslySelected answers being retained we can fix by overriding the onConfirm handler for options - which triggers on blur of the field (regardless of auto select being enabled) and clearing the currently value of the underlying select, where adam's patch is hooking into the form submit

if we apply the patches in this way, then they should also work for any number of autocompletes on one page

if people are manually init'ing the autocompelte rather than using our data-module="hmrc-accessible-autocomplete" then they would just need to copy the js patches into their usage, which wouldn't be to onerous, since ideally not many people will be using that approach

@oscarduignan oscarduignan force-pushed the PLATUI-3353 branch 3 times, most recently from 7cef20f to 265b05e Compare December 5, 2024 14:29
… fixes from adams path

add suggested fix for invalid options being retained

fix indentation
@oscarduignan oscarduignan changed the title fix flakey test (maybe) and try one method of introducing integrating fixes from adam's patch proof of concept - how could we integrate accessible autocomplete fixes from adam's patch Dec 13, 2024
@oscarduignan
Copy link
Contributor Author

the test failures are visual regression tests where dropdown arrow is now visible correctly, if we go ahead with this, we probably want to look at the position of the dropdown arrow when the font has been scaled up, can't remember how we're changing the font size for this - assuming we're changing the root font size maybe

CleanShot 2024-12-13 at 16 11 23@2x

@oscarduignan oscarduignan changed the title proof of concept - how could we integrate accessible autocomplete fixes from adam's patch PLATUI-3353: proof of concept - how could we integrate accessible autocomplete fixes from adam's patch Dec 13, 2024
@oscarduignan
Copy link
Contributor Author

oscarduignan commented Dec 16, 2024

flagged up upstream some of the fixes just in case they can at some point be sorted there, since they are so small https://github.com/alphagov/accessible-autocomplete/pull/769/files

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.

1 participant