Versions Compared

Key

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

...

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

...

(paths relative to common/static/coffee/src/discussion)

...

discussion_filter.coffee

...

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

...

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.

views/discussion_thread_list_view.coffee

This class currently does way too much. It manages all of the elements in the sidebar itself. This functionality should be broken out into more specific views.

The goHome function is only invoked by DiscussionRouter. It was originally implemented on DiscussionThreadListView because the sidebar used to have a home button that would return to the discussion home screen, but that functionality should really have resided all along in whatever view contains DiscussionThreadListView.

The updateSidebar function ensures that the sidebar appears to stay fixed relative to the viewport (but within the confines of the header/footer). This has historically been error prone, and it is computationally relatively expensive for something that is executed every time the user scrolls. Along with the fact that some users have specifically expressed displeasure with the feature, we should consider simply removing it.

views/discussion_thread_profile_view.coffee

This view should be consolidated with the other thread views. It used to be the case that the user profile page acted similarly to an inline discussion (i.e. each thread was presented in an abbreviated form and could be expanded and viewed inline). I do not know why this functionality was removed, but given the recent refactoring of the thread views, it could probably be reinstated without much difficulty.

views/discussion_thread_view.coffee

I think this view should be broken into four parts:

  1. A full thread view (i.e. most of what it does now)
  2. An abbreviated thread view
  3. A container view that would switch between the abbreviated and full views when the thread is expanded/collapsed. This view would only be used for inline discussions (and maybe user profiles at some point)
  4. A new response view

This would avoid complexity around re-rendering the body content (which has caused bugs around failing to render markup and XSS attacks) and hiding specific pieces of the full view (look through the templates for the post-extended-content class).

views/thread_response_view.coffee

The new comment view should be split off from this view.