View people who have approved merge request

Resources

PM @victorwu | UX @hazelyang

Description

• We want to support the unapprove merge request feature (#894 (closed)). The first step is to re-design the existing approvals interface to support #894 (closed) to and allow future (#761) to be created with a clean design.
• The net new functionality for this issue is viewing who has already approved a merge request. (This does not currently exist.)
• This merge request for this issue should only contain this issue and no other scope.

Out of scope

• Removing an approval is not part of this issue.

Design and mockups

• View who has already approved a merge request with the profile pictures with checkmark icon in the top-right corner.
• Tooltip on the profile icons and link to those people, similar to the sidebar.
• View how many remaining approvals are required with the dotted circles.

Changed title: Approv → Merge request approvals re-design

@hazelyang : Should we have a hover state on the profile pictures with a tooltip to show the users are? Should we link to them from the profile circle pictures?

In the other mocks, you added the checkmarks too. If you want that, feel free to update the description with that version.

Added ~874214 frontend ~238305 labels

Mentioned in issue #894 (closed)

Changed title: Merge request approvals re-design → View people who have approved a merge request

@smcgivern : I've added this to the 8.15 meta issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/24344

@victorwu

Should we have a hover state on the profile pictures with a tooltip to show the users are?

Good suggestion. We should do that.

Should we link to them from the profile circle pictures?

Yes!

In the other mocks, you added the checkmarks too. If you want that, feel free to update the description with that version.

Updated.

Milestone changed to %8.15

Added Deliverable label

Mentioned in issue #769

Reassigned to @brycepj

@hazelyang @victorwu

Is it just me, or is the way we're displaying possible approvers pretty bland (all grouped in an h4). It seems like the list of approvers' names within the parenthesis should be styled differently than the text preceding it.

It would be pretty easy code-wise to implement something nicer, if we want. WDYT?

@hazelyang I don't believe we have an existing asset for dotted circles... is that correct?

Changed title: View people who have approved a merge request → View people who have approved merge request

@brycepj Correct.

mentioned in merge request !949 (closed)

mentioned in merge request !906 (closed)

mentioned in merge request !954 (merged)

changed milestone to %8.16

added Stretch label

removed Deliverable label

@brycepj @jneen @smcgivern @jschatz1 : I'd like to have these two issues prioritized for 8.16 as stretch, and have them worked on serially one-by-one, instead of together. Do you folks agree that's do-able and advisable for 8.16? Feel free to disagree or propose alternatives. And let's have a separate chat if it warrants one.

https://gitlab.com/gitlab-org/gitlab-ee/issues/1262

https://gitlab.com/gitlab-org/gitlab-ee/issues/894

@victorwu It doesn't appear to me that there is any backend work to be done for those two.

I'd like to have these two issues prioritized for 8.16 as stretch, and have them worked on serially one-by-one, instead of together.

@victorwu I'm confused Are you talking about issues #1262 (closed) and #894 (closed)? Both of these are done, afaik.

@smcgivern : According to @brycepj, looks like the merge requests are all ready to go, just pending final merge. Let me know if we feel comfortable doing that sometime during 8.16. Thanks!

@victorwu if an MR has the 8.16 milestone, it will get reviewed and done in time.

I think the only thing to do here is to mark https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/954 as closing #1262 (closed) and #894 (closed), which I'll do now!

closed via merge request !954 (merged)

mentioned in commit eb518c89

mentioned in issue #580