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

Upgrade jQuery #3017

Merged
merged 3 commits into from May 21, 2021
Merged

Upgrade jQuery #3017

merged 3 commits into from May 21, 2021

Conversation

dracos
Copy link
Member

@dracos dracos commented May 4, 2020

Does what it says on the tin.

I followed the jQuery upgrade instructions, and ran migrate alongside each upgrade, and fixed the obvious things. Not sure we can know whether everything has been caught.

I manually fixed issues in fancybox, we should do a comparator of similar more modern things and pick the nicest/smallest.

(diff is low because lots of the click() -> on('click') changes are within parts of the code not tested.)

@dracos dracos requested a review from zarino May 4, 2020 16:01
@dracos dracos force-pushed the remove-ie8-upgrade-jquery branch from bc96d85 to 8900bb7 Compare May 4, 2020 18:23
@codecov
Copy link

codecov bot commented May 4, 2020

Codecov Report

Merging #3017 (0a213b0) into master (08d6e6c) will decrease coverage by 26.32%.
The diff coverage is 93.02%.

❗ Current head 0a213b0 differs from pull request most recent head ba6788c. Consider uploading reports for the commit ba6788c to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           master    #3017       +/-   ##
===========================================
- Coverage   82.55%   56.23%   -26.33%     
===========================================
  Files         324      310       -14     
  Lines       21810    21051      -759     
  Branches     3283     3130      -153     
===========================================
- Hits        18006    11837     -6169     
- Misses       2757     8279     +5522     
+ Partials     1047      935      -112     
Impacted Files Coverage Δ
web/cobrands/fixmystreet/fixmystreet.js 73.28% <92.85%> (+1.22%) ⬆️
web/js/map-OpenLayers.js 77.24% <100.00%> (-3.35%) ⬇️
perllib/FixMyStreet/Roles/BoroughEmails.pm 6.25% <0.00%> (-93.75%) ⬇️
perllib/FixMyStreet/Map/Bing.pm 6.66% <0.00%> (-93.34%) ⬇️
perllib/FixMyStreet/WorkingDays.pm 6.66% <0.00%> (-93.34%) ⬇️
perllib/FixMyStreet/SendReport/Email/Highways.pm 7.14% <0.00%> (-92.86%) ⬇️
perllib/FixMyStreet/Map/OS/FMS.pm 9.09% <0.00%> (-90.91%) ⬇️
perllib/FixMyStreet/Map/CheshireEast.pm 9.09% <0.00%> (-90.91%) ⬇️
perllib/FixMyStreet/Cobrand/Zurich.pm 3.25% <0.00%> (-87.58%) ⬇️
perllib/FixMyStreet/Cobrand/Bromley.pm 4.44% <0.00%> (-87.41%) ⬇️
... and 169 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08d6e6c...ba6788c. Read the comment docs.

@dracos dracos force-pushed the remove-ie8-upgrade-jquery branch from 8900bb7 to 1547727 Compare May 5, 2020 09:54
@dracos dracos marked this pull request as ready for review May 5, 2020 12:37
Copy link
Member

@zarino zarino left a comment

Choose a reason for hiding this comment

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

Looks good! Tests pass, and I’ve checked it out and poked around locally. I wonder whether we should test the core javascript functionality in IE9 on BrowserStack, just to be sure that nothing we’ve done has affected support there? Seems unlikely, but you never know.

Nice to have stripped out all that IE6–8 cruft! And a more modern version of jQuery too. Lovely.

@dracos dracos force-pushed the remove-ie8-upgrade-jquery branch from 1547727 to d2e4b6b Compare May 15, 2020 11:49
@dracos dracos force-pushed the remove-ie8-upgrade-jquery branch 2 times, most recently from bf47d47 to a1e6881 Compare July 15, 2020 11:07
@dracos dracos force-pushed the remove-ie8-upgrade-jquery branch 10 times, most recently from e802f8b to 986e531 Compare November 23, 2020 12:31
@dracos dracos changed the title Drop IE8 support, upgrade jQuery Upgrade jQuery Jan 7, 2021
@dracos dracos force-pushed the remove-ie8-upgrade-jquery branch 2 times, most recently from 53c6d0d to f72db98 Compare May 20, 2021 18:36
@dracos dracos force-pushed the remove-ie8-upgrade-jquery branch 2 times, most recently from 86ac4c8 to 79a2179 Compare May 20, 2021 20:32
dracos and others added 2 commits May 21, 2021 10:25
Include some tests for the drawer opening.
Remove use of jQuery UI spinner entirely, and cut down the included
jQuery UI CSS/JS to only the used autocomplete library.
@dracos dracos force-pushed the remove-ie8-upgrade-jquery branch from 79a2179 to ba6788c Compare May 21, 2021 09:26
@dracos dracos merged commit ba6788c into master May 21, 2021
@github-pages github-pages bot temporarily deployed to github-pages May 21, 2021 09:58 Inactive
@dracos dracos mentioned this pull request May 21, 2021
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

2 participants