Skip to content

Conversation

jasonkhoe
Copy link
Contributor

Followed advice from @seshness

@seshness
Copy link
Contributor

🉑 👍 🌞

Copy link
Member

Choose a reason for hiding this comment

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

Nitpicks about the indentation and lane breaks here:

  1. success should be lined up with url.
  2. Why is autocomplete on a separate line? It fits within 80 characters with the previous line.
  3. result should be indented more.
  4. Having three nested anonymous functions in the success handler is pretty confusing to read. I would create a new named function and just reference it here.
  5. There is inconsistent spacing around function arguments. I would personally prefer no spaces before or after parentheses and a space after every comma.

@richardxia
Copy link
Member

I realize you weren't responsible for most of the things I pointed out, but I figured we might as well clean it up while we're at it. Everything else LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah...maybe this wasn't the best way of curing the problem. We might as well jQuery all the way.
Instead of using the ID selectors, a class applied to the inputs and the hidden fields might be a better idea. Say the fields currently id'd as iboxN had the class instructor-name. You could then write:

$('.instructor-name').each(function() {
  // ... your code here, where this refers to each of the elements with the class 'instructor-name'
});

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