flipping the r+ combo box on the "finish review" dialog shows the "Loading..." indicator

RESOLVED WORKSFORME

Status

RESOLVED WORKSFORME
2 years ago
2 years ago

People

(Reporter: froydnj, Assigned: glob)

Tracking

(Blocks: 1 bug)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
I just finished reviews in https://reviewboard.mozilla.org/r/65572/ and every time I hit "Finish Review..." and flipped the r+ combo box, I was given the "Loading..." indicator at the top for multiple seconds.  I'm unsure what it is doing, but flipping a combo box should not require deep thought on MozReview's part.

Updated

2 years ago
Blocks: 1288133
The "Loading" indicator shows because the R/+/-/? value is being saved.
(Assignee)

Comment 2

2 years ago
i suspect this is a reviewboard bug which appears to have already been fixed on master when migrating to es6..

> RB.setActivityIndicator = function(status, options) {
>     var $activityIndicator = $("#activity-indicator"),
>         $indicatorText = $activityIndicator.children('.indicator-text');
> 
>     if (status) {
>         if (RB.ajaxOptions.enableIndicator && !options.noActivityIndicator) {
>             $indicatorText
>                 .text((options.type || options.type === "GET")
>                       ? gettext("Loading...") : gettext("Saving..."));
> ...

when saving options.type is 'PUT', however that incorrectly results in 'Loading...'

https://github.com/reviewboard/reviewboard/blob/1ddc83098007246c1790d7a1cd9b2c4cdb4713c0/reviewboard/static/rb/js/utils/apiUtils.es6.js#L26

>                 .text((options.type && options.type === 'GET')
>                       ? gettext("Loading...") : gettext("Saving..."));


david - can you verify this is the correct fix?
Assignee: nobody → dwalsh
Yeah, that looks correct.
(Assignee)

Updated

2 years ago
Assignee: dwalsh → glob
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8793618 [details]
Backport fix for 'Loading...' indicator being displayed instead of 'Saving...' (bug 1288109)

https://reviewboard.mozilla.org/r/80332/#review79746
Attachment #8793618 - Flags: review?(dwalsh) → review+

Comment 6

2 years ago
mozreview-review
Comment on attachment 8793618 [details]
Backport fix for 'Loading...' indicator being displayed instead of 'Saving...' (bug 1288109)

https://reviewboard.mozilla.org/r/80332/#review79750

Ooops, R+'d this too soon.  We should copy to a `_mozreview.js` file, yes?
Attachment #8793618 - Flags: review+ → review-
(Assignee)

Comment 7

2 years ago
mozreview-review-reply
Comment on attachment 8793618 [details]
Backport fix for 'Loading...' indicator being displayed instead of 'Saving...' (bug 1288109)

https://reviewboard.mozilla.org/r/80332/#review79750

i don't think so - we're backporting an upstream fix, not making a mozreview specific modification.
(Assignee)

Comment 8

2 years ago
this was fixed as part of another bug.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.