Skip to content

Change course create form to asynchronous validation. - #643

Merged
peter-fogg merged 3 commits into
masterfrom
peter-fogg/course-creation-validation
Aug 13, 2013
Merged

Change course create form to asynchronous validation.#643
peter-fogg merged 3 commits into
masterfrom
peter-fogg/course-creation-validation

Conversation

@peter-fogg

Copy link
Copy Markdown
Contributor

@cahrens @talbs, review?

Comment thread cms/static/sass/elements/_forms.scss Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I understand why you removed this reference class, but is there a way we could keep the animation and de-couple it from the base .tip-error styling? My recommendation would be to have a class of is-showing (with the @extend .anim-fadeIn associated with it) and a class of is-hiding (@extend .anim-fadeOut associated similarly).

Would you mind adding the management of those two classes to your JS logic and trying it out to preserve the animation in the UI?

@talbs

talbs commented Aug 12, 2013

Copy link
Copy Markdown
Contributor

@peter-fogg, looks very good - nice logic and handling of all of the conditions. I did leave one in-line code comment about trying to keep the animating in/out of the error messages if possible. Let me know what you think.

Other than that, 👍

Comment thread cms/static/js/base.js Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Personally, I'd move all these validate functions into addNewCourse so they are local variables instead of globally defined functions (they are only used within the scope of addNewCourse).

@cahrens

cahrens commented Aug 12, 2013

Copy link
Copy Markdown

Found a bug--

  1. Create a new course with information identical to an existing course.
  2. On "Create" you get message that organization or course number must be unique.
  3. Change Course Number so it is unique
  4. Error does not go away on organization field, so you cannot click "Create" (without modifying the organization).

@cahrens

cahrens commented Aug 12, 2013

Copy link
Copy Markdown

Update https://edx-wiki.atlassian.net/wiki/display/STU/Course+Creation+BDD for any changes in behavior (make sure to capture the bug I found above).

@peter-fogg

Copy link
Copy Markdown
Contributor Author

@talbs Made that change; error messages fade in/out now without stuttering on every keystroke.

@cahrens I changed the feedback for a non-unique course just a bit, to make it consistent with the 80-character limit validation. Now the message shows up on top of the form, but individual fields aren't highlighted, and the error disappears once any edit is made.

@cahrens

cahrens commented Aug 12, 2013

Copy link
Copy Markdown

When I press Cancel and then "New Course" again (without refreshing the page), I am surprised to see that my previous field submissions (that I Canceled) are still there. I would expect the form to come up empty.

Related to this, there's some strange behavior when the thing I canceled conflicted with an existing course:

  1. Create a new course with information identical to an existing course.
  2. On "Create" you get message that organization or course number must be unique.
  3. Press Cancel
  4. Press "New Course"
  5. Change course number or organization -- message goes away
  6. Click "Create". The old error message shows (about conflicts with existing courses), although the new course is created.

In fact, I get the error message about a previous course conflicting even if the last error message I was viewing was about the total length being > 80 chars. Something funky is going on.

@talbs

talbs commented Aug 12, 2013

Copy link
Copy Markdown
Contributor

@peter-fogg, thanks - the animation aspect of things look good. I appreciate the extra spin around designville.

@cahrens

cahrens commented Aug 13, 2013

Copy link
Copy Markdown

👍 Make sure the BDD (manual) script contains a case for Cancel (fields empty when you press "New Course" after a Cancel).

peter-fogg pushed a commit that referenced this pull request Aug 13, 2013
Change course create form to asynchronous validation.
@peter-fogg
peter-fogg merged commit 9ea2be5 into master Aug 13, 2013
@peter-fogg
peter-fogg deleted the peter-fogg/course-creation-validation branch August 13, 2013 15:12
itsjeyd referenced this pull request in open-craft/openedx-platform Mar 4, 2016
dfrojas pushed a commit to eduNEXT/edx-platform that referenced this pull request Aug 31, 2017
…namese-translations

Update Vietnamese static js i18n file
rediris pushed a commit to gymnasium/edx-platform that referenced this pull request Feb 25, 2021
Sujeet1379 pushed a commit to chandrudev/edx-platform that referenced this pull request Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants