Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

We should use require.js for this code. One of the nasty consequences of not having done so is that the directory structure and file naming are crafted so that the files are loaded according to dependency order. This results in files that are not in the right place, oddly named classes and files, and multiple classes in one file. As we implement require.js, we should be sure to fix all of these problems.

Remove globals

Several bits of state are stored in global scope, including the list of users with privileged roles, the current user, and the course id. These should all be passed as appropriate to the objects and functions that need them.

Markdown and MathJax

These are consistent pain points for their complexity. In testing, we frequently turn functions related to these features into spies to avoid invoking them. We should figure out ways to isolate them and simplify the code that invokes them.

Use <button> instead of <a>

In many views and templates, we have special code to ensure that our things that look like buttons also act like buttons (i.e. use the button role and activate upon pressing the spacebar). We should really just use button elements, but we must ensure that we make any necessary CSS changes to avoid changing their appearance.

Better use of the Backbone framework

...

http://stackoverflow.com/questions/16823746/backbone-js-listento-vs-on

Let views create their own elements

Most of the time, we pass an existing element in the DOM as the el option when constructing a view. It is more efficient and concise to define tagName and className on the view and add its element to the page when rendering has completed (because the browser can do a single update).

Specific files/classes (paths relative to common/static/coffee/src/discussion)

DiscussionFilter (discussion_filter.coffee)

Duplicates functionality in DiscussionThreadListView; the two should share code to the extent possible.

DiscussionRouter (discussion_router.coffee)

This class currently acts as a main entry point for the discussion tab app. We should separate that behavior into another class and make DiscussionRouter actually just a Backbone Router. Also, we should implement more routes than the two currently available, which are the discussion home and the single-thread view. At the very least, we should have a route for a specific topic.