Remove jquery from govspeak #1657
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
force-pushed
the
remove-jquery-from-govspeak
branch
from
03eba2c to
15c87bb
Compare
- relies on magna charta, which is a jquery plugin, so we have to convert the passed element to a jquery object still, but this will be dealt with later
- magna-charta (https://github.com/alphagov/magna-charta) is an archived project and relies on jquery, which we are trying to remove - this commit introduces a local, unminified copy of the code to the gem and adds a test for it, based on the original code's test: https://github.com/alphagov/magna-charta/blob/master/test/magna-charta_test.js - the main js file has also been tidied up due to linting requirements - this has all been done so that we can rewrite magna-charta to not use jquery at a later date
- had to include an option for testing to return a reference to the object, in order not to have to completely rewrite the tests - at least one test was redundant so have removed, others have been modified
andysellick
force-pushed
the
remove-jquery-from-govspeak
branch
from
15c87bb to
ce9d4d4
Compare
injms
reviewed
Aug 28, 2020
app/assets/javascripts/govuk_publishing_components/components/govspeak.js
Outdated
Show resolved
Hide resolved
injms
reviewed
Aug 28, 2020
app/assets/javascripts/govuk_publishing_components/lib/govspeak/barchart-enhancement.js
Outdated
Show resolved
Hide resolved
injms
reviewed
Aug 28, 2020
app/assets/javascripts/govuk_publishing_components/lib/govspeak/barchart-enhancement.js
Outdated
Show resolved
Hide resolved
injms
reviewed
Aug 28, 2020
app/assets/javascripts/govuk_publishing_components/lib/govspeak/magna-charta.js
Outdated
Show resolved
Hide resolved
injms
reviewed
Aug 28, 2020
app/assets/javascripts/govuk_publishing_components/lib/govspeak/magna-charta.js
Outdated
Show resolved
Hide resolved
injms
reviewed
Aug 28, 2020
app/assets/javascripts/govuk_publishing_components/lib/govspeak/magna-charta.js
Show resolved
Hide resolved
injms
suggested changes
Aug 28, 2020
- includes a few extra lines of code, but because it worked in IE8 with jquery before it was converted, and the changes to support IE8+ are relatively minor, I'm operating on the principle of 'do no harm'
andysellick
force-pushed
the
remove-jquery-from-govspeak
branch
from
ce9d4d4 to
b446ca9
Compare
|
Thanks @injms. I've gone with |
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 govspeak, and associated scripts (youtube link enhancement and bar chart enhancement).
Govspeak relies on magna charta, a jquery plugin, originally included as a minified copy of the code. Magna charta has now been archived, so I've copied the code into the gem and rewritten it as a module without jquery. This has included copying the original tests across from magna charta and rewriting them.
Ultimately we may want to replace magna charta altogether, but this change at least gives us a functioning, non-jquery version that we can apply fixes/changes to if required at any point.
Tests still use jquery in places but jasmine has its own jquery so we don't need to worry about that.
Why
To reduce our dependency on jquery.
Visual Changes
None.