Code review comment for lp:~thumper/launchpad/hiding-review-fields

Hi Tim,

Great to see this being worked on, I've often thought when just adding a comment that ignoring a 'Review' outcome field (with '-Select-' selected) was a bit strange when it's not indicated as optional etc.

I say the following with full knowledge that you know your audience here better than I do, but:

1) I'm not convinced the changing show/hide in '[Show|Hide] review fields' is necessary? Wouldn't a static 'Optional review fields' be more consistent. My impression is that people understand the toggle is to show/hide.

2) It seems kind-of weird to me that toggling a drop-down changes other parts of the UI (ie. it *doesn't* just drop down some options). Actually, I'm not sure what the benefit of hiding them is if they are shown by default. (ie. I decided to add a comment only, so I hide the extra fields rather than ignoring them, which causes the UI to update a bit to my specific situation).

3) Just a thought: I wonder whether all of the complexity here could be removed by simply hiding everything initially, except for two links: (1) Add comment (2) Add review . When the user clicks on Add comment, the text area appears with the Save comment button. If instead they clicked on the 'Add review' link, the text area appears with the additional fields and a 'Save review' button. If the user changes their mind half-way through, they can still click on the other link (Add comment, or Add review) which will update the UI (kindof like GMail's reply/reply all - you can click reply all after already writing your reply etc.) The downside to this is that, unless you were happy for comment to be the default (which from what you've said above isn't a good idea), then it wouldn't be possible to display the textarea initially without confusing the UI. Interested to hear what you think.

4) Personally, I'd prefer the review fields *below* the text area. I often find myself doing the following:
  i) Choose a review (Approve) based on my initial thought.
  ii) Start writing the comment, but half way through changing my mind because of something I've discovered, so
  iii) Going back up and changing the Review outcome.
If the fields were after the comment, it would suite (my) workflow, but that could just be a personal thing. Another benefit is that in encourages (but doesn't force) people to write a comment *before* selecting the review outcome (hint, hint, Paul ;) ). Again, I'm interested to hear your thoughts there.

So, a simple alternative that incorporates (4) and requires very little JS would be:

http://people.canonical.com/~michaeln/tmp/codereview.png

where the normal '-Select-' default review option is replaced by 'Comment only', and the only JS required is to disable the review type field unless something other than 'Comment only' is selected. You mentioned above that you'd tried a branch where the fields were visible all the time, but didn't like it? I'm not sure how different my mockup above is from what you tried.

If on the other hand, you think that the fields definitely need to be hidden when adding a comment only, have a think about (3) above. If you hate both those options, then maybe Martin has more thoughts.

I hope that's helpful feedback!