Remove jq from set ga client #2260
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andysellick
reviewed
Nov 1, 2021
JonathanHallam
force-pushed
the
remove-jq-from-set-ga-client-id
branch
from
b73db0e to
ffb37c3
Compare
The only code in application.js is to initialise the ga tracker so they're being done at the same time. I've removed one `get` but not the other because the `tracker` object is defined by google while the `form` object is a standard HTML element You will notice that the application.js calls on the `getElementsByClassName` method which can return multiple results. Then we're only using the first element of that list. This isn't ideal but we have confidence that this class can only be used once on each page which makes this fine. Call route for the set-ga-client-id-on-form - application.js is the only file that calls on it - it only applies it to elements with the `js-service-sign-in-form` class - that class is only created by [this partial](https://github.com/alphagov/government-frontend/blob/main/app/views/content_items/service_sign_in/_choose_sign_in.html.erb#L25) - that partial is only rendered by [this page](https://github.com/alphagov/government-frontend/blob/main/app/views/content_items/service_sign_in.html.erb#L8) by way of a method in [this presenter](https://github.com/alphagov/government-frontend/blob/main/app/presenters/service_sign_in/choose_sign_in_presenter.rb#L5) - This means we can be confident there's only one element of that class on a page, huzah!
tracker is an object that comes directly from google. This is why the `get` method call has been kept on it, it's not a jQ object so the method is defined by google rather than jQ. I've renamed the `tracker` object in the tests to make it more obvious that it's something we don't have ownership over
JonathanHallam
force-pushed
the
remove-jq-from-set-ga-client-id
branch
from
ffb37c3 to
3d40e41
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Remove jQuery from the set ga client id file.
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.
Trello
https://trello.com/c/bqdisKbm/277-from-jquery-from-government-frontend-set-ga-client-id-on-formjs-and-applicationjs-moderate-js-knowledge-required