diff --git a/reviewboard/static/rb/js/reviews/views/abstractCommentBlockView.ts b/reviewboard/static/rb/js/reviews/views/abstractCommentBlockView.ts index 136a0730be8a1410b3c3c782ef32726dbc46815d..0f0cda8e7b1e693f558fa90c1015b11f4d3a0201 100644 --- a/reviewboard/static/rb/js/reviews/views/abstractCommentBlockView.ts +++ b/reviewboard/static/rb/js/reviews/views/abstractCommentBlockView.ts @@ -258,10 +258,6 @@ export class AbstractCommentBlockView< if (!options.boundsUpdated) { this.notify(gettext('Comment Saved')); } - - if (!EnabledFeatures.unifiedBanner) { - RB.DraftReviewBannerView.instance.show(); - } }); this.$el.addClass('draft'); diff --git a/reviewboard/static/rb/js/reviews/views/diffViewerPageView.ts b/reviewboard/static/rb/js/reviews/views/diffViewerPageView.ts index 3b0917bcb5faedfdcb3a6c783e8a6834fe9a915f..7f2577d21eb53c9cf6f5e5ef8153398c58f34a6a 100644 --- a/reviewboard/static/rb/js/reviews/views/diffViewerPageView.ts +++ b/reviewboard/static/rb/js/reviews/views/diffViewerPageView.ts @@ -768,10 +768,6 @@ export class DiffViewerPageView extends ReviewablePageView< anchorOffset - DiffViewerPageView.DIFF_SCROLLDOWN_AMOUNT); scrollAmount = this.#computeScrollHeight( anchorOffset - newOffset); - - } else if (RB.DraftReviewBannerView.instance) { - scrollAmount = (DiffViewerPageView.DIFF_SCROLLDOWN_AMOUNT + - RB.DraftReviewBannerView.instance.getHeight()); } this.#$window.scrollTop(anchorOffset - scrollAmount); diff --git a/reviewboard/static/rb/js/reviews/views/reviewDialogView.ts b/reviewboard/static/rb/js/reviews/views/reviewDialogView.ts index dc93ef10e76244a22c131e8ce37fdaad094e8f7d..add9a4df6a20fda0d4bdb96dc20888f22963f667 100644 --- a/reviewboard/static/rb/js/reviews/views/reviewDialogView.ts +++ b/reviewboard/static/rb/js/reviews/views/reviewDialogView.ts @@ -1671,10 +1671,6 @@ export class ReviewDialogView extends BaseView< await this.model.destroy(); ClientCommChannel.getInstance().reload(); - - if (!EnabledFeatures.unifiedBanner) { - RB.DraftReviewBannerView.instance.hideAndReload(); - } }}> ${_`Discard`} @@ -1775,24 +1771,10 @@ export class ReviewDialogView extends BaseView< this.close(); - if (EnabledFeatures.unifiedBanner) { - if (publish) { - // Reload the page. - RB.navigateTo( - this.model.get('parentObject').get('reviewURL')); - } - } else { - const reviewBanner = RB.DraftReviewBannerView.instance; - - if (reviewBanner) { - if (publish) { - reviewBanner.hideAndReload(); - } else if (this.model.isNew() && !madeChanges) { - reviewBanner.hide(); - } else { - reviewBanner.show(); - } - } + if (publish) { + // Reload the page. + RB.navigateTo( + this.model.get('parentObject').get('reviewURL')); } } catch (err) { console.error('Failed to save review', err); diff --git a/reviewboard/static/rb/js/reviews/views/reviewRequestEditorView.ts b/reviewboard/static/rb/js/reviews/views/reviewRequestEditorView.ts index 0895374dbe040a3b9ec917dfdfd253aef7bab5ce..064cda7e3e3efde6ea8c7316a232f8f2d95e3e27 100644 --- a/reviewboard/static/rb/js/reviews/views/reviewRequestEditorView.ts +++ b/reviewboard/static/rb/js/reviews/views/reviewRequestEditorView.ts @@ -900,11 +900,6 @@ export class ReviewRequestEditorView extends BaseView { view.on('beginEdit', () => this.model.incr('editCount')); view.on('endEdit', () => this.model.decr('editCount')); - - if (!EnabledFeatures.unifiedBanner) { - view.on('commentSaved', - () => RB.DraftReviewBannerView.instance.show()); - } } /** diff --git a/reviewboard/static/rb/js/reviews/views/reviewablePageView.ts b/reviewboard/static/rb/js/reviews/views/reviewablePageView.ts index 12dd4bfd1a00761da6ad39c91a883a6bc06f953e..7af514086f5ef0591dd9264c4c8381ef51fe324f 100644 --- a/reviewboard/static/rb/js/reviews/views/reviewablePageView.ts +++ b/reviewboard/static/rb/js/reviews/views/reviewablePageView.ts @@ -243,9 +243,6 @@ export class ReviewablePageView< /** The review request editor. */ reviewRequestEditorView: ReviewRequestEditorView; - /** The draft review banner, if present. */ - draftReviewBanner: RB.DraftReviewBannerView; - /** The unified banner, if present. */ unifiedBanner: UnifiedBannerView = null; @@ -331,28 +328,17 @@ export class ReviewablePageView< const pendingReview = this.model.get('pendingReview'); const reviewRequest = this.model.get('reviewRequest'); - if (EnabledFeatures.unifiedBanner) { - if (UserSession.instance.get('authenticated')) { - this.unifiedBanner = new UnifiedBannerView({ - el: $('#unified-banner'), - model: new UnifiedBanner({ - pendingReview: pendingReview, - reviewRequest: reviewRequest, - reviewRequestEditor: this.model.reviewRequestEditor, - }), - reviewRequestEditorView: this.reviewRequestEditorView, - }); - this.unifiedBanner.render(); - } - } else { - this.draftReviewBanner = RB.DraftReviewBannerView.create({ - el: $('#review-banner'), - model: pendingReview, - reviewRequestEditor: this.model.reviewRequestEditor, + if (UserSession.instance.get('authenticated')) { + this.unifiedBanner = new UnifiedBannerView({ + el: $('#unified-banner'), + model: new UnifiedBanner({ + pendingReview: pendingReview, + reviewRequest: reviewRequest, + reviewRequestEditor: this.model.reviewRequestEditor, + }), + reviewRequestEditorView: this.reviewRequestEditorView, }); - - this.listenTo(pendingReview, 'destroy published', - () => this.draftReviewBanner.hideAndReload()); + this.unifiedBanner.render(); } this.listenTo(this.model.reviewRequestEditor, @@ -370,10 +356,6 @@ export class ReviewablePageView< * This object, for chaining. */ remove(): this { - if (this.draftReviewBanner) { - this.draftReviewBanner.remove(); - } - if (this.unifiedBanner) { this.unifiedBanner.remove(); } @@ -554,11 +536,6 @@ export class ReviewablePageView< undefined, UserSession.instance.get('commentsOpenAnIssue')); - if (!EnabledFeatures.unifiedBanner) { - this.listenTo(comment, 'saved', - () => RB.DraftReviewBannerView.instance.show()); - } - RB.CommentDialogView.create({ comment: comment, reviewRequestEditor: this.model.reviewRequestEditor, diff --git a/reviewboard/static/rb/js/views/draftReviewBannerView.es6.js b/reviewboard/static/rb/js/views/draftReviewBannerView.es6.js deleted file mode 100644 index 52fb5f024b96583e927c4caf2afe9ead25e1bcee..0000000000000000000000000000000000000000 --- a/reviewboard/static/rb/js/views/draftReviewBannerView.es6.js +++ /dev/null @@ -1,277 +0,0 @@ -/** - * A banner that represents a pending draft review. - * - * The banner displays at the top of the page and provides buttons for - * editing the review, publishing, and discarding. - * - * The banner is a singleton. There's only ever one at a time. - */ -RB.DraftReviewBannerView = Backbone.View.extend({ - events: { - 'click #review-banner-discard': '_onDiscardClicked', - 'click #review-banner-edit': '_onEditReviewClicked', - }, - - /** - * Initialize the view. - * - * Args: - * options (object): - * Options for the view. - * - * Option Args: - * reviewRequestEditor (RB.ReviewRequestEditor): - * The review request editor. - */ - initialize(options) { - this.options = options; - }, - - /** - * Render the view. - * - * Returns: - * RB.DraftReviewBannerView: - * This object, for chaining. - */ - render() { - this._$buttons = this.$('input'); - this._$banner = this.$('.banner'); - - const model = this.model; - this.listenTo(model, 'saving destroying', - () => this._$buttons.prop('disabled', true)); - this.listenTo(model, 'saved destroyed', - () => this._$buttons.prop('disabled', false)); - this.listenTo(model, 'publishError', errorText => alert(errorText)); - - this._publishButton = new RB.MenuButtonView({ - ariaMenuLabel: gettext('More publishing options'), - el: this.$('#review-banner-publish-container'), - id: 'review-banner-publish', - menuItems: [ - { - id: 'review-banner-publish-submitter-only', - onClick: () => this._onPublishClicked({ - publishToOwnerOnly: true, - }), - text: _`... and only e-mail the owner`, - }, - { - id: 'review-banner-publish-and-archive', - onClick: () => this._onPublishClicked({ - publishAndArchive: true, - }), - text: _`... and archive the review request`, - }, - ], - onPrimaryButtonClick: () => this._onPublishClicked(), - text: gettext('Publish Review'), - }); - - this._publishButton.render(); - - if (!this.$el.prop('hidden')) { - this.show(); - } - - this.$el.addClass('ui-ready'); - - return this; - }, - - /* - * Show the banner. - * - * The banner will appear to slide down from the top of the page. - */ - show() { - const height = this._$banner.outerHeight(); - - RB.scrollManager.markForUpdate(this.$el); - - this.$el - .prop('hidden', false) - .removeClass('hidden') - .css({ - height: height, - maxHeight: height, - }); - RB.scrollManager.scrollYOffset += height; - RB.scrollManager.markUpdated(this.$el); - }, - - /* - * Hide the banner. - * - * The banner will slide up to the top of the page. - */ - hide() { - RB.scrollManager.markForUpdate(this.$el); - - const height = this._$banner.outerHeight(); - - this.$el - .prop('hidden', true) - .addClass('hidden') - .css('max-height', ''); - - /* - * If we set the height immediately, the browser will appear to not - * animate, since it can't transition heights (only max-heights). So - * we delay for a short period after we know the transition will have - * completed. - */ - _.delay( - () => { - this.$el.css('height', ''); - RB.scrollManager.markUpdated(this.$el); - RB.scrollManager.scrollYOffset -= height; - }, - 500); - }, - - /** - * Hide the banner and reloads the page. - * - * XXX Remove this function when we make the pages more dynamic. - */ - hideAndReload() { - this.hide(); - - /* - * hideAndReload might have been called from within a $.funcQueue. - * With Firefox, later async functions that are queued in the - * $.funcQueue will not run when we change window.location, which - * means that we might miss out on some teardown that was - * scheduled. We defer changing the location until the next tick - * of the event loop to let any teardown occur. - */ - _.defer(() => { - RB.navigateTo(this.model.get('parentObject').get('reviewURL')); - }); - }, - - /** - * Return the height of the banner. - * - * Returns: - * number: - * The height of the banner. - */ - getHeight() { - return this._$banner.outerHeight(); - }, - - /** - * Remove the banner from the page. - */ - remove() { - if (this._publishButton) { - this._publishButton.remove(); - } - - _super(this).remove.call(this); - }, - - /** - * Handler for the Edit Review button. - * - * Displays the review editor dialog. - * - * Returns: - * boolean: - * false, always. - */ - _onEditReviewClicked() { - RB.ReviewDialogView.create({ - review: this.model, - reviewRequestEditor: this.options.reviewRequestEditor, - }); - - return false; - }, - - /** - * Handler for the Publish button. - * - * Publishes the review. - * - * Args: - * options (object): - * Options that determine special cases for submission. - * - * Option Args: - * publishToOwnerOnly (boolean): - * Whether or not we should only notify the submitter of the - * review. - * - * publishAndArchive (boolean): - * Whether or not we should archive the review after it is - * published. - * - * Returns: - * boolean: - * false, always. - */ - _onPublishClicked(options={}) { - if (options.publishToOwnerOnly) { - this.model.set('publishToOwnerOnly', true); - } - - if (options.publishAndArchive) { - this.model.set('publishAndArchive', true); - } - - this.model.publish({ - attrs: ['public', 'publishToOwnerOnly', 'publishAndArchive'], - }); - - return false; - }, - - /** - * Handler for the Discard button. - * - * Prompts the user to confirm that they want the review discarded. - * If they confirm, the review will be discarded. - * - * Returns: - * boolean: - * false, always. - */ - _onDiscardClicked() { - $('

