chipx86 got a fish trophy!

Summary

Move the review banner code out into a DraftReviewBannerView.

Review Request #4114 — Created May 8, 2013 and submitted May 13, 2013, 12:42 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Move the review banner code out into a DraftReviewBannerView.

DraftReviewBannerView handles showing/hiding the banner in response to
events on the draft review, and handles the buttons on the banner.

DraftReviewBannerView is a singleton. It's created once up-front.
Callers can access the instance through
RB.DraftReviewBannerView.instance in order to show or hide the banner.

Testing Done

Manually tested all buttons, and various events that show the banner.

Unit tests pass.

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/settings.py
  Ignored Files:
    reviewboard/static/rb/js/views/abstractCommentBlockView.js
    reviewboard/templates/js/tests.html
    reviewboard/static/rb/js/views/draftReviewBannerView.js
    reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.js
    reviewboard/static/rb/js/views/reviewRequestEditorView.js
    reviewboard/static/rb/js/diffviewer.js
    reviewboard/static/rb/js/reviews.js

reviewboard/static/rb/js/views/draftReviewBannerView.js (Diff revision 1)

So the more I encounter this sort of thing, the more I think it should be:

this.$el
    .slideDown();
this.$el.find('.banner')
    .hide()
    .slideDown();

chipx86 May 12, 2013, 1:22 a.m.

I don't know. Either works. It's designed for this kind of thing though. If you need to go back a step in the chain, you just use end().

david May 13, 2013, 12:11 a.m.

I guess keep it as-is, but I may decide later on that I prefer the other one enough to raise a stink.

chipx86 May 13, 2013, 12:32 a.m.

I'll go ahead and change it and submit, if you're otherwise happy with the change.

david May 13, 2013, 12:33 a.m.
```
Yep, ship it.
```

Status:: Completed