Code review comment for lp:~abentley/launchpad/mp-tweakage

Revision history for this message
Martin Albisetti (beuno) wrote :

> > I feel that going down the path of "Add a review or comment" has not been
> good. Reviewing and commenting are very different things, and mixing them just
> creates confusion.
>
> Thanks for your review.
>
> Can you explain what makes them different? To me, a review is just a
> comment with a little bit of extra information. It forms part of a
> discussion, which is why I want people to be able to perform a review in
> reply to a comment.

I've been involved in many MPs where I just want to comment, not weigh in on the decision. Setting a status is a strong message, and a lot of people don't feel they have that kind of authority. Commenting helps to bring down the barrier of entry.

> I think part of the confusion is that the dropdown says "-Select-",
> implying the user must choose. It should say "Just a comment", "Not a
> review", or something like that.

Yes, I think this is part of the problem. Collapsing this section if you click on "Add a comment", and optionally allow you to expand may be a good middle-ground, but it may also end of feeling like "design by committee",

> > I think we should clearly separate them into different links, and just
> distinguish the page it goes to by having the review section collapsed or
> expanded by default. That way we still allow people to do both, but lead them
> down two different paths.
>
> I still feel that drawing such a strong distinction, and then showing it
> to be unnecessary by allowing people to do either from either page
> doesn't seem like a good approach to me.

Cramming both options in one link because we can feels very wrong.

> I should mention that people who reply often don't know whether they're
> providing a review or a comment until after they've finished typing it
> up. They only know whether they can make a verdict until after they've
> fully explored the issues through typing their response up, and become
> certain there are no outstanding questions.

Yes, this happens to me very often. Sometimes I really know (I click on the review link), sometimes I know I don't want to review (click on comment), and sometimes I don't know (I tendo to click on comment). Which is why I still want the option, but hide the review boxes in one.

> If we have separate links, then we should provide them in all the same
> places. Everywhere it currently says "Reply", it would have a link for
> "Reply as comment" and "Reply as review".

Two links instead of one? That makes it even harder for the user.

> > I also think the way that the links are piled up make it hard to find them.
> When I have been requested to review, I have a [Review] link, and a "Add a
> review or comment" very close to it.
>
> As you suggested recently, I'd like to have people choose which request
> their review satisfies using a dropdown.
>
> That would mean we could remove the [Review] links, which is what I plan
> to do.

I think that's great. If you want to land this, we need to fix this layout issue though.

> > Makes me think that, leaving aside the issue mentioned before, that link
> should at a very minimum be outside the table.
>
> I can do that, but I deliberately placed it as close to the review
> requests as I possibly could, so that when people see the request to
> review, it's very easy to respond.

Could you try it out?

> > As for the link to resubmit the proposal, I feel that clicking a link to
> then click a button is not the best experience. Can't we just have the button
> (as a link!) on the page?
>
> Resubmitting is a painful process to reverse, so I think it makes sense
> to require confirmation.
>
> We should fix 383352, which would make the resubmit page more than
> merely a confirmation page.
>
> I understood that links which change state were against our policy.

> > In the 3.0 spirit, this seems like something that would go on the global
> actions portlets we will have on the top-right. Since that template is not yet
> available, I think we should fix it's current placement, which feels slapped
> on randomly.
>
> As I explained in the cover letter, resubmit is placed next to the
> information that it affects, so it's not random even if that's how it feels.

I think it's not so obvious to the user, and thus the global actions portlet is more appropriate.

> > I wonder if the right place is where we display commits? That area can be
> pushed down quite a bit, so I don't know.
>
> I think we should do our best to keep global actions above the fold.
> (Replying to a comment is specific to the comment, so it's okay if
> *that*'s below the fold.)

I think I agree.

« Back to merge proposal