-
Notifications
You must be signed in to change notification settings - Fork 1
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
Quick: Remove Unused JavaScript #475
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notes for reviewers.
/* WARNING: This code is deprecated. It must be removed AFTER GH-195/FP-1015. */ | ||
|
||
/** | ||
* Load JSON from an endpoint, and perform success-dependent actions | ||
* @param {string} path - The path to an API endpoint | ||
* @param {function} onSuccess - Callback for data retrieval success | ||
* @param {function} onError - Callback for data retrieval success | ||
*/ | ||
function getData(path, onSuccess, onError) { | ||
var xhr = new XMLHttpRequest(); | ||
xhr.onreadystatechange = function () { | ||
if (xhr.readyState === XMLHttpRequest.DONE) { | ||
if (xhr.status === 200) { | ||
if (typeof onSuccess === 'function') { | ||
onSuccess(JSON.parse(xhr.responseText)); | ||
} | ||
} else { | ||
// NOTE: The original code did not ever log an error, thus `undefined` | ||
onError(xhr, undefined); | ||
} | ||
} | ||
}; | ||
xhr.open("GET", path, true); | ||
xhr.send(); | ||
} | ||
|
||
(function () { | ||
/* | ||
* Show (and populate) correct Portal navigation menu | ||
* - If user is authenticated: | ||
* - Populate username (to show that user is logged in) | ||
* - Show portal nav dropdown (it is pre-hidden via classname) | ||
* - If user is NOT authenticated: | ||
* - Show portal login link (it is pre-hidden via classname) | ||
*/ | ||
getData('/api/users/auth/', | ||
function populateData(data) { | ||
console.info('portal user data', data); | ||
|
||
document.getElementById('portal-username').textContent = data.username; | ||
// FAQ: Show the auth'd menu | ||
document.getElementById('auth-menu').classList.remove('d-none'); | ||
}, | ||
function outputRequest(xhr, isFatal) { | ||
if (isFatal) console.error(xhr); | ||
|
||
// FAQ: Show the un-auth'd menu | ||
document.getElementById('unauth-menu').classList.remove('d-none'); | ||
} | ||
); | ||
})(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explanation: This whole file is unused. Instead of manipulating duplicated HTML, we now load the server-side–prepared HTML directly into place.
'use strict'; | ||
|
||
console.info('Core `site.js` has no logic.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original Intent: Note to self that there is a site.js
for site-wide JavaScript. But no site-wide JavaScript has since been needed.
Overview
Remove cruft JavaScript files.
Related
none
Changes
site.js
and call to load it.nav_portal.js
(long since deprecated and never loaded).Testing
site.js
is not loaded.Screenshots