1291628 - Review comment lost after hitting 'publish'

Status: RESOLVED FIXED

(MozReview Graveyard :: Review Board: User Interface, defect, P1)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: reviewboard-comment-publish.png

See attached screenshot (for reference).

I wrote a lengthy review comment, updated the review state and pressed "Publish". Unfortunately I was required to hit "OK" first. Because of this I lost my comment and had to retype it. This is clearly a 'user error' but it would be nice if the interface could prevent me from doing something stupid. :)

Comment 1

[Steven MacLeod [:smacleod]]

Ah, I have a feeling the publish button inside the old dialog would perform a save, and we didn't make sure the button in the banner now does that.

David, any chance you could look into making sure the draft banner's publish will perform a save of all the text fields when the drop down is open?

Comment 2

[David Walsh :davidwalsh]

Attachment: MozReview: 'maybeSave' unpublished comments within dropdown dialog (Bug 1291628).

Review commit: https://reviewboard.mozilla.org/r/69004/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69004/

Comment 3

[David Walsh :davidwalsh]

https://reviewboard.mozilla.org/r/69004/#review66082

Comment 4

[Steven MacLeod [:smacleod]]

Comment on attachment 8777476 [details]
MozReview: 'maybeSave' unpublished comments within dropdown dialog (Bug 1291628).

https://reviewboard.mozilla.org/r/69004/#review66084

::: reviewboard/reviewboard/static/rb/js/views/draftReviewBannerView_mozreview.js:178
(Diff revision 1)
>       * Handler for the Publish button.
>       *
>       * Publishes the review.
>       */
>      _onPublishClicked: function() {
> +        this._RDVInstance._saveReview(true);

In cases where the dialog hasn't been opened, this will fail won't it?

Comment 5

[David Walsh :davidwalsh]

Comment on attachment 8777476 [details]
MozReview: 'maybeSave' unpublished comments within dropdown dialog (Bug 1291628).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69004/diff/1-2/

Comment 6

[David Walsh :davidwalsh]

https://reviewboard.mozilla.org/r/69004/#review66084

> In cases where the dialog hasn't been opened, this will fail won't it?

Good catch; so many ways to use this dialog.  Updated to accommodate said situation.

Comment 7

[Steven MacLeod [:smacleod]]

Comment on attachment 8777476 [details]
MozReview: 'maybeSave' unpublished comments within dropdown dialog (Bug 1291628).

https://reviewboard.mozilla.org/r/69004/#review66816

This *seems* like it should be fine to me... lets cross our fingers.

Comment 9

[David Walsh :davidwalsh]

AFAIK this just needs deploy

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/webtools/reviewboard/rev/677d00d65abc
MozReview: 'maybeSave' unpublished comments within dropdown dialog . r=smacleod