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

Remove JQuery from EnableAriaControls module #2519

Merged
merged 1 commit into from
May 26, 2021

Conversation

leenagupte
Copy link
Contributor

Trello: https://trello.com/c/jK6hrK3d

What's changed?

Replace jQuery code in GOV.UK JavaScript (JS) code with vanilla (plain/normal) JavaScript in the EnableAriaControls module.

Why?

We're using an old and unsupported version of jQuery for browser support reasons. Rather than upgrade, it's far better to remove our dependence.

jQuery makes writing JavaScript easier, but it doesn't do anything that you can't do with vanilla JavaScript, because it's all written in JavaScript.

Once it's removed, we no longer have to worry about upgrading it, and users don't have to download the jQuery library when they visit GOV.UK.

Co-authored-by: Andy Sellick andy.sellick@digital.cabinet-office.gov.uk

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

@bevanloon bevanloon temporarily deployed to finder-front-convert-en-yuug0e May 25, 2021 12:37 Inactive
We're using an old and unsupported version of jQuery for browser support
reasons. Rather than upgrade, it's far better to remove our dependence.

jQuery makes writing JavaScript easier, but it doesn't do anything that
you can't do with vanilla JavaScript, because it's all written in
JavaScript.

Once it's removed, we no longer have to worry about upgrading it, and
users don't have to download the jQuery library when they visit GOV.UK.

Co-authored-by: Andy Sellick <andy.sellick@digital.cabinet-office.gov.uk>
@leenagupte leenagupte force-pushed the convert-enable-aria-controls branch from 42174ad to 66c0d32 Compare May 25, 2021 12:38
@bevanloon bevanloon temporarily deployed to finder-front-convert-en-yuug0e May 25, 2021 12:38 Inactive
Copy link
Contributor

@owenatgov owenatgov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks peachy.

One non-blocking query I have is what this code is for. I wasn't patient enough to go looking for a live example but from what it looks like, we're sending markup to the frontend with a data-attribute containing aria controls. Why can't we just set aria controls at the template level and bypass the need for this module altogether? Is this a govspeak thing or something?

@leenagupte
Copy link
Contributor Author

Code looks peachy.

One non-blocking query I have is what this code is for. I wasn't patient enough to go looking for a live example but from what it looks like, we're sending markup to the frontend with a data-attribute containing aria controls. Why can't we just set aria controls at the template level and bypass the need for this module altogether? Is this a govspeak thing or something?

@owenatgov Thanks for the review. To be honest, I don't know how the code is used. I think perhaps @andysellick might be able to answer the question about why the aria-controls are set as they are.

@leenagupte leenagupte merged commit 334d249 into main May 26, 2021
@leenagupte leenagupte deleted the convert-enable-aria-controls branch May 26, 2021 09:03
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.

None yet

3 participants