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

Restore accessible autocomplete component #2481

Closed
wants to merge 5 commits into from

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Nov 26, 2021

What

Restores the accessible autocomplete component, with changes.

This uses the accessible autocomplete code to progressively enhance a select element. This is based on the accessible autocomplete component that existed briefly in the gem until we didn't need it anymore, but with some changes, specifically:

  • we don't currently need tracking
  • tracking might be needed in the future so this includes a custom onConfirm function (a custom onConfirm was originally included because we needed to externally control the component in finder-frontend)
  • the autocomplete onConfirm seems to have a bug where it fires when the autocomplete is blurred, but passes undefined even if something has been correctly entered in the autocomplete. An event listener has been added to the component to compensate for this
  • all of the stuff to do with multiple selections has been removed, because we don't need that and it was really experimental
  • tests have been updated

Why

We're exploring using this component in a travel smart answer and it's currently the most suitable thing available.

Visual Changes

Screenshot 2021-12-03 at 12 27 16

@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2481 November 26, 2021 12:08 Inactive
@andysellick andysellick force-pushed the add-accessible-autocomplete branch from cff859c to dd62a75 Compare November 26, 2021 12:13
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2481 November 26, 2021 12:13 Inactive
- this is the code for the previous version of the component, removed in #1038
- does not use the git branch or code for multiple selections, as this is not needed
- otherwise unchanged and apparently functional, but some functionality here is not required and tests are failing
- finder-frontend had some very specific requirements for controlling the autocomplete outside of the component itself (facet tags) and this is not needed
@andysellick andysellick force-pushed the add-accessible-autocomplete branch from dd62a75 to c9e66e4 Compare November 29, 2021 15:29
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2481 November 29, 2021 15:30 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2481 November 29, 2021 15:34 Inactive
- also converts it to be a GOV.UK init module
- and remove tracking code
@andysellick andysellick force-pushed the add-accessible-autocomplete branch from ff6aeb6 to b224821 Compare November 29, 2021 15:41
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2481 November 29, 2021 15:42 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2481 December 3, 2021 10:51 Inactive
@andysellick andysellick force-pushed the add-accessible-autocomplete branch from 13d9551 to 041f409 Compare December 3, 2021 12:09
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2481 December 3, 2021 12:09 Inactive
@andysellick andysellick force-pushed the add-accessible-autocomplete branch from 041f409 to 61d02c7 Compare December 3, 2021 12:11
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2481 December 3, 2021 12:11 Inactive
- make the code easier to read and more clearly separated into functions
- adds a custom event listener for keyup, so that we can clear the hidden select if the autocomplete is cleared. This doesn't seem to work properly with the standard autocomplete
- update tests to remove jQuery and fix the events so they stop failing intermittently
@andysellick andysellick force-pushed the add-accessible-autocomplete branch from 61d02c7 to bd50892 Compare December 3, 2021 12:31
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2481 December 3, 2021 12:31 Inactive
@andysellick andysellick changed the title Add accessible autocomplete Restore accessible autocomplete component Dec 3, 2021
@andysellick andysellick marked this pull request as ready for review December 3, 2021 13:34
@andysellick andysellick requested a review from alex-ju December 6, 2021 08:33
@andysellick
Copy link
Contributor Author

I'm creating this with reservations - there's something of a disconnect between the autocomplete and the hidden select, in that it's possible to choose a valid option then edit it such that what the user thinks they've entered is different from what will actually be submitted in the hidden select.

The use of the autocomplete with a hidden select was chosen to make the non-JS version slightly more helpful for users than an empty input but I'm strongly considering changing this component to use a normal input and a datalist.

@alex-ju
Copy link
Contributor

alex-ju commented Dec 7, 2021

Before reviewing the code, if it's only needed in one application (be that smart-answers), shall we add it there instead (so we can iterate it quicker if needed)?

Update: as discussed, the component is used by other applications (content-data-admin and content-publisher) in different forms, so there may be scope in having it in the library. Note the content-publisher implementation uses a fork of the accessible-autocomplete.

@andysellick
Copy link
Contributor Author

Thanks for the information @alex-ju. content-data-admin uses the autocomplete component from the gem (a really old gem when that was still a component) and I have some concerns about the autocomplete component in content-publisher - the code looks quite complicated (it has multiple configuration options including enhancing a standard input and a hidden select and having a multiple select option, which is more than I need) and there doesn't seem to be a test file.

I'm still trying to decide how to proceed. I think my options are:

  1. port the autocomplete from content-publisher (although I might need to write a test file)
  2. write an autocomplete in smart-answers that only does what I need i.e. enhances a regular input and datalist
  3. write that 'simple' autocomplete here in the gem, then iterate it based on the content-publisher one with the aim of future consolidation

@alex-ju
Copy link
Contributor

alex-ju commented Dec 9, 2021

I'm sharing some thoughts on this, but you are closer to the problem than I am.

Thanks for the information @alex-ju. content-data-admin uses the autocomplete component from the gem (a really old gem when that was still a component) and I have some concerns about the autocomplete component in content-publisher - the code looks quite complicated (it has multiple configuration options including enhancing a standard input and a hidden select and having a multiple select option, which is more than I need) and there doesn't seem to be a test file.

I'm still trying to decide how to proceed. I think my options are:

  1. port the autocomplete from content-publisher (although I might need to write a test file)

The autocomplete code itself has unit tests in the main repo, hence in content-publisher we only made sure the different types of initialisations required in different areas of the project were captured in feature tests.

  1. write an autocomplete in smart-answers that only does what I need i.e. enhances a regular input and datalist

This approach has the advantage of being able to test and iterate quickly without needing to release a new version of the component. It can also be retired with much ease.

  1. write that 'simple' autocomplete here in the gem, then iterate it based on the content-publisher one with the aim of future consolidation

The work to converge/consolidate may fall between the cracks (as none the C19 or the Publishing team may be willing to take the time to do it).

@andysellick
Copy link
Contributor Author

@alex-ju I've created a new component in smart-answers based on this code and the code in content-publisher, see alphagov/smart-answers#5694

@andysellick andysellick deleted the add-accessible-autocomplete branch March 8, 2023 10:48
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.

3 participants