From 8fa4b4dbd1e0ce95845f3464ca5684e3ed53e88f Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Tue, 6 Aug 2013 16:59:15 -0400 Subject: [PATCH 1/3] Change course create form to synchronous validation. --- .../contentstore/features/courses.feature | 2 +- cms/static/js/base.js | 157 +++++++++++------- cms/static/sass/elements/_forms.scss | 1 - cms/templates/index.html | 2 +- 4 files changed, 102 insertions(+), 60 deletions(-) diff --git a/cms/djangoapps/contentstore/features/courses.feature b/cms/djangoapps/contentstore/features/courses.feature index 455313b0e2..a0ba8099ac 100644 --- a/cms/djangoapps/contentstore/features/courses.feature +++ b/cms/djangoapps/contentstore/features/courses.feature @@ -8,6 +8,6 @@ Feature: Create Course And I am logged into Studio When I click the New Course button And I fill in the new course information - And I press the "Save" button + And I press the "Create" button Then the Courseware page has loaded in Studio And I see a link for adding a new section diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 80b24776da..72ef5991a3 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -605,80 +605,118 @@ function cancelNewSection(e) { function addNewCourse(e) { e.preventDefault(); $('.new-course-button').addClass('is-disabled'); + $('.new-course-save').addClass('is-disabled'); var $newCourse = $('.wrapper-create-course').addClass('is-shown'); var $cancelButton = $newCourse.find('.new-course-cancel'); - $newCourse.find('.new-course-name').focus().select(); - $newCourse.find('form').bind('submit', saveNewCourse); + var $courseName = $('.new-course-name'); + $courseName.focus().select(); + $('.new-course-save').on('click', saveNewCourse); $cancelButton.bind('click', cancelNewCourse); $body.bind('keyup', { $cancelButton: $cancelButton }, checkForCancel); + // Handle validation asynchronously + _.each( + ['.new-course-org', '.new-course-number', '.new-course-run'], + function(ele) { + var $ele = $(ele); + $ele.on('keyup', function(event) { + // Don't bother showing "required field" error when + // the user tabs into a new field; this is distracting + // and unnecessary + if(event.keyCode === 9) { + return; + } + var error = validateCourseItemEncoding($ele.val()); + setNewCourseFieldInErr($ele.parent('li'), error); + validateTotalCourseItemsLength(); + }); + } + ); + var $name = $('.new-course-name'); + $name.on('keyup', function() { + var error = validateCourseName($name.val()); + setNewCourseFieldInErr($name.parent('li'), error); + validateTotalCourseItemsLength(); + }); +} + +function setNewCourseFieldInErr(el, msg) { + el.children('.tip-error').remove(); + if(msg) { + el.addClass('error'); + el.append('' + msg + ''); + $('.new-course-save').addClass('is-disabled'); + } + else { + el.removeClass('error'); + // One "error" div is always present, but hidden or shown + if($('.error').length === 1) { + $('.new-course-save').removeClass('is-disabled'); + } + } +}; + +function validateCourseName(name) { + if(name.length === 0) { + return gettext('Required field.'); + } + return ''; +} + +// Check that a course (org, number, run) doesn't use any special characters +function validateCourseItemEncoding(item) { + if(item === '') { + return gettext('Required field.'); + } + if(item !== encodeURIComponent(item)) { + return gettext('Please do not use any spaces or special characters in this field.'); + } + return ''; +} + +// Ensure that all items are less than 80 characters. +function validateTotalCourseItemsLength() { + var totalLength = _.reduce( + ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], + function(sum, ele) { + return sum + $(ele).val().length; + }, 0 + ); + if(totalLength > 80) { + $('.wrap-error').addClass('is-shown'); + $('#course_creation_error').html('

' + gettext('Course fields must have a combined length of no more than 80 characters.') + '

'); + $('.new-course-save').addClass('is-disabled'); + } + else { + $('.wrap-error').removeClass('is-shown'); + } } function saveNewCourse(e) { e.preventDefault(); + // One final check for empty values + var errors = _.reduce( + ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], + function(acc, ele) { + var $ele = $(ele); + var error = $ele.val().length === 0 ? gettext('Required field.') : ''; + setNewCourseFieldInErr($ele.parent('li'), error); + return error ? true : acc; + }, + false + ); + var $newCourseForm = $(this).closest('#create-course-form'); var display_name = $newCourseForm.find('.new-course-name').val(); var org = $newCourseForm.find('.new-course-org').val(); var number = $newCourseForm.find('.new-course-number').val(); var run = $newCourseForm.find('.new-course-run').val(); - var required_field_text = gettext('Required field'); - - var display_name_errMsg = (display_name === '') ? required_field_text : null; - var org_errMsg = (org === '') ? required_field_text : null; - var number_errMsg = (number === '') ? required_field_text : null; - var run_errMsg = (run === '') ? required_field_text : null; - - var bInErr = (display_name_errMsg || org_errMsg || number_errMsg || run_errMsg); - - // check for suitable encoding - if (!bInErr) { - var encoding_errMsg = gettext('Please do not use any spaces or special characters in this field.'); - - if (encodeURIComponent(org) != org) - org_errMsg = encoding_errMsg; - if (encodeURIComponent(number) != number) - number_errMsg = encoding_errMsg; - if (encodeURIComponent(run) != run) - run_errMsg = encoding_errMsg; - - bInErr = (org_errMsg || number_errMsg || run_errMsg); - } - - var header_err_msg = (bInErr) ? gettext('Please correct the fields below.') : null; - - var setNewCourseErrMsgs = function(header_err_msg, display_name_errMsg, org_errMsg, number_errMsg, run_errMsg) { - if (header_err_msg) { - $('.wrapper-create-course').addClass('has-errors'); - $('.wrap-error').addClass('is-shown'); - $('#course_creation_error').html('

' + header_err_msg + '

'); - } else { - $('.wrap-error').removeClass('is-shown'); - $('#course_creation_error').html(''); - } - - var setNewCourseFieldInErr = function(el, msg) { - el.children('.tip-error').remove(); - if (msg !== null && msg !== '') { - el.addClass('error'); - el.append('' + msg + ''); - } else { - el.removeClass('error'); - } - }; - - setNewCourseFieldInErr($('#field-course-name'), display_name_errMsg); - setNewCourseFieldInErr($('#field-organization'), org_errMsg); - setNewCourseFieldInErr($('#field-course-number'), number_errMsg); - setNewCourseFieldInErr($('#field-course-run'), run_errMsg); - }; - - setNewCourseErrMsgs(header_err_msg, display_name_errMsg, org_errMsg, number_errMsg, run_errMsg); - - if (bInErr) + if(errors) { return; + } analytics.track('Created a Course', { 'org': org, @@ -698,8 +736,13 @@ function saveNewCourse(e) { window.location = '/' + data.id.replace(/.*:\/\//, ''); } else if (data.ErrMsg !== undefined) { var orgErrMsg = (data.OrgErrMsg !== undefined) ? data.OrgErrMsg : null; + if(orgErrMsg) { + setNewCourseFieldInErr($('.new-course-org').parent('li'), orgErrMsg); + } var courseErrMsg = (data.CourseErrMsg !== undefined) ? data.CourseErrMsg : null; - setNewCourseErrMsgs(data.ErrMsg, null, orgErrMsg, courseErrMsg, null); + if(courseErrMsg) { + setNewCourseFieldInErr($('.new-course-number').parent('li'), orgErrMsg); + } } } ); diff --git a/cms/static/sass/elements/_forms.scss b/cms/static/sass/elements/_forms.scss index c78e2f3692..0d2dec68a0 100644 --- a/cms/static/sass/elements/_forms.scss +++ b/cms/static/sass/elements/_forms.scss @@ -226,7 +226,6 @@ form[class^="create-"] { } .tip-error { - @extend .anim-fadeIn; display: block; color: $red; } diff --git a/cms/templates/index.html b/cms/templates/index.html index 9702eedada..5a88054563 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -123,7 +123,7 @@
- +
From c611470e97edab910ae57eee24acde0070ff460d Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Mon, 12 Aug 2013 16:28:04 -0400 Subject: [PATCH 2/3] Correct non-unique course validation; code cleanup; better error style. --- cms/static/js/base.js | 98 +++++++++++++--------------- cms/static/sass/elements/_forms.scss | 8 +++ cms/templates/index.html | 4 ++ 3 files changed, 58 insertions(+), 52 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 72ef5991a3..b68b82dfa6 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -615,6 +615,37 @@ function addNewCourse(e) { $body.bind('keyup', { $cancelButton: $cancelButton }, checkForCancel); + + // Check that a course (org, number, run) doesn't use any special characters + var validateCourseItemEncoding = function(item) { + var required = validateRequiredField(item); + if(required) { + return required; + } + if(item !== encodeURIComponent(item)) { + return gettext('Please do not use any spaces or special characters in this field.'); + } + return ''; + } + + // Ensure that all items are less than 80 characters. + var validateTotalCourseItemsLength = function() { + var totalLength = _.reduce( + ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], + function(sum, ele) { + return sum + $(ele).val().length; + }, 0 + ); + if(totalLength > 80) { + $('.wrap-error').addClass('is-shown'); + $('#course_creation_error').html('

' + gettext('Course fields must have a combined length of no more than 80 characters.') + '

'); + $('.new-course-save').addClass('is-disabled'); + } + else { + $('.wrap-error').removeClass('is-shown'); + } + } + // Handle validation asynchronously _.each( ['.new-course-org', '.new-course-number', '.new-course-run'], @@ -635,21 +666,25 @@ function addNewCourse(e) { ); var $name = $('.new-course-name'); $name.on('keyup', function() { - var error = validateCourseName($name.val()); + var error = validateRequiredField($name.val()); setNewCourseFieldInErr($name.parent('li'), error); validateTotalCourseItemsLength(); }); } +function validateRequiredField(msg) { + return msg.length === 0 ? gettext('Required field.') : ''; +} + function setNewCourseFieldInErr(el, msg) { - el.children('.tip-error').remove(); if(msg) { el.addClass('error'); - el.append('' + msg + ''); + el.children('span.tip-error').addClass('is-showing').removeClass('is-hiding').text(msg); $('.new-course-save').addClass('is-disabled'); } else { el.removeClass('error'); + el.children('span.tip-error').addClass('is-hiding').removeClass('is-showing'); // One "error" div is always present, but hidden or shown if($('.error').length === 1) { $('.new-course-save').removeClass('is-disabled'); @@ -657,42 +692,6 @@ function setNewCourseFieldInErr(el, msg) { } }; -function validateCourseName(name) { - if(name.length === 0) { - return gettext('Required field.'); - } - return ''; -} - -// Check that a course (org, number, run) doesn't use any special characters -function validateCourseItemEncoding(item) { - if(item === '') { - return gettext('Required field.'); - } - if(item !== encodeURIComponent(item)) { - return gettext('Please do not use any spaces or special characters in this field.'); - } - return ''; -} - -// Ensure that all items are less than 80 characters. -function validateTotalCourseItemsLength() { - var totalLength = _.reduce( - ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], - function(sum, ele) { - return sum + $(ele).val().length; - }, 0 - ); - if(totalLength > 80) { - $('.wrap-error').addClass('is-shown'); - $('#course_creation_error').html('

' + gettext('Course fields must have a combined length of no more than 80 characters.') + '

'); - $('.new-course-save').addClass('is-disabled'); - } - else { - $('.wrap-error').removeClass('is-shown'); - } -} - function saveNewCourse(e) { e.preventDefault(); @@ -701,23 +700,23 @@ function saveNewCourse(e) { ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], function(acc, ele) { var $ele = $(ele); - var error = $ele.val().length === 0 ? gettext('Required field.') : ''; + var error = validateRequiredField($ele.val()); setNewCourseFieldInErr($ele.parent('li'), error); return error ? true : acc; }, false ); + if(errors) { + return; + } + var $newCourseForm = $(this).closest('#create-course-form'); var display_name = $newCourseForm.find('.new-course-name').val(); var org = $newCourseForm.find('.new-course-org').val(); var number = $newCourseForm.find('.new-course-number').val(); var run = $newCourseForm.find('.new-course-run').val(); - if(errors) { - return; - } - analytics.track('Created a Course', { 'org': org, 'number': number, @@ -735,14 +734,9 @@ function saveNewCourse(e) { if (data.id !== undefined) { window.location = '/' + data.id.replace(/.*:\/\//, ''); } else if (data.ErrMsg !== undefined) { - var orgErrMsg = (data.OrgErrMsg !== undefined) ? data.OrgErrMsg : null; - if(orgErrMsg) { - setNewCourseFieldInErr($('.new-course-org').parent('li'), orgErrMsg); - } - var courseErrMsg = (data.CourseErrMsg !== undefined) ? data.CourseErrMsg : null; - if(courseErrMsg) { - setNewCourseFieldInErr($('.new-course-number').parent('li'), orgErrMsg); - } + $('.wrap-error').addClass('is-shown'); + $('#course_creation_error').html('

' + data.ErrMsg + '

'); + $('.new-course-save').addClass('is-disabled'); } } ); diff --git a/cms/static/sass/elements/_forms.scss b/cms/static/sass/elements/_forms.scss index 0d2dec68a0..69bf3a6566 100644 --- a/cms/static/sass/elements/_forms.scss +++ b/cms/static/sass/elements/_forms.scss @@ -225,6 +225,14 @@ form[class^="create-"] { color: $red; } + .is-showing { + @extend .anim-fadeIn; + } + + .is-hiding { + @extend .anim-fadeOut; + } + .tip-error { display: block; color: $red; diff --git a/cms/templates/index.html b/cms/templates/index.html index 5a88054563..e5cf2fa54a 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -99,23 +99,27 @@ ${_("The public display name for your course.")} +
  • ${_("The name of the organization sponsoring the course")} - ${_("Note: No spaces or special characters are allowed. This cannot be changed.")} +
  • ${_("The unique number that identifies your course within your organization")} - ${_("Note: No spaces or special characters are allowed. This cannot be changed.")} +
  • ${_("The term in which your course will run")} - ${_("Note: No spaces or special characters are allowed. This cannot be changed.")} +
    • From b3aa20db64d659ad2fcf264a3eee42f2bc90f0ce Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Mon, 12 Aug 2013 17:18:29 -0400 Subject: [PATCH 3/3] Correctly clear fields when cancelling course create. --- cms/static/js/base.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index b68b82dfa6..9ccac6f602 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -746,6 +746,16 @@ function cancelNewCourse(e) { e.preventDefault(); $('.new-course-button').removeClass('is-disabled'); $('.wrapper-create-course').removeClass('is-shown'); + // Clear out existing fields and errors + _.each( + ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], + function(field) { + $(field).val(''); + } + ); + $('#course_creation_error').html(''); + $('.wrap-error').removeClass('is-shown'); + $('.new-course-save').off('click'); } function addNewSubsection(e) {