Skip to content

Refactor/update/jquery #2022

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

Open
wants to merge 4 commits into
base: gh-pages
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 17 additions & 18 deletions assets/js/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
// Read Sprockets README (https://github.com/rails/sprockets#sprockets-directives) for details
// about supported directives.
//
//= require jquery-1.7.1.min
//= require jquery-ui-1.8.18.custom.min
//= require jquery-3.7.1.min
//= require jquery-ui-1.14.1.min
Comment on lines -13 to +14
Copy link
Member

Choose a reason for hiding this comment

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

It does not make much sense to split the JQuery version updates in application.js from the ones in the header.

I am glad that you dropped the unrelated change from this commit where jquery.defaultvalue was dropped. But it seems you no longer drop this altogether? Is it still supported? Does it support the newest JQuery version?

//= require jquery.defaultvalue
//= require session.min

Expand Down Expand Up @@ -43,8 +43,7 @@ $(document).ready(function() {

function onPopState(fn) {
if (window.history && window.history.pushState) {
return $(window).bind('popstate', function(event) {
var section;
return $(window).on('popstate', function () {
Copy link
Member

Choose a reason for hiding this comment

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

Can you find some documentation about this? If so, that would be good information for the commit message.

Also: Does this new call work with the outdated JQuery version that git-scm.com currently has? If so, this commit needs to come before the upgrade to the newer JQuery.

initialPop = !popped && location.href === initialURL;
popped = true;
if (initialPop) {
Expand Down Expand Up @@ -115,7 +114,7 @@ var GitTurns20 = {
} else {
let start = 0
let count = 0
$("#tagline").click(e => {
$("#tagline").on('click', e => {
if (count === 0 || e.timeStamp > start + count * 1000) {
start = e.timeStamp;
count = 1;
Expand Down Expand Up @@ -181,20 +180,20 @@ var Search = {
},

observeFocus: function() {
$('form#search input').focus(function() {
$('form#search input').on('focus', function () {
$(this).parent('form#search').switchClass("", "focus", 200);
});
$('form#search input').blur(function() {
$('form#search input').on('blur', function () {
Search.resetForm();
});
},

observeTextEntry: function() {
$('form#search input').keyup(function(e) {
$('form#search input').on('keyup', function () {
Search.runSearch();
});

$('form#search input').keydown(function(e) {
$('form#search input').on('keydown', function (e) {
if ($('#search-results').not(':visible') && e.which != 27) {
$('#search-results').fadeIn(0.2);
Search.highlight(Search.selectedIndex);
Expand All @@ -220,16 +219,16 @@ var Search = {
},

observeResultsClicks: function() {
$('#search-results').mousedown(function(e) {
$('#search-results').on('mousedown', function (e) {
e.preventDefault();
});
},

installKeyboardShortcuts: function() {
$(document).keydown(function(e) {
$(document).on('keydown', function (e) {
if (e.target.tagName.toUpperCase() !== 'INPUT' && ['s', 'S', '/'].includes(e.key)) {
e.preventDefault();
$('form#search input').focus();
$('form#search input').trigger('focus');
}
else if (e.target.tagName.toUpperCase() !== 'INPUT') GitTurns20.keydown(e);
});
Expand Down Expand Up @@ -491,7 +490,7 @@ var Forms = {
},

observeCopyableInputs: function() {
$('input.copyable').click(function() {
$('input.copyable').on('click', function () {
$(this).select();
});
}
Expand Down Expand Up @@ -545,7 +544,7 @@ var Downloads = {
},

observeGUIOSFilter: function() {
$('a.gui-os-filter').click(function(e) {
$('a.gui-os-filter').on('click', function (e) {
e.preventDefault();
var os = $(this).attr('data-os');

Expand All @@ -565,7 +564,7 @@ var Downloads = {
},

observePopState: function() {
onPopState(function() {
onPopState(function () {
Comment on lines -568 to +567
Copy link
Member

Choose a reason for hiding this comment

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

That's not updating a deprecated function call, but it changes the code style to disagree with the remainder of the file.

Downloads.filterGUIS();
});
},
Expand Down Expand Up @@ -648,7 +647,7 @@ var DarkMode = {
}
button.css("display", "block");

button.click(function(e) {
button.on('click', function (e) {
e.preventDefault();
let theme
if (prefersDarkScheme) {
Expand Down Expand Up @@ -780,12 +779,12 @@ var PostelizeAnchor = {

// Scroll to Top
$('#scrollToTop').removeClass('no-js');
$(window).scroll(function() {
$(window).on('scroll', function () {
$(this).scrollTop() > 150
? $('#scrollToTop').fadeIn()
: $('#scrollToTop').fadeOut();
});
$('#scrollToTop').click(function(e) {
$('#scrollToTop').on('click', function (e) {
Comment on lines -783 to +787
Copy link
Member

Choose a reason for hiding this comment

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

I see a lot of $(...).<something>(...) -> $(...).on('<something>, ...) changes here. Do they all belong to the same class of deprecation, logically? If so, it's fine to cluster them in one large commit. Otherwise it makes reviewing easier (and hence bug catching, too, which is what reviewing is all about) to split the changes up by logical topic (which would then need to be described in the commit message, accompanied by references you can find).

e.preventDefault();
$("html, body").animate({
scrollTop: 0
Expand Down
4 changes: 2 additions & 2 deletions layouts/partials/footer.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
<img src="{{ relURL "images/icons/chevron-up@2x.png" }}" width="20" height="20" alt="scroll-to-top"/>
</a>

<script src="{{ relURL "js/jquery-1.7.1.min.js" }}"></script>
<script src="{{ relURL "js/jquery-ui-1.8.18.custom.min.js" }}"></script>
<script src="{{ relURL "js/jquery-3.7.1.min.js" }}"></script>
<script src="{{ relURL "js/jquery-ui-1.14.1.min.js" }}"></script>
<script src="{{ relURL "js/jquery.defaultvalue.js" }}"></script>
<script src="{{ relURL "js/session.min.js" }}"></script>
{{ $js := resources.Get "js/application.js" | resources.ExecuteAsTemplate "js/application.js" . | resources.Minify | fingerprint }}
Expand Down
2 changes: 2 additions & 0 deletions static/js/jquery-3.7.1.min.js

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions static/js/jquery-ui-1.14.1.min.js

Large diffs are not rendered by default.

Loading