You only need & if you want to specialize the parent selector e.g.:

.foo {
    &.bar {
        // Equivalent to .foo.bar
    }
}


So you can just do > h1, > p { ....

You only need _.defaults when combining objects. This can just be:

defaults: {
    hasDraft: false,
},

Nit: space between if and (

We only need the [].join('') in ES5 because it does not have multiline strings. You can just do:
javascript:
template: _.template(dedent`
    <h1><%- title %></h1>
`),

ES6 lets us do render(). Same below.

This is going to run when the file is sourced, and so this object might not exist? We may want to store the default selector rather than the actual result of querying the selector. It also allows us to be lazy and only perform that query when necessary.

this is undefined here. The second object passed to Backbone.{...}.extend is applied directly to the created object instead of its prototype.
You will have to do:

if (!RB.UnifiedReviewBannerView._instance) {
    // ...
}

Hey Sarah, Great work so far! 

I just wanted to clarify, I know that var is bound to the execution context, in this case the function. Wouldn't let be more appropriate?
let banner,
    pendingReview,
    reviewRequest;

Something similar to this^?

Same thing as above, do these need to be declared with var?

I'm only asking cause I thought we should avoid var entirely.

I think this description is a bit of a mouthful. 
Would something like this be a more sufficient summary?

"A unified, multi-mode banner that provides basic support for publishing/editing/discarding, batch operations and more."

You can use an arrow function here in order to avoid assigning const that = this;
Since arrow functions don't have their own value of this, the _.each function can be rewritten as:

_.each(defaults, item => {
  this.register(item);
});

Hey Sarah! Is this TODO for you to work on in your project, or is this for somebody in the future to work on?

Hey Sarah! Is this TODO for you to work on in your project, or is this for somebody in the future to work on?

This comment should be a full sentence, with a period.

This comment should be a full sentence, with a period.

This comment should be a full sentence, with a period.

Can you add screenshots?

This should be // TODO: Full sentence..
Though I don't know that this is necessary.

Same here.

This should probably be #unified-banner-docked.
Same for other selectors: they should spell out the entire phrase.

Should be organized like:

.selector {
    property: value;

    &-selectors {}

    .other-selectors {}
}

.selector * is super expensive.
CSS selectors are applied right-to-left, so the * will match every DOM element and then the browser will check .selector against every parent.

.selector * is super expensive. See other comment.

This needs a corresponding afterEach() to reset the feature to its original value.

Doc comments should be of the form:

/**
 * Single line summary.
 *
 * Multi-line description.
 */

Since reviewReplyDrafts is an Array, which is mutable, this needs to be a function:

defaults() {
    return {
        hasDraft: false,
        // ...
    };
},

Missing doc comment.

Why these? These are in the defaults.

The arrow function should be aligned with the first argument.
The arrow functino should be () => this._updateHasDraft(). Making it multiline is overkill.

Again, this should just be hasDraft.
Also, we can rewrite this using Backbone.Model.prototype.isNew().

const hasDraft = (!this.get('reviewRequest').isNew() ||
                 !this.get('pendingReview').isNew() ||
                 !!this.get('reviewReplyDrafts').length)

We are declaring a variable and then immediately using it once. We should move it inline.

Doc comments should be of the format:

/**
 * Single line summary.
 *
 * Multi-line description.
 */

These arrow functions can be a single line.

Missing a summary.

We don't do local vars with leading underscores.

You are re-using this in closures. Is this intentional?

This should be formatted as either:

this.listenTo(reviewReply, 'saved',
              () => {
                  // ...
              });

// or
this.listenTo(
    reviewReply, 'saved',
    () => {
        // ...
    });


(Probably the latter to avoid rightward drift.)

Blank line between these.

Instead of _.findWhere, why not:


if (!_reviewReplyDrafts.includes(reviewReply))


_.findWhere compares all attributes instead of just comparing for equality. Two models with the same ID will have the property that a == b.

This can be rewritten as:

this.listenTo(
    reviewReply, 'destroying',
    () => this.set('reviewReplyDrafts', _.reject(
        this.get('reviewReplyDrafts'),
        draft => draft.id === reviewReply.id)));

Any reason you're not using arrow syntax?

This should be formatted as:

this.listenTo(reviewReply, 'destroyed',
              () => this._updateHasDraft());


The { and } are unnecessary for a single-line lambda.

Blank line between these.

Same here re: findWhere

Doc comments should be of the format:

/**
 * Single line summary.
 *
 * Multi-line description.
 */

Blank line between these.

Undo this formatting change.

There should be a corresponding afterEach() that resets RB.EnabledFeatures.unifiedBanner.

There should be a corresponding afterEach() that resets RB.EnabledFeatures.unifiedBanner.

This is unnecessary.

This should be a full sentence.

I have a bunch of comments, but they're almost all very simple formatting issues. Great work!

"Attributes" should be capitalized here.

Since this is implemented as a method, it should have a doc comment.

This can be cleaned up a bit:

_.bindAll(this, '_updateHasDraft');
this.listenTo(reviewRequest.draft, 'saved destroy',
              this._updateHasDraft);
this.listenTo(pendingreview, 'saved destroy',
              this._updateHasDraft);

Instead of !!, can we just compare > 0?

If you do the bindAll in initialize, you can just pass in ready: this._updateHasDraft, here.

I think it would be better to have multiple const reviewReplyDrafts = ... in each place that it's used. Right now you share this same variable between the function and two inner closures, which seems ripe for strange bugs.

Assuming reviewReplies is an array, we can probably just use !reviewReplies.includes(reviewReply) (I haven't tested that, though).

There's a better way to wrap this:

this.listenTo(reviewReply, 'saved', () => {
    reviewReplyDrafts = this.get('reviewReplyDrafts');
    ...
});


(saves one level of indentation and a couple lines)

Same wrapping here as above.

Leftover debug output?

If you have the bindAll in initialize mentioned before, this can just pass in _updateHasDraft directly.

Probably better to use 

this.get('reviewReplyDrafts').forEach(reviewReply => {
   ...
});



We're slowly replacing a lot of underscore use with modern JS features like that.

You should be able to skip the truthy check here. isFunction will return false if it's undefined/null/etc.

Please put a blank line between these two lines.

This is nested very deep. Can we break this stuff out into its own function, and then maybe define the registries in their own variable first before passing them into create()? Just to flatten the code a little bit.

We should probably save the value inside beforeEach instead of here when the suite is defined.

We should probably save the value inside beforeEach instead of here when the suite is defined.

Only one blank line here.

We should probably save the value inside beforeEach instead of here when the suite is defined.

We should probably save the value inside beforeEach instead of here when the suite is defined.

We should probably save the value inside beforeEach instead of here when the suite is defined.

This is a regular JS file for now, so the last item in an object shouldn't have a trailing comma.

Add this line back please.

Add a blank line between these two.

This can just be:

this.listenTo(view, 'fieldSaved', this.showBanner);

Add a blank line between these two.

Better to use listenTo:

this.listenTo(this.model, 'saved', this.showBanner);

Alignment is wacky here. Should look like:
```javascript

} else if (state === RB.ReviewRequest.PENDING &&

           this.model.get('hasDraft')) {

    BannerClass = DraftBannerView;

}

Should probably save the value inside beforeEach

Should save the value inside beforeEach

Unless we change this, it should be defined as const.
This is also a great place to use dedent.

Can you include an "Option Args" section here, explaining what can be in options?

Should have a "Returns" section.

We're moving away from _super because it's not reliable. Can you just use RB.FloatingBannerView.prototype.render.call?

Formatting of this comment isn't correct. It should be one line summary, blank line, longer description, and then include an "Args" section explaining isDraft.

Should have a one-line summary, then a blank line, then longer description.

Please include an "Option Args" section.

Dedent this 4 spaces.

Can we line everything up after the (?

Second line here should line up with the other !options in the conditional.

Dedent this 4 spaces.

"Attributes" should be capitalized.

Type can be Backbone.View or string

Add a blank line between these.

Should just be jQuery for the type.

Please include a doc comment here.

If this is a backbone view (having el), it will also have a $el.

"Attributes" should be capitalized.

Indentation in these should be 4 spaces instead of 2.

Needs a "Returns" section.

This can use an ES6 template string:

console.error(`Index (${index}) is out of range.`);

Should have a summary/description format. You can probably just add a blank line after "Register an item."

Indentation in these should be 4 spaces instead of 2.

item isn't used outside of the loop, so I'd rather do:

while (registryCollection.length > 0) {
    const item = registryCollection.pop();
    item.get('$view').remove();
}

Probably better to use forEach instead of _.each.

Description should be on the next line after the type.

Indentation should be 4 spaces, not 2.

Should use summary/description format.

Please add a doc comment here.