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

ATGU-507: Course Setup Checklist Updates #24

Merged
merged 7 commits into from
Aug 15, 2016
61 changes: 38 additions & 23 deletions js/checklist.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
(function() {
// Hard-coded external tool IDs for account_id=39 (i.e. Harvard College/GSAS)
var POLICY_WIZARD_TOOL_ID = 1509;
var MANAGE_COURSE_TOOL_ID = 17079;

// Ensure that these scripts only run on the appropriate pages
var is_course_page = ("/courses/" == window.location.pathname.substr(0, "/courses/".length));
if (is_course_page) {
Expand All @@ -9,7 +13,8 @@
require(['jquery'], modify_import_content_page);
}
}



/**
* This function adds some text about migrating content from iSites to the "Import Content" page.
*
Expand All @@ -20,7 +25,9 @@
function modify_import_content_page($) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this shouldn't live in checklist.js but rather be a separate file, no? I guess it was like this before, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it probably should live in a separate file since that code isn't really part of the checklist module. Good suggestion.


// Holds the content that will be added to the page
var html = "<p>If you would like to incorporate content from a previous iSite, please contact the Academic Technology Group at <a href=\"mailto:atg@fas.harvard.edu\">atg@fas.harvard.edu</a>.</p>";
var BASE_COURSE_URL = window.location.pathname.replace("/content_migrations", "");
var url = BASE_COURSE_URL + "/external_tools/" + MANAGE_COURSE_TOOL_ID;
var html = '<p>If you would like to incorporate content from a previous iSite, click <a href="'+url+'">here</a>.</p>';
Copy link
Contributor

Choose a reason for hiding this comment

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

For greater accessibility, a suggestion would be to make the link text more useful to screen readers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, that link could be much clearer to screen readers. Aside from changing the link text itself, would an aria-label attribute be appropriate here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Chatted with @Vittorio2015 about this; he suggested including a title attribute on the anchor link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, the title attribute makes complete sense. Done!


// Holds a function that when executed, will call its callback when the selector returns an element.
// The assumption is that the element may not exist in the DOM on the first try.
Expand All @@ -42,13 +49,13 @@
*/
function pollForElement(el, num_tries, timeout, success) {
var callback = function() {
var exists = $(el).length != 0;
var exists = $(el).length !== 0;
--num_tries;
if (exists) {
success($(el));
} else {
if (num_tries > 0) {
window.setTimeout(callback, timeout)
window.setTimeout(callback, timeout);
}
}
};
Expand Down Expand Up @@ -111,34 +118,42 @@
* or if the external tools themselves are modified such that the IDs are no longer valid.
*
*/

// Hard-coded external tool IDs
var POLICY_WIZARD_TOOL_ID = 1509; // Tool ID for account_id=39
var MANAGE_PEOPLE_TOOL_ID = 3958; // Tool ID for account_id=39


// Base course URL (i.e. /courses/1234)
var BASE_COURSE_URL = window.location.pathname;
var DEBUG = false; // (window.location.pathname == "/courses/39");

//----- CHANGE #1 -----
// REMOVE: Modify "Import Content" item
ListItems[0].text = "If you've been using another course management system, you probably have stuff in there that you're going to want moved over to Canvas. We can walk you through the process of easily migrating your content into Canvas. If you would like to incorporate content from a previous iSite, please contact the Academic Technology Group at atg@fas.harvard.edu";

//---------------------------------
// CHANGE: "Import Content" item
ListItems[0].text = "If you've been using another course management system, you probably have stuff in there that you're going to want moved over to Canvas. We can walk you through the process of easily migrating your content into Canvas.";

//----- CHANGE #2 -----
//---------------------------------
// REMOVE: "Add Students" item
ListItems.splice(2, 1);

//----- CHANGE #3 -----
//---------------------------------
// CHANGE: "Add Files" item
// Remove the text that says "We'll show you how." When you click the button to go to the file upload page,
// nothing actually happens so removing this text will avoid any confusion.
ListItems[2].text = ListItems[2].text.replace("We'll show you how.", "");

//---------------------------------
// CHANGE: "Select Navigation Links" item
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments are really helpful.

ListItems[3].url += "#tab-navigation";

//---------------------------------
// CHANGE: "Choose a Course Home Page" item
ListItems[4].text = ListItems[4].text.replace('The default is the course activity stream.', 'The default is the Syllabus Page with course description.');

//---------------------------------
// CHANGE: "Add TAs" item text and move up near the top of the list
var add_tas = ListItems.splice(6, 1)[0];
ListItems.splice(1, 0, add_tas);
$.each(['text', 'title'], function(idx, prop) {
add_tas[prop] = add_tas[prop].replace(/TA(s)?/g, "TF$1");
});
add_tas.url = BASE_COURSE_URL + "/external_tools/" + MANAGE_PEOPLE_TOOL_ID;


//----- CHANGE #4 -----
add_tas.url = BASE_COURSE_URL + "/external_tools/" + MANAGE_COURSE_TOOL_ID;

//---------------------------------
// INSERT: Academic Integrity Policy tool
ListItems.splice(7, 0, {
key:'policy_wizard',
Expand All @@ -148,10 +163,10 @@
url: BASE_COURSE_URL + "/external_tools/" + POLICY_WIZARD_TOOL_ID,
iconClass: 'icon-educators'
});


//----- DEBUG -----
if(DEBUG) {
if(false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is being left in there so the dev can tweak it and doesn't need to rewrite the logic each time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, left it in there so the dev can tweak it while working on it.

$.getJSON("/api/v1/accounts/39/external_tools").done(function(data) {
console.log("List of tools for account_id 39:");
$.each(data, function(idx, tool) {
Expand All @@ -162,4 +177,4 @@
}

}
})();
})();