513013 - Reject/Approve UI needs some visual adjustments

Status: VERIFIED FIXED

(support.mozilla.org :: General, defect)

Description

[Paul Craciunoiu [:paulc]]

Apparently, in some circumstances, the text box for rejecting does not wrap to the next line, and falls on the same line as the label.

Also, David mentioned some wording changes in bug 446082 comment 28.

Comment 1

[Paul Craciunoiu [:paulc]]

I find changing the approval box label to "Message to author" inconsistent with "Reason for rejecting" -- so I think we either change both to match or neither. Waiting on David for this.

Comment 2

[David Tenser [:djst]]

If you reject an edit, you have to have a good reason for it, hence "Reason for rejecting (will be e-mailed to author):". 

However, if you approve an edit, do you need a reason for that? It feels artificial to ask for a reason for it. However, you might have feedback to the author, e.g. "Great edit, thanks! I've approved it, but for the future, could you blah blah?"

My suggestion is "Feedback to the author (optional):"

Chris, do you agree here?

Comment 3

[Chris Ilias [:cilias]]

use-cases for that text field are to:
* say thanks
* tell the contributor of any tweaks you are going to make to their edit
* invite them the contributors forum

Sometimes all three. :-)

I agree with "Feedback to the author (optional):"

Comment 4

[Paul Craciunoiu [:paulc]]

Attachment: patch, v1

This renames the approve option and adds a line break for the reject option.

As David said, we can leave the javascript policy for contributors for later.

Comment 5

[James Socol [:jsocol, :james]]

Comment on attachment 397738 [details] [diff] [review]
patch, v1

Looks good to me. Thanks for adding that <br/>

Comment 6

[Paul Craciunoiu [:paulc]]

r50801 / r50802

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED -- suggestions in comment 2 have been implemented.

Comment 8

[Louis-Philippe Huberdeau]

Turns out the feedback form was entirely missing. Upstreamed it.