') - .text(_`If you discard this review, all related comments will be permanently deleted.`) - .modalBox({ - buttons: [ - $('') - .val(_`Cancel`), - $('') - .val(_`Discard`) - .click(() => this.model.destroy()), - ], - title: _`'Are you sure you want to discard this review?`, - }); - - return false; - }, -}, { - instance: null, - - /** - * Create the draft review banner singleton. - * - * Returns: - * RB.DraftReviewBannerView: - * The banner view. - */ - create(options) { - if (!this.instance) { - this.instance = new RB.DraftReviewBannerView(options); - this.instance.render(); - } - - return this.instance; - }, -}); diff --git a/reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.es6.js b/reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.es6.js deleted file mode 100644 index fcbe74b035a19d444ff16e8f0caa872124a35c4c..0000000000000000000000000000000000000000 --- a/reviewboard/static/rb/js/views/tests/draftReviewBannerViewTests.es6.js +++ /dev/null @@ -1,164 +0,0 @@ -suite('rb/views/DraftReviewBannerView', function() { - const template = dedent` -

' - `; - let model; - let view; - - beforeEach(function() { - const $banner = $(template).appendTo($testsScratch); - - model = new RB.DraftReview(); - view = new RB.DraftReviewBannerView({ - el: $banner, - model: model, - }); - - view.render(); - }); - - afterEach(function() { - view.remove(); - }); - - describe('Button states', function() { - let $buttons; - - beforeEach(function() { - $buttons = view.$('input'); - }); - - describe('Enabled', function() { - it('Default', function() { - expect($buttons.prop('disabled')).toBe(false); - }); - - it('When saved', function() { - $buttons.prop('disabled', true); - model.trigger('saved'); - expect($buttons.prop('disabled')).toBe(false); - }); - - it('When destroyed', function() { - $buttons.prop('disabled', true); - model.trigger('destroyed'); - expect($buttons.prop('disabled')).toBe(false); - }); - }); - - describe('Disabled', function() { - it('When saving', function() { - model.trigger('saving'); - expect($buttons.prop('disabled')).toBe(true); - }); - - it('When destroying', function() { - model.trigger('destroying'); - expect($buttons.prop('disabled')).toBe(true); - }); - }); - }); - - describe('Button events', function() { - it('Edit Review', function() { - spyOn(RB.ReviewDialogView, 'create'); - - view.$('#review-banner-edit').click(); - - expect(RB.ReviewDialogView.create).toHaveBeenCalled(); - expect(RB.ReviewDialogView.create.calls.argsFor(0)[0].review) - .toBe(model); - }); - - it('Publish', function() { - spyOn(model, 'publish'); - - view.$('.rb-c-menu-button__primary').click(); - - expect(model.publish).toHaveBeenCalled(); - }); - - it('Publish to Submitter Only', function() { - spyOn(model, 'publish'); - - /* - * The alternative buttons from the split button are added to the - * . - */ - $('#review-banner-publish-submitter-only').click(); - - expect(model.publish).toHaveBeenCalled(); - }); - - it('Publish and Archive', function() { - spyOn(model, 'publish'); - spyOn(view, '_onPublishClicked').and.callThrough(); - - /* - * The alternative buttons from the split button are added to the - * . - */ - $('#review-banner-publish-and-archive').click(); - - expect(model.publish).toHaveBeenCalled(); - expect(view._onPublishClicked).toHaveBeenCalledWith({ - publishAndArchive: true - }); - }); - - it('Discard', function() { - let $buttons = $(); - - spyOn(model, 'destroy'); - spyOn($.fn, 'modalBox').and.callFake(options => { - options.buttons.forEach($btn => { - $buttons = $buttons.add($btn); - }); - - /* Simulate the modalBox API for what we need. */ - return { - modalBox: cmd => { - expect(cmd).toBe('buttons'); - return $buttons; - } - }; - }); - - view.$('#review-banner-discard').click(); - expect($.fn.modalBox).toHaveBeenCalled(); - - $buttons.filter('input[value="Discard"]').click(); - expect(model.destroy).toHaveBeenCalled(); - }); - }); - - describe('Methods', function() { - it('show', function() { - expect(view.$el.hasClass('hidden')).toBe(true); - expect(view.$el.prop('hidden')).toBe(true); - - view.show(); - expect(view.$el.hasClass('hidden')).toBe(false); - expect(view.$el.prop('hidden')).toBe(false); - }); - - it('hide', function() { - view.$el - .addClass('hidden') - .prop('hidden', true); - - view.hide(); - expect(view.$el.hasClass('hidden')).toBe(true); - expect(view.$el.prop('hidden')).toBe(true); - }); - }); -}); diff --git a/reviewboard/staticbundles.py b/reviewboard/staticbundles.py index ff5b8db1638aaeb8dd1eff85b10468587e7e9359..d4d11062fe572659b750798ca2f9062fd145cf6c 100644 --- a/reviewboard/staticbundles.py +++ b/reviewboard/staticbundles.py @@ -78,7 +78,6 @@ 'rb/js/views/tests/collectionViewTests.es6.js', 'rb/js/views/tests/diffFragmentQueueViewTests.es6.js', 'rb/js/views/tests/diffFragmentViewTests.es6.js', - 'rb/js/views/tests/draftReviewBannerViewTests.es6.js', 'rb/js/views/tests/screenshotThumbnailViewTests.es6.js', 'rb/js/views/tests/uploadAttachmentViewTests.es6.js', ), @@ -160,7 +159,6 @@ 'rb/js/utils/textUtils.es6.js', 'rb/js/views/diffFragmentQueueView.es6.js', 'rb/js/views/diffFragmentView.es6.js', - 'rb/js/views/draftReviewBannerView.es6.js', 'rb/js/views/uploadAttachmentView.es6.js', 'rb/js/views/revisionSelectorView.es6.js', 'rb/js/views/fileAttachmentRevisionLabelView.es6.js',