Fix some rendering-related problems with ListView and ListItemView.

Review Request #5718 — Created April 16, 2014 and submitted

Information

Djblets
master
fda56b7...

Reviewers

ListView was attaching to collection events in initialize and rendering
as a result. This meant that they could render before render() was
called, causing breakages. Now, these events are attached in render()
instead.

ListItemView had some whitespace before the default displayed text,
which isn't ideal. This changes the template to not indent.

Unit tests in RBC that were failing before now pass.

Description From Last Updated

render() is allowed to be called multiple times without side effects. I think you should wrap this with a guard …

daviddavid

There are an awful lot of blank lines for such a simple function.

daviddavid

Is it really worth having a separate method for this? We only call it from render().

daviddavid
david
  1. 
      
  2. Show all issues

    render() is allowed to be called multiple times without side effects. I think you should wrap this with a guard of some sort.

    1. Most of our render() calls definitely do not allow this and will break horribly. We haven't been following that rule very well. I'll figure out something for this case, though.

  3. 
      
chipx86
david
  1. 
      
  2. djblets/static/djblets/js/configForms/views/listView.js (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
    Show all issues

    There are an awful lot of blank lines for such a simple function.

  3. djblets/static/djblets/js/configForms/views/listView.js (Diff revision 2)
     
     
     
     
     
     
     
     
     
    Show all issues

    Is it really worth having a separate method for this? We only call it from render().

    1. We call it from render() and also in the listenTo(..., 'reset', ...)

  4. 
      
chipx86
david
  1. Ship It!

  2. 
      
chipx86
Review request changed
Status:
Completed