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.

Sign up for GitHub

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

Modernise javascript #402

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Modernise javascript #402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a910a9e
the !== operator is used for strict inequality comparison, while the…
MunyaradziMagura May 23, 2023
ae8b7ae
replacing var with const because const is immutable and the variables…
MunyaradziMagura May 23, 2023
91944f0
added commenting to better explain how the url is being manipulated
MunyaradziMagura May 23, 2023
1531c4a
replaced var with const where applicable and console logged the error
MunyaradziMagura May 23, 2023
795ec03
Update team_data_sets.js
MunyaradziMagura May 23, 2023
df33de7
Update scorecards.js
MunyaradziMagura May 23, 2023
42e770a
Update projects.js
MunyaradziMagura May 23, 2023
e71fb30
Update entries.js
MunyaradziMagura May 23, 2023
6257d46
made name relate more to the function of the variable
MunyaradziMagura May 23, 2023
ed4b7e2
added error log to catch
MunyaradziMagura May 23, 2023
bb07c67
made immutable variables const
MunyaradziMagura May 24, 2023
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
17 changes: 7 additions & 10 deletions app/assets/javascripts/admin/teams/scorecards.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,18 @@

$( document ).on('ready turbolinks:load', function() {

var windowObjectReference;
var strWindowFeatures = "menubar=no,location=no,resizable=yes,scrollbars=yes,status=no,width=700,height=800";
const strWindowFeatures = "menubar=no,location=no,resizable=yes,scrollbars=yes,status=no,width=700,height=800";

function openRequestedPopup(team_scorecards_path) {
windowObjectReference = window.open(team_scorecards_path, 'Team Scorecards', strWindowFeatures);
}
const openRequestedPopup = (team_scorecards_path) => windowObjectReference = window.open(team_scorecards_path, 'Team Scorecards', strWindowFeatures);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is windowObjectReference used somewhere?


function refreshParent() {
window.opener.location.reload();
}

const refreshParent = () => window.opener.location.reload();
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find its call. Could you please check if there is?



$('.open-team-scorecards').click(function (event) {
event.preventDefault();
var team_id = $(this).data("id");
var include_judges =$(this).data("include-judges");
const team_id = $(this).data("id");
const include_judges =$(this).data("include-judges");
openRequestedPopup(_admin_teams_path + '/' + team_id + '/scorecards?include_judges='+ include_judges +'&popup=true');
});

Expand Down
18 changes: 11 additions & 7 deletions app/assets/javascripts/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ $(document).ready(function() {

$( document ).on('turbolinks:load', function() {
// Initilize the table and save any state changes.
var table = $('[id$="table"]').DataTable({
const table = $('[id$="table"]').DataTable({
stateSave: true,
responsive: true
});
Expand All @@ -42,18 +42,22 @@ $( document ).on('turbolinks:load', function() {
});

// Check if filter needs to be applied
if ((window._search != undefined) && (window._search.length > 0 )) {
if ((window._search !== undefined) && (window._search.length > 0 )) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested edge cases? AFAIK there is a difference between != and !==. For example,

null != undefined       // false
null !== undefined      // true

// Apply the filter param to the filter box.
table.search(window._search);
table.draw();

// Clear the filter in the URL when the search param is updated.
table.on( 'search.dt', function () {
if ((table.search() != window._search) && (window._search.length > 0 )) {
var url = window.location.href;
var tail = url.substring(url.lastIndexOf('/') + 1);
var clean_tail = tail.substring(0, tail.lastIndexOf('?'));
var clean_title = document.title.replace(/#(.*?)\s/, '');
if ((table.search() !== window._search) && (window._search.length > 0 )) {
const url = window.location.href;
const tail = url.substring(url.lastIndexOf('/') + 1);

// Remove any query parameters from the segment
const clean_tail = tail.substring(0, tail.lastIndexOf('?'));
Comment on lines +53 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the whole purpose of these lines are to clean up the url. In this case we should wrap it into one function. Please, take this code as an example only:

Suggested change
const url = window.location.href;
const tail = url.substring(url.lastIndexOf('/') + 1);
// Remove any query parameters from the segment
const clean_tail = tail.substring(0, tail.lastIndexOf('?'));
...
const url_tail_without_parameters = url => {
const tail = url.substring(url.lastIndexOf('/') + 1);
// Remove any query parameters from the segment
return tail.substring(0, tail.lastIndexOf('?'));
}
...
const clean_tail = url_tail_without_parameters(window.location.href);


// Clean up the document title by removing text after '#' symbol and whitespace
const clean_title = document.title.replace(/#(.*?)\s/, '');

history.replaceState({}, clean_title, clean_tail);
document.title = clean_title;
Expand Down
12 changes: 4 additions & 8 deletions app/assets/javascripts/projects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,12 @@
// All this logic will automatically be available in application.js.
$( document ).on('ready turbolinks:load', function() {

var windowObjectReference;
var strWindowFeatures = "menubar=no,location=no,resizable=yes,scrollbars=yes,status=no,width=500,height=800";
const strWindowFeatures = "menubar=no,location=no,resizable=yes,scrollbars=yes,status=no,width=500,height=800";

function openRequestedPopup() {
windowObjectReference = window.open(_scorecard_path, 'Scorecard', strWindowFeatures);
}
const openRequestedPopup = () => windowObjectReference = window.open(_scorecard_path, 'Scorecard', strWindowFeatures);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is windowObjectReference used somewhere?



function refreshParent() {
window.opener.location.reload();
}
const refreshParent = () => window.opener.location.reload();
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find its call. Could you please check if there is?


$('#open-scorecard').click(function (event) {
event.preventDefault();
Expand Down
20 changes: 11 additions & 9 deletions app/assets/javascripts/team_management/entries.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@

$( document ).on('ready turbolinks:load', function() {
$("#team_data_set_url").blur(function() {
var url = $("#team_data_set_url").val();
var separated = url.split("/dataset/");
console.log(url);
const teamUrlData = $("#team_data_set_url").val().split("/dataset/");

var data = {
id: separated[1]
id: teamUrlData[1]
}
$.ajax({
type: "GET",
data: data,
url: separated[0] + "/api/3/action/package_show",
url: teamUrlData[0] + "/api/3/action/package_show",
success: function (data) {
$("#team_data_set_name").val(data.result.title);
$("#team_data_set_description").val(data.result.notes);

$("#hiddendescription").width($("#data_set_description").width());
var content = data.result.notes.replace(/\n/g, "<br>");

const content = data.result.notes.replace(/\n/g, "<br>");

$("#hiddendescription").html(content);
$("#team_data_set_description").css("height", $("#hiddendescription").outerHeight());
},
Expand All @@ -27,10 +27,12 @@ $( document ).on('ready turbolinks:load', function() {
});
try {
$("#hiddendescription").width($("#data_set_description").width());
var content = $("#team_data_set_description").val().replace(/\n/g, "<br>");

const content = $("#team_data_set_description").val().replace(/\n/g, "<br>");

$("#hiddendescription").html(content);
$("#team_data_set_description").css("height", $("#hiddendescription").outerHeight());
} catch (e) {

console.error('An error occured.', e)
Copy link
Contributor

Choose a reason for hiding this comment

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

Dp we use some error monitoring service like BugSnag?

}
});
19 changes: 14 additions & 5 deletions app/assets/javascripts/team_management/team_data_sets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

$(document).ready(function() {
$("#team_data_set_url").blur(function() {
var url = $("#team_data_set_url").val();
var separated = url.split("/dataset/");
const url = $("#team_data_set_url").val();
const separated = url.split("/dataset/");

var data = {
id: separated[1]
Expand All @@ -18,19 +18,28 @@ $(document).ready(function() {
$("#team_data_set_description").val(data.result.notes);

$("#hiddendescription").width($("#team_data_set_description").width());
var content = data.result.notes.replace(/\n/g, "<br>");

const content = data.result.notes.replace(/\n/g, "<br>");

$("#hiddendescription").html(content);
$("#team_data_set_description").css("height", $("#hiddendescription").outerHeight());
},
dataType: "jsonp"
})
});

try {

$("#hiddendescription").width($("#team_data_set_description").width());
var content = $("#team_data_set_description").val().replace(/\n/g, "<br>");

const content = $("#team_data_set_description").val().replace(/\n/g, "<br>");

$("#hiddendescription").html(content);
$("#team_data_set_description").css("height", $("#hiddendescription").outerHeight());

} catch (e) {


console.error("An error occurred:", e);

}
});