Refine merge request widget

changed milestone to %Backlog

added ~874211 ~897282 ~13921 labels

@dimitrieh : As discussed, go ahead and update the description as necessary. I'll try to document most of the existing scenarios and future ones.

cc @awhildy

changed title from Refine merge request information box to Refine merge request widget

cc @markpundsack

Thanks so much @victorwu for the incredible work here!

@hazelyang perhaps you can chip in here with your thoughts about the MR widget designs you propose here: https://gitlab.com/gitlab-org/gitlab-ee/issues/1125#note_19673334

How about we separate the Pipeline and GitLab Version Service statuses from the merge widget? They are close to each other visually, so I think people will still think they are a group.

I am also for a split in any case as for:

The information box contains relevant information as well as calls-to-action. How should this be more effectively displayed to the user?

One more thing that can block a merge: unresolved comments.

Thanks for putting this together, @victorwu! I think it would be great to harmonise everything that's in here

@hazelyang + @dimitrieh : Although I disagree (currently at least) with parts of the design / concept in https://gitlab.com/gitlab-org/gitlab-ee/issues/1125, I super like how you've separated the different status messages into different rows, each with it's own border. I really like the consistency. It's scalable and it's consistent. As a user scans the mr, they see a few rows of information that GitLab is alerting them too. And if some of the rows have a call to action, it can be styled differently. So even if the mr has a lot of things going on, the user can review them, one by one. Their eyes are not darting around the page. And one design is that for higher priority things we want to alert the user or ask them to do something, we can just put them higher in the list. So we are just using a simple list layout to indicate priority / importance to the user. The user may feel like, "oh crap, my mr has a lot of problems", but at least we can make their life easier by presenting them a single list. And if we do end up using a flat list of alerts / call-to-actions, it would probably make design and coding easier going forward, since everything could essentially be templatized and you could have a consistent style guide. Just my $0.02 / £0.02 / €0.02 / ¥0.02 though. I'm sure you folks will continue to evolve and iterate.

thanks @smcgivern . Yeah, I mentioned "resolving discussions" in the list. Adding your link to the description.

Would it be feasible to mark each file reviewed? Then as additional commits are added (as comments are addressed), only a diff since the last review would be shown. Could be as easy as logging the last commit the file was marked reviewed at.

@imPure thanks for the suggestion! I'm not sure this is directly related to this issue, though? This is just about the widget area.

Also, what you're asking for sounds a lot like merge request versions: https://docs.gitlab.com/ce/user/project/merge_requests/versions.html Does that help at all?

Thanks, perhaps this is the wrong place to suggest this. I was aware of this feature, but it has a requires one to remember what they've looked at before. Consider my comment closed though :)

Adding my two cents from a comment on another issue about merge actions and pipeline status:

The merge widget is a bit confusing right now. It is mixing information/actions about the merging process of the MR and separate information about the pipeline for the MR. Today we have, in order:

MR merging info/status: “Request to merge…” and “Merged […] by…”

MR browsing actions/options: “Checkout branch” and “Download as”

Pipeline info

MR merging primary actions/options: “Accept merge request” or “Merge when build succeeds” or “Squash and merge” or “Revert”

MR merging secondary actions/options: “Remove source branch”, “Modify commit message” and “Cherry pick”

MR merging information/consequences: “Accepting this merge request will close…”, “The source branch will be removed” and “The source branch has been removed”

For me, the MR and the Pipeline content should be consumed separately. For example, as @dimitrieh suggested in this mockup.

mentioned in issue #25569 (moved)

mentioned in issue #21135 (closed)

added ~19492 label

assigned to @dimitrieh

mentioned in issue #25996 (closed)

mentioned in issue #26001 (moved)

Finally I got to this issue!

The idea here as said before is to create a system on which we can extend if necessary, doesn't alienate current work flows, is clearer and more aesthetically pleasing.

I created 4 situation examples following the same design system. Probably we can still improve a lot on those, but i wanted to get some reactions and thoughts first.

MR-open-accept-mr:

MR-merged:

MR-open-pipeline-fail:

MR-open-wip:

The idea here is to have a clear hierarchy and connection between request (see what will happen), meta data(context like pipeline, deploy & wip etc) and action (to merge, see what happened).

As you can see in the examples, the connection towards the action gets severed if something is not passing or prevents it from being merged. Also a clear status message appears and tells you exactly what needs to be done in order to proceed.

this doesn't yet include mobile designs, but i am sure we can figure that stuff out :)

I would love to get some thoughts on this: @tauriedavis @pedroms @markpundsack @filipa @grzesiek @victorwu

@dimitrieh This design is really interesting!

One thing to note is that MR widget needs some improvements before we can implement this (or similar) design. See https://gitlab.com/gitlab-org/gitlab-ce/issues/25366.

@dimitrieh thanks! I'm not yet sold on having this ragged right edge:

What happens when I choose to modify the commit message? Does that box expand, or am I editing the commit message in a narrow box?
The buttons aren't in predictable locations. In particular, the 'check out branch' and 'download as' buttons will have their positions change depending on the source and target branch names.
The timestamps aren't aligned, and the items with timestamps aren't necessarily in order! A MR may have been marked as WIP before or after the last pipeline, for instance. I don't think these should be in timestamp order, as that's both hard and confusing, but I think putting the timestamps creates the expectation that there is an order.

I do like the part of this approach that allows for child items, such as the deployments

@dimitrieh : Thanks for this design. It's definitely a major departure from our existing implementation, which is exactly what we need in order to spark conversations we are having now. This in itself is a major win. Thanks!

Mobile: Let's worry about this as an absolute last priority. I would prefer we just make things not even available or extremely simple for mobile. Whatever the least engineering resources to implement and maintain since mobile users are absolutely not our priority right now.
I definitely like the hierarchy and connection design that you've created. To me, this design aims to allow the user to completely understand the state of the merge request in a pretty detailed fashion. Is this what you are designing for? One opposite extreme is hiding everything, and just showing things that are immediately actionable. Can you talk more about the design strategy that you've chosen here to balance complexity in showing everything versus showing only what is immediately actionable? Why do you think the user should see everything in detail when they see the merge request?
As @smcgivern says, buttons being in unpredictable places I can see maybe would be annoying to power users who just quickly glance at the page and want to interact with it. Again, similar to above, if you are more explicit with presenting information in a hierarchy, and that breaks button location expectation, is that a bad thing? Is there a tradeoff here?
Regarding @smcgivern 's comment about timestamp order, it seems that @dimitrieh 's design is aimed at visualizing hierarchy, component-wise relationships, and dependencies/gates, like you've already described in your comments. But notably, timestamps, time order, and just the timeline itself is not a first class citizen in this design. To me that makes sense, since as a user, you care less about what has happened in the far past, and you care more about the current state of the merge request and what needs to be done now to move it to the next stage. (Maybe you care about the recent history too. But not too much history.) Furthermore, other areas such as the Discussion tab in the merge request could be leveraged to provide a more detailed history for the user for those other use cases where you want the entire history.

And finally, the top to down flow of the components implies a high-level time order of things that are done in a merge request. And that seems good enough in terms of "time"-related things. So I'm less concerned than @smcgivern regarding time things. But definitely I'd like to hear what @dimitrieh 's design thinking is regarding this.

Thanks again for this awesome design. Excited it is generating some discussion. And would like others to chime in too!

@DouweM : Can you take a look at @dimitrieh 's latest designs and provide some feedback? I like them overall and want to proceed with some of this work soon.

The latest designs are in updated in the description.

Thanks so much, @dimitrieh! While conceptually I really like the direction, visually, I have a lot of concerns. I think this adds visual noise to an already busy page, and the whole thing doesn't hang together as a single element. There is a lot of padding and strokes that add distraction to the page. There are also a lot of different type treatments and colors. (Though I do appreciate you getting rid of the grey background! )

Some questions I would ask:

What are the important connections/relationships to show? Is it really valuable to try and show all of them?
What variables can we reduce (type size, type color, etc.)
Can we make the whole thing visually clump together better?

The whole timeline/relationship/connections approach is awesome. I feel like it communicates some interesting details. But at the moment, it feels like it is trying to communicate too much and adds too many elements for someone to have to understand and take in. Maybe there is a simplified view of this that shows the high level, important information, that a person can 'explode' out into this more detailed view if they desire?

Also, what lines/strokes/borders can you remove, using spacing to relate elements instead? Sometimes just getting the right spacing, and removing the lines can clean up a design. Thanks!!

Awesome to see all the conversation happening, this first release was to spark exactly that! We can indeed improve a lot

thanks I will come back to this discussion later on, for now just a note to myself (and others)

perhaps take cues from this issue's design: https://gitlab.com/gitlab-org/gitlab-ce/issues/26173

as it includes the nesting

Thanks @awhildy + @dimitrieh .

To me, we have a few big areas of the merge request page:

Title and description
Widget
Tabs (Discussion, Commits, ...)

Do we have a clear idea what each area is intended to do? And are they optimized? For example, we have increasingly more complexity in the widget regarding CI/CD information. Does it make sense to simplify much of the complexity there, and pare it down to always display at most one line of CI/CD information. And when you click somewhere on that line, it scrolls you down to the Tabs section with the relevant information displayed.

In this design strategy, the Widget is used to show brief, glanceable, but immediately actionable info, while the Tabs show further detail requiring more friction/work on the part of the user to dig deeper to discover more.

mentioned in issue #26169 (moved)

@dimitrieh Do you have any new concepts / designs you want to chat through in our meeting tomorrow? Are you still working through some updated designs?

@victorwu I will make sure I have updated designs by tomorrow, incorporating all existing feedback!

mentioned in issue #25139 (closed)

Thanks @dimitrieh . Sorry for the last minute change. I'm bogged down tomorrow. Let's reschedule for next week. Looking forward to the chat.

@victorwu alright! working on it anyway see you in Cancun!

Before I will commit new designs I will first react to all comments made previously. Thanks in any case for all the positivity! This first revision was to spark discussion and to see to what extent we can stretch this.

To me, this design aims to allow the user to completely understand the state of the merge request in a pretty detailed fashion. Is this what you are designing for? Can you talk more about the design strategy that you've chosen here to balance complexity in showing everything versus showing only what is immediately actionable?

@victorwu This is pretty nicely said, however the focus is slightly different. I think we have to think in two ways:

We have to design a new general meta data widget design. One which makes sense to add on additional data in another way then just another cell, as if it was a spreadsheet.
Imo we have a special case with the Merge Request widget specifically. As the actionability is linked to its meta data. In that regard its my focus to clearly visualise any obstacles preventing a merge, in order for developers to easier and more efficiently review the MR in its current state. Any details should be important in order for it to be merged.

One opposite extreme is hiding everything, and just showing things that are immediately actionable.

@victorwu This has crossed my mind, still I don't want to hide much or anything behind another click. My focus here is to design it in such a way that this won't be necessary. Especially as you cannot completely predict in what information exactly every user is interested. Again: "Any details should be important in order for it to be merged."

The buttons aren't in predictable locations The timestamps aren't aligned, and the items with timestamps aren't necessarily in order!

@smcgivern clear directions! thanks will take note!

Again, similar to above, if you are more explicit with presenting information in a hierarchy, and that breaks button location expectation, is that a bad thing? Is there a tradeoff here?

@victorwu I think with the comments from @awhildy I will overcome these challenges. I think after the first design revision it's time to lean down on the design in order to solve such problems. Also I will try to get it as close to the existing design as possible.. in order to not lose any previous optimisations done to the existing design.. such as button placement etc

But notably, timestamps, time order, and just the timeline itself is not a first class citizen in this design.

@victorwu these are things we can decide on to hide or perhaps put in tooltips instead ;)

Furthermore, other areas such as the Discussion tab in the merge request could be leveraged to provide a more detailed history for the user for those other use cases where you want the entire history.

@victorwu Yes! Please see https://gitlab.com/gitlab-org/gitlab-ce/issues/26173 cc: @markpundsack (I cc'd you within that issue as well, to get to know your thoughts on that)

implies a high-level time order of things that are done in a merge request.

@victorwu my thinking of order is set in this way:

show what we want to happen
automatic system related information that can prevent merging
manual triggers/information that need to be changed in order for a merge (WIP etc)
actual actions to merge or revert
Show what happened? (or alternatively we can change 1. to reflect this)

I think this adds visual noise to an already busy page, and the whole thing doesn't hang together as a single element

@awhildy correct, exactly my thinking when i was designing this. Will try to get on this by leaning down! the same for clumping together and reducing variables

What are the important connections/relationships to show? Is it really valuable to try and show all of them?

@awhildy as said before my focus is 2 fold, see this comment at the top.

Maybe there is a simplified view of this that shows the high level, important information, that a person can 'explode' out into this more detailed view if they desire?

@awhildy If I can succeed I would like to not complicate this and just come up with 1 thing that solves the situation for any user with a clear focus.

Do we have a clear idea what each area is intended to do?

@victorwu Imo the tabs are ways to deeper inspect the MR at hand, be it either the discussion or the changes, even the test results on the changes... a possibility here could be to show the pipelines in the context of commits (effectively merging commit and pipelines tab, builds tab will be gone soon anyway)

However these tabs are not mandatory in order to merge the MR (although its always smart to check). That information should all be present in some way in the MR widget (do you agree here @awhildy ?)

An example of this could be.. where we currently have a mixed state of information necessary to merge the MR: resolved discussions.

If all discussions have to be resolved in order for this MR to be able to be merged, this information should imo be visible in the MR widget and not hidden in some tab.

Would love to have your thoughts on this, while I work on the next iteration.

Sorry for the long comment! I tried to clearly communicate to whom I write each part of information or is an answer to who's question. cheers!

Thanks, @dimitrieh! Overall sounds great. Some thoughts:

I really appreciate you thinking about the goal of the MR widget, and scoping it down to the information necessary to merge the MR. My only suggestion is to consider it more like pointers to the information necessary to merge the MR. From looking at the MR widget, I should know if I need to look somewhere for more information on the MR, and where that somewhere is. I don't expect the MR widget to have all the information necessary to merge, but instead the right information for me to be confident with merging it, or help me know what information I don't know (and then link me to that information). Not sure if this is just a nitpick, or what you already meant, but figured I would call it out as it may help you slim down the design.
The two focuses you call out make sense to me. Balancing what is right for the MR widget specifically, with building a system that is extensible and flexible enough to handle the more general meta data widget case. However, I'm worried that there is a lot going on, though, and could really make solving all of these problems at the same time challenging. I would confuse myself, so I would choose to either focus on the MR widget, find what is right for that, and then we can do a pass to see how that would apply to other widgets. Or, quickly understand all the other widgets we have to consider, and their general goals (should all the other widgets also be actionable? or do they have different goals), and then once you roughly map that out, dig into the MR widget in detail. But totally depends on how you work and want to approach this complex problem.

mentioned in issue #26173 (moved)

@dimitrieh @victorwu I think of the system information box has providing status and the tabs providing detail, but I realize it's a blurry line. When you see status, you want to act on it right there, which is why we have action buttons on a lot of them. And sometimes some flows are just so common that we want to put the information right there too. That's why we want the pipeline minigraph in the status so you can quickly jump to the failed job rather than having to dig through the pipeline tab.

@dimitrieh I think exploring hierarchy and dependencies is interesting, but something about it bugs me. Yes, it adds overall visual complexity, which should be avoided, but I think it's more than that.

I don't think of the deployment as being a child of the pipeline. I think of the code being deployed. Sure, it technically was deployed by a job in the pipeline, but as a user, I think "Was this MR deployed", not "Did the MR have a pipeline that deployed". And the design with "Merged by User Name" with a child of "The changes were merged" feels like this is really just a single message "User Name merged into master".

I'm not saying there's never a reason for hierarchy, but there's no need for hierarchy in the examples shown.

I assume the real impetus for this view is because you wanted to be able to show that merging is being blocked by a failed pipeline, and have that relationship be clear (and not confused with any other information on the page such as the deployment. And so indenting the deployment message allowed you to draw a direct line from the pipeline to the accept button. I don't have any good answer for that, unfortunately, but I don't think this is the path. Isn't graying out plus adding appropriate message saying why it's disabled sufficient? :)

Also consider that we're looking to add monitoring/metrics information (https://gitlab.com/gitlab-org/gitlab-ce/issues/24254#note_20309327) and feature flag information (https://gitlab.com/gitlab-org/gitlab-ce/issues/24254#note_20890123). Monitoring information could be considered subordinate to the deploy, which if you consider that subordinate to the pipeline, then that's pretty deep nesting, but hey, it might work. But the feature flag information seems like it would be a peer relationship. So would you have to put it above the pipeline information so that you can still draw a line between the pipeline and the merge request? For that matter, if we add any more lines to that area, how would you generically enforce that you can always draw a line between any dependent items? What if the pipeline failing and the metrics dropping could both block the merge? Throw in license checker and there's a third source of blocking. What if all three fail at the same time? Do you draw three lines?

Compare this with GitHub, where all checks are drawn at the same level (with the same graphic checkmark). It's not as rich as ours, where we can include the pipeline mini graph, for example. But their focus on a singular concept of "checks" works to their advantage here.

i think this issue is becoming to big as stated before by @awhildy and I will either have to focus on the MR widget first or all the widgets in general..

@markpundsack my main issue with the current state of thing are, that we are just adding and adding and adding.. while not looking at the bigger picture.. or at the very least consider if what we are doing is the right thing..

We can just keep stacking items under each other as if it was a normal list, but is that beneficial to the user?

Can't we improve and give a better way to support the main goal of that screen: merging.. by giving a glanceable way of inspecting the status and what prevents it from being merged?

I mean we plan to add additional settings as approvals, feature flags, monitoring, deploy statistics, etc etc more and more and more.. until the list is a view on itself to decipher.

Perhaps I am trying to do to much here and I should go back to basics, by perhaps just decoupling status from actions in the MR widget...

Have a status widget. for people looking to know the status and an action widget for people wanting to do some action.. be it merging, reverting or in the future deploying, turning on feature flags etc

GH does this in some very limited basic way

still i feel we need to have a meeting about this to align our thoughts, before I can do meaningful work here

Did a quick survey to see what everybody thought about the current situation:

most annoying things is:

doesn't show all things blocking the MR at the same time..
doesn't update live... (the actionable stuff)
it's place can shift dramatically because of screenshots included (which perhaps even include the mr itself..)
- branch name should be visible on top
- copy branch name, as you already checked it out once
ugly!
less apparent pipeline status

What we want preferably:

Just a checklist to check off
- Have WIP showed consistently styled
Things which are correct or good... perhaps don't show those...
- GH folds stuff away! (toggle-able)

mentioned in issue #26752 (closed)

@victorwu @awhildy @markpundsack @dzaporozhets

Below I show the next iteration of the redesign of the MR widget. It tries to tackle most of the problems described in previous comments, the painpoints described in https://gitlab.com/gitlab-org/gitlab-ce/issues/25424#note_21432001 and the design direction discussed in mexico and described at https://gitlab.com/gitlab-org/gitlab-ce/issues/26752

Sometimes it goes beyond what is currently possible/done, but the design system is pretty clear i think and holds quite true to what we currently have.

I will create another feature request issue soon, that tackles: "it's place can shift dramatically" with a sticky go to button on the side. Probably there are some additional issues needed that tackle some of the extended features i visualised here.

Would love to have your thoughts!

MR- nothing to merge

MR protected branch/ not enough permissions to merge

MR Open, but with failed pipeline (collapsed failed build logs) and WIP

MR Open, but with failed pipeline (expanded failed build logs) and WIP

MR able to merge

MR-merged

Awesome @dimitrieh! I really like it! Some detailed critique:

In the cases where I can't merge the MR, I wonder if it is cleaner to just not have the 'Merge' button. You have the nice clear text that explains why you can't merge it. I'm not sure a disabled 'Merge' button is necessary. Then, in the cases where you can merge it, it is even more clear that you can merge it (as it is the difference between no button and a bright green button vs. a disabled button and an enabled button).
The larger status icons on the left work well for me, but I wonder for the status icons that are inline, if they should be smaller so they are more visually secondary.
I'm not sure about 'Stop environment' being right aligned. I wonder if it is better if it is just inline, after the row about the environment. Also, is the icon for the 'Stop environment' command necessary?

Overall, I think this is a really great, clean direction. Thanks so much!

mentioned in issue #27063 (moved)

@dimitrieh awesome. I like it. Few notes:

Blue color seems different from what we use everywhere else. I rather use existing one.
Not sure if "6 commits behind" information is that useful to be in the header. Especially after merge it has no value.

@dimitrieh : This looks awesome. Small comment: Why did you opt to use just blank white as the background? Current widget implementation has it a light grey to set it apart from the rest of the page's content. White is good enough now?

Nice @dimitrieh! This very clearly shows the "Can merge" vs "Should merge"

I agree with the blue color, it doesn't look quite like what we use currently. Also I'd love to see it with the left icons smaller like Allison mentioned.

Should the stop environment button have a border? It is a button, correct? Making it black text like the mockup doesn't visually indicate that it is clickable. I also think placing this inline rather than right aligned makes sense.

@victorwu A lot of people felt the grey was ugly. I personally don't mind it and like that it was set apart from the rest of the information but there was an overwhelming response to remove it :)

Maybe to set it apart a little more without using a ton of grey is to make the header portion grey like we do for Cycle Analytics

@tauriedavis : Agreed. I don't mind the grey. So either way is fine by me. Clearly regarding the amount of whitespace / white color, we are far from the extreme of iOS 7 . So it's fine by me! Grey header is a nice compromise.

Yea. I think the challenge for me with the grey was that it felt like it was sending mix signals. I knew this box was important, but the grey also had a way of pushing it back visually, making it feel secondary and less important. A grey header might work :)

@dimitrieh : Feel free to make any final tweaks and I'll scope it out a little more specifically for at least the first iteration and we are good to go for building this. Excited!

The merge button I think we agreed should simply be Merge, but text copy can be further tweaked later. I.e. https://gitlab.com/gitlab-org/gitlab-ce/issues/24559, and we could essentially implement that change with whatever design you have here together.

@dimitrieh I really like it! My only concern with the designs is that I don't see any actions that are blocking a merge with buttons. What I mean is, 'unmark as WIP' uses a link, but things like approvals and resolving conflicts use buttons instead. (There may also be a case for 'unmark as WIP' using a button.) What would that look like?

Also, what do the ellipses after the source branch name mean? They look like the ellipses that we use to expand a commit message, but from the context I'm assuming that they must be to expand the branch name?

mentioned in issue #27199 (closed)

mentioned in issue #23015 (closed)

mentioned in issue #24857 (moved)

mentioned in issue #18292 (moved)

mentioned in issue #26896 (moved)

mentioned in issue #25072 (moved)

mentioned in issue #25330 (closed)

In the cases where I can't merge the MR, I wonder if it is cleaner to just not have the 'Merge' button. You have the nice clear text that explains why you can't merge it. I'm not sure a disabled 'Merge' button is necessary. Then, in the cases where you can merge it, it is even more clear that you can merge it (as it is the difference between no button and a bright green button vs. a disabled button and an enabled button).

@awhildy I believed this was in my list of complaints, but apparently it wasn't, it was more stated like: "doesn't show all things blocking the MR at the same time.."

So yes, I will improve upon those

The larger status icons on the left work well for me, but I wonder for the status icons that are inline, if they should be smaller so they are more visually secondary.

@awhildy good point! although my initial thinking was to refrain from resizing these icons anywhere, and make it more obvious with stating: with stages.. still I will get this edit in ;) and make them from 22px to 18px.. it seems logical as well!

I'm not sure about 'Stop environment' being right aligned. I wonder if it is better if it is just inline, after the row about the environment. Also, is the icon for the 'Stop environment' command necessary?

good points again! in fact i have never been really happy with how its been... let me try and fix that as well.

I have 2 scenarios of button groups:

word buttons (descriptive and fits better with unmark WIP button cc: @smcgivern ):
icon buttons (more similar to the pipeline and environment page):

Maybe to set it apart a little more without using a ton of grey is to make the header portion grey like we do for Cycle Analytics

@tauriedavis nice one, it sets apart the buttons even more, a little.

Blue color seems different from what we use everywhere else. I rather use existing one.

@dzaporozhets @tauriedavis correct! i got this blue color somewhere in my mockups mixed up... i just mean to use the color used by gitlab.. let me change that!

Not sure if "6 commits behind" information is that useful to be in the header. Especially after merge it has no value.

This is a feature currently already in there... i decreased the boldness, and excluded it from merged MR's.

The merge button I think we agreed should simply be Merge, but text copy can be further tweaked later. I.e. #24559 (closed), and we could essentially implement that change with whatever design you have here together.

@victorwu I adjusted the merge buttons and included a visual for merging after pipeline passes see if that works out.

I really like it! My only concern with the designs is that I don't see any actions that are blocking a merge with buttons. What I mean is, 'unmark as WIP' uses a link, but things like approvals and resolving conflicts use buttons instead. (There may also be a case for 'unmark as WIP' using a button.) What would that look like?

@smcgivern adjusted it! and is now in line with the buttons used to stop an environment/launch terminal for environments from the mr widget. I cc'd you at the point I describe it above.

Also, what do the ellipses after the source branch name mean? They look like the ellipses that we use to expand a commit message, but from the context I'm assuming that they must be to expand the branch name?

@smcgivern exactly. This is a design element that can become increasingly long as we name the branches after issue titles.. these should be kept in check. So yes to expand and show the whole branch name.

Additional changes

I included one more view on an alternative version of showing the "merged" MR widget which reduces on vertical space and tries to tackle: "Things which are correct or good... perhaps don't show those...GH folds stuff away! (toggle-able)". cc: @awhildy (as i told you in the chat we had )

Apart from that I included a way of showing a discoverability function on CI/CD within the MR widget if CI/CD is not activated, experimental, but perhaps nice to think about and create an issue on. cc: @markpundsack (check below for the images, and see how you like it)

And I included a way to retry all failed builds from the pipeline and a way to individually retry expanded failed builds.

Updated Designs

MR- nothing to merge

Not enough permissions to merge to repo

Not enough permissions to merge to repo and/or protected branch

MR Open, but with failed pipeline (collapsed failed build logs) and WIP

MR Open, but with failed pipeline (expanded failed build logs) and WIP

MR able to merge

MR able to merge (alternative version, reduced vertical space, checks are folded in)

MR able to merge when pipeline succeeds

MR able to merge, no CI-CD enabled yet (discoverability)

What an amazing work. So much potential. That being said, my 2 cents...

We want to make GitLab super powerful but super easy to use and accessible. Try bringing those UI in front of someone who don't understand our world and want to merge something quickly. This person would be scared to touch anything at this point.
I feel we are designing merge-request --verbose, not merge-request.
Moreover, I feel we are missing a distinct box to represent the actions, or the results of the checks if the action can't be taken (ie Resolve WIP status to be able to merge).

These should even be the things that draw the attention the most when viewing the page:

As a user, I want to know: can I merge?
If not, what's blocking (summarized in one small sentence)?
If I want to know more, show me.

@dimitrieh I truly appreciate the enormous amount of efforts of these designs. Good job.

There's so much in this issue, it's hard to give coherent and comprehensive feedback. I find it hard to articulate my concern, but I'll give it a shot.

One of the main changes is the hierarchy indicated by the ⎣ graphic. The biggest impact of this treatment seems to be the added emphasis on rows that need your attention, with a secondary benefit of providing more space to talk about that item. But my first reaction when I see those secondary lines is that the information should have been presented in a single line and that there's no reason to have the second line at all (ignoring for the moment when there's so much information it would wrap around to a second line anyway).

So why do we feel that this treatment is better? I think it gets back to the emphasis part. Several rows of checkmarks hide the one item that needs your attention. So maybe the problem is that those rows of checkmark information just shouldn't be there at all.

But digging deeper, many of those rows contain reference information that should be accessible. e.g. Showing the pipelines stages of a successful pipeline let's me know if the manual actions have been triggered already. And of course showing me where its deployed let's me click on the URL of the environment to see the running code. So I don't see any easy way to hide that information, while having the interface be fast for intermediate users. (Ideas that come to mind are grouping CI into a single checkmark, and treating the deployment URL as a separate piece of meta information outside of the check/fail status.)

But my net reaction is that I feel this design was aimed to solve a different problem, and if we attack the root problem head-on, we might come up with something different. I'm not against the hierarchy per se, just that it feels forced a bit. It's particularly telling when you look at the post-merge mockup where there are 3 sub-items that indicate what happened because of the merge, but the arrows make them originate from the Revert button, which makes no sense at all. Pressing the Revert button won't make those things happen; they already happened. Those 3 items should be tied to the "Merged by User Name" line, but that doesn't work as well because that information is in the header. That might be a small issue with some obvious solution, but indicates to me that something is just not right here.

For completeness, here's some more feedback.

Despite my rant above about hierarchy, I like the clarity of the "Closes issues #123 (closed) and #456 (closed)" line. Maybe because it's distinct information from the parent.

Having "stop environment" right justified is more problematic in this mockup because of the lack of row separators that in current design help associate the right action with the left information. Having said that, I like switching to action icons and putting both terminal and stop actions there, in which case making them follow the information works. Note however that we right-justify those actions in lots of other places, so objecting to right-justification itself feels a bit weird.

Putting sub-items under the Merge button is interesting. It does make it fairly clear that these will be consequences of pushing the button. But I have to say that it looks weird. It makes the button float upwards a bit, with information above and below it. Most UIs tend to put buttons like that at the bottom only. That shouldn't be enough of a reason not to do it, but just trying to add some context to perhaps why it feels weird.

Personally, I like having the button be grayed out. I know this can be argued either way, but people should easily know where an action would live if they were allowed to do it, but clearly told that they can't, and why. Imagine someone using it for the first time while the button was missing, they could spend some time looking all over for that button, only to eventually realize it's actually not there. A grayed-out button is usually quicker to grok than reading text, especially in a page full of text.

I really with "with stages" wasn't necessary.

Lastly, @dimitrieh writes that the main goal of this screen is merging. I'm going to posit that the main goal of this page is to review someone's work. For any developer with the right permissions, if they wanted it merged, they could have just merged their branch themselves and not even created an MR. Yet they still create MRs because they want someone else to review their work before merging. Maybe this is a meaningless difference of semantics, but it's worth considering because the proposed designs do indeed seem to be focused on the Merge button as if that is the primary need. Pressing the merge button is a very small percentage of effort compared to reviewing the code, reading the comment thread, checking pipeline status, playing with the review app, etc.

people should easily know where an action would live if they were allowed to do it, but clearly told that they can't, and why. Imagine someone using it for the first time while the button was missing, they could spend some time looking all over for that button, only to eventually realize it's actually not there. A grayed-out button is usually quicker to grok than reading text, especially in a page full of text.

I agree with @markpundsack here. I can certainly see people looking for the merge button for a long time before realizing (or making an issue) that it is not there because they are unable to merge.

I feel we are designing merge-request --verbose, not merge-request.

@regisF Our complexity of our product is increasing if we want that or not :). If you look closely you will see that most of the situations above describe mixed states that are currently not possible (or even extra states that are not possible yet, possible explorations). Currently we show one state and hide everything else going on, making our audience confused on what else they have to fix additionally in order to merge (one of the pain points layed out in https://gitlab.com/gitlab-org/gitlab-ce/issues/25424#note_21432001). For beginners or small projects, the view will mostly be similar to the last image of https://gitlab.com/gitlab-org/gitlab-ce/issues/25424#note_22375306

Apart from that, please see the rest of the comment, as it may answer some of your other points as well in some fashion.

There's so much in this issue, it's hard to give coherent and comprehensive feedback. I find it hard to articulate my concern, but I'll give it a shot.

This is a broad issue, that with the design layed out we can look at a spectrum of different scenarios to see if the design system works. After that we can focus our attention to each state and edge case with a individual issue, which will be part of the plan of action.

Apart from that, I try to quote people their text and mention them directly as to speak my mind on their comment

One of the main changes is the hierarchy indicated by the ⎣ graphic. The biggest impact of this treatment seems to be the added emphasis on rows that need your attention, with a secondary benefit of providing more space to talk about that item. But my first reaction when I see those secondary lines is that the information should have been presented in a single line and that there's no reason to have the second line at all (ignoring for the moment when there's so much information it would wrap around to a second line anyway).

So why do we feel that this treatment is better? I think it gets back to the emphasis part. Several rows of checkmarks hide the one item that needs your attention. So maybe the problem is that those rows of checkmark information just shouldn't be there at all.

@markpundsack This issue was created as we are heading off into a direction that is not good or great with the MR widget. I think wanting to keep everything in 1 row/line is one of those decisions that doesn't make sense for the added complexity we want to bring to our Merge Request view now and in the future. As said previously, most of the items above display a mixed state of things going on.

In the ux meeting about this particular issue we discussed going back to basics and see how we would do it with pen and paper, without any fancy graphics etc. The checklist idea is the result of this, with added emphasis.

Another point out of this conversation is that merging and code review are not the only goals one might have, now or in the future (specifically when we add monitoring etc). Basically we are designing for a host of different people and their goals at once. We have to find a way to works for most of them.

As for when to hide stuff.. i think its finding the correct balance between preemptively hiding information behind a click (fold is a click as well) and showing it instantly (good to keep in mind: goal - review someone's or your owns work cc: @markpundsack )

Ideas that come to mind are grouping CI into a single checkmark, and treating the deployment URL as a separate piece of meta information outside of the check/fail status.

This could be an option, although i think this is perhaps only suited for review app deployments and not staging or production which are more important.

It's particularly telling when you look at the post-merge mockup where there are 3 sub-items that indicate what happened because of the merge, but the arrows make them originate from the Revert button, which makes no sense at all. Pressing the Revert button won't make those things happen; they already happened. Those 3 items should be tied to the "Merged by User Name" line, but that doesn't work as well because that information is in the header. That might be a small issue with some obvious solution, but indicates to me that something is just not right here.

This is a good spot.. perhaps a point of improvement on my experimental version of the merged state MR widget. I think this is purely a way of correctly sorting the order of information presented and will come back to this.

people should easily know where an action would live if they were allowed to do it, but clearly told that they can't, and why. Imagine someone using it for the first time while the button was missing, they could spend some time looking all over for that button, only to eventually realize it's actually not there. A grayed-out button is usually quicker to grok than reading text, especially in a page full of text.

this is funny, as in my previous design iteration i provided the merge button always, either faded or not and counters some of the comments made previously.

All in all, i think i can do some fiddling still (points described above), but with a design piece this big I will always hurt somebodies flow of work/thinking one way or another. Imo this is an improvement on the current state of things in multiple ways:

it tackles points from https://gitlab.com/gitlab-org/gitlab-ce/issues/25424#note_21432001 (either by design, or re-coding with Vue.js)
it puts a modular system in place on which we can extend, as discussed in the ux meeting in mexico
it puts emphasis on certain information, by creating structure and formatting (which increases information clearness and efficiency)
it handles different audiences that may view the widget by handling the situations "can I", "should i", "what now?" , whatever their goal may be.

@markpundsack do you agree/see value? Imo we are trying to do something that hasn't been done before (not something that is this integrated). Nobody knows the way But i do think these are great steps and deliver on a design on which our product can once again grow in.

I think wanting to keep everything in 1 row/line is one of those decisions that doesn't make sense for the added complexity we want to bring to our Merge Request view now and in the future.

@dimitrieh I'm not claiming that keeping things in 1 row is a goal in itself, just that many of the lines in your mockups feel like they should be one line because they are directly related like a continuation of a single thought. Splitting them into 2 lines feels more like trying to fit the data into a model rather than designing the model around the data.

Another point out of this conversation is that merging and code review are not the only goals one might have, now or in the future (specifically when we add monitoring etc). Basically we are designing for a host of different people and their goals at once. We have to find a way to works for most of them.

Right. I didn't say "code" review was the main goal, but reviewing someone's work. Perhaps I should have said reviewing a change. In order to review that change, you need to see it live, see performance metrics as a result of the change, etc. And different people may be interested in different aspects of that review. But they all have the same underlying goal.

I'm not claiming that keeping things in 1 row is a goal in itself, just that many of the lines in your mockups feel like they should be one line because they are directly related like a continuation of a single thought. Splitting them into 2 lines feels more like trying to fit the data into a model rather than designing the model around the data.

@markpundsack can you give an exact example?

or do you refer to for example, that the deployment is not really part of a checklist for something to get merged?

About auto-rebase and squashing commits, this should be on CE IMHO.

This is not a very "professional" feature, just some other merge methods which can be useful for everyone, even for public project.

GitHub already implement it for public repositories.

If you need a MR for that, I'm sure the community would like to do that.

Regards.

EDIT: Same opinion and fact for MR approvals IMHO.

@dimitrieh I have some small, but concrete, pieces of feedback:

I don't think we should duplicate the branch names in the header and in the hierarchy view.
We don't currently show 'closed #x and #y' on merged MRs. I'm not even sure we can do that, and be accurate about it. (What if the MR would have closed the issue, but it was closed while the pipeline was running?)

On another note, we could possible have a 'Rebase and merge when pipeline passes' button https://gitlab.com/gitlab-org/gitlab-ee/issues/895#note_22633439

mentioned in issue #27574 (closed)

mentioned in issue #15275 (moved)

changed milestone to %9.0

added Deliverable label

@dimitrieh can you add a SSOT to this issue?

@dimitrieh - something to think about in regards to the merge widget when there are merge conflicts: https://gitlab.com/gitlab-org/gitlab-ce/issues/21947#note_22711524

Theres been confusion over source/target branches for resolving merge conflicts. One community member suggests it may help to position merge conflict resolution in a different location than the merge button since they are doing the opposite thing.

mentioned in issue #21947 (moved)

@jschatz1 I will come up with a plan tonight, in order to get this into a proper state, in which it solves the correct problems and is scopable in order for us to get it into a shippable state. I have had a discussion with mark on this topic and it needs some edits.

@awhildy @smcgivern @victorwu @markpundsack @jschatz1 @DouweM

Let's first state that I will refactor this issue and its description to a meta issue that will keep track of solving various known problems of the Merge Request widget with their own sub issues, rather than using it as a big redesign issue.

The reason for this is that I have become aware that this issue is juggling too many problems, improvements or changes at the same time in order for it to be clear in exactly what way it will bring value to our users their workflows and it's scope-able enough in order to get it actually implemented in a or multiple release cycle(s).

Still this issue has had quite some momentum and exploration that has opened, in any case, my eyes in new ways to approach this. I also think that the visual language described above is sufficiently beautiful enough to be used as an example in solving at least one of the complaints (aesthetics).

A new angle

Apart from that, we have come to know that there are actually multiple reasons for people to use the Merge Request view, however these depend on the state and/or the person.

States

For example we can have the following states:

Unmerged
- WIP
- Nothing to merge
- Branch does not exist
- Still checking to see if it can be merged (more of a loading/implementation thing blocking the merge)
- Cannot merge as project is archived
- Has merge conflicts)
- Pipeline must pass in order to merge
- All discussions must be resolved in order to merge)
- All discussions must be resolved in order to merge)
- License finder failed for X licenses
- Needs to be approved/need X more approvals
- Able to merge
- No actions possible as persona (Merged without any access to the project) has no permissions to act at all
Merged
- Source branch has been removed
- Source branch can still be removed
- No actions possible as persona (Merged without any access to the project) has no permissions to act at all
(now/near future) Deployed to env X
(near future) Activated for Feature flag X

Personas

While we can have the following personas:

Assignee (assuming its not the reviewer, but the one who is contributing code)
Code Reviewer
Implementation Reviewer (UX / Visual UI)
Merger
- Without write access
- With write access
- Without any access to the project (not logged in for example)
(now/near future) Release Manager/Deployment Engineer
(near future) Performance Reviewer

Goals/jobs

All of those will have the following in mind:

To review (most important)
To merge (secondary, but still important) or to revert (if deemed necessary)
(now/near future) deploy/re-deploy to environment or roll-back to a previous deployment
(near future) Activate or deactivate for Feature flag X

Where before I was thinking on optimising for all use-cases/personas in every state at once, I now think we can do a better job here. The previous assumption was trying to do too much, while we could handle each situation individually by approaching each as a problem on its own with: "How can we better serve person X in doing their job Y with the Merge Request being in state Z?

Also currently we just have one widget that is being used for everything. Looking at this, I think that is one of the pain points of the current implementation. It isn't clear in separating its goals. In that sense I think it will be smart to separate the widget based on those goals.

Looking back at our "Should I, Can I, what now", it becomes clear to me that we didn't go far enough into exactly defining when we create value to our users.

Restating the pain points described previously

Let's restate the problems that I found out about in https://gitlab.com/gitlab-org/gitlab-ce/issues/25424#note_21432001 and be sure that our problems are actually problems and not just hypothesises or questions to find a problem in order for us to focus down on finding the minimum viable change. Plus they can be related to the story I layed out above.

The list

the following is a bit over stated, but it gets the point across

The current page does not always reflect the current situation if it has changed since I loaded the page, as it doesn't update live and is preventing me from efficiently being able to do my job/goal. (Implementation problem, rather than a UI problem)
When I want to merge, the widget doesn't show all the things blocking the MR at the same time, preventing me from efficiently being able to merge. (goal/job = merge, persona = merger or Merger (with insufficient rights to merge), states = unknown) becomes: The widget is unclear in showing the status or mix of statuses in order for me to effectively be able to merge and do my job/goal.
I can not easily copy the branch name for checking out the branch locally when I already have checked it out before.
There is no consistent way of navigating to the widget and it is preventing me from efficiently being able to do my job/goal.
The aesthetics of the pipeline are suboptimal, which i bugging me on an emotional level, probably making me less likely to efficiently be able to do my job/goal.
The pipeline status is not apparent enough for me to clearly distinguish the current state in which the view is at. Becomes: The visual hierarchy of the pipeline status vs the other elements is suboptimal, which affects my efficiency in doing my job/goal.
The WIP state of the MR view is not consistently displayed or apparent enough for me to clearly distinguish, which affects my efficiency in doing my job/goal.
The current implementation of displaying information is not optimised or even mixed with information which is less or even not relevant for my job/goal.

Plan of action

The plan of action will span multiple releases and is not set in stone. It does however try to factor in the 9.0 release in which some bigger things are expected/wanted. Also it tries to keep into mind what can be done, without mixing multiple issues into 1 MR or have 1 MR be dependent on another to be finished first. I ordered it in the way I think it should be scheduled.

I think with this plan we have enough time to actually design, implement and ship this. While keeping risk to a minimum and be ready for future functionality!

9.0

5 The aesthetics of the pipeline are suboptimal, which i bugging me on an emotional level, probably making me less likely to efficiently be able to do my job/goal.
- By updating the visual language on the existing implementation (so very close to what is already there, but prettier. Make it resemble the style (not structure) described in mockups above)
- Perhaps this can solve 6 as well: The visual hierarchy of the pipeline status vs the other elements is suboptimal, which affects my efficiency in doing my job/goal.
1 The current page does not always reflect the current situation if it has changed since I loaded the page, as it doesn't update live and is preventing me from efficiently being able to do my job/goal.
- By converting the existing implementation to Vue and making it fully real time (also for future functionality)
3 I can not easily copy the branch name for checking out the branch locally when I already have checked it out before.
- Small feature request and easily added copy to clipboard button
4 There is no consistent way of navigating to the widget and it is preventing me from efficiently being able to do my job/goal.
- This stands loose from the widget, but can probably solved by having a fixed position shortcut on the screen when the widget is out of view.

9.1

2 The widget is unclear in showing the status or mix of statuses in order for me to effectively be able to merge and do my job/goal.
- By splitting the widget based on the goals/jobs.

9.1 + beyond

Handle each possibility in: "How can we better serve person X in doing their job Y with the Merge Request being in state Z?
- 7 Can be considered part of this. The WIP state of the MR view is not consistently displayed or apparent enough for me to clearly distinguish, which affects my efficiency in doing my job/goal.
- 8 The current implementation of displaying information is not optimised or even mixed with information which is less or even not relevant for my job/goal.

???

PROFIT

Adding a bit more fuel to the fire here, but we have a stretch goal for 9.0 to add more complexity (performance sparkline) to this workflow. @dimitrieh can we factor that in to our design consideration? Would be good to think about this sooner than later!

Issue is https://gitlab.com/gitlab-org/gitlab-ce/issues/26944.

@joshlambert ha yes thank you! Basically with the 9.0 release i want to stay very close to the current structure of things, but update the aesthetics as we convert the widget to Vue ;)

Today I will deliver the whole or part of SSOT required for 9.0 :)

The added benefit of this is that for existing designs (like #26944 (closed)), will most likely still work with the updated aesthetics version or need only minor edits

I just wanted to state here (as perhaps before it got lost in the incredible amount of information provided), that the 9.0 release will include both aesthetic updates similar to the designs previously given, but without to many structural changes (designs will be delivered soon..and make this a ). The bigger change will be that of the move to Vue, which makes sense to get it into a state where it is real time, always giving a correct state of things (actively being SSOT of the status at all times).

@dimitrieh Thanks for rethinking the basic approach to this and decoupling the issues.

Thanks @dimitrieh for the great proposal.

Visualizing what you are thinking with the design will help out (when you get a chance ). However, after our quick chat and your clarification in your last comment, your approach makes a lot sense and sounds good. I don't actually think this will be a big change from what we were previously talked about - and will still greatly improve the MR widget for 9.0. It also ensure that we keep the 9.0 work mostly UX+FE, where we have the most open resources currently.

@markpundsack @awhildy tomorrow I will set up the plan with sub issues etc

However in the mean time you can get a preview of some of the states (this will be the 9.0 revision, staying close to the current state of things):

Please say if you see some things that still do not make sense!

also some states have added pipelines&pipelines vs the examples below them

Note to self: Consideration: let the disabled merge button say: "unable to merge"

mentioned in merge request !8576 (merged)

I appreciate all of the effort put into this, some very good points raised in this healthy discussion. However, I must say that this issue is so large and dense that it's very difficult to participate in a constructive manner.

@dimitrieh I agree with you, it's necessary to break this issue into tiny parts ASAP so we can move forward. I suggest closing this issue after creating the other ones.

To be most efficient, I believe the plan of action should start with aesthetic improvements only and no kind of structural changes (I think this is what you suggested).

After those improvements are done (or while developing them), I encourage you to reach out to @sarahod and test the personas, goals, and pain points, to make sure we understand what must be solved. After that, trace a plan of how we can solve them in small iterations. Let's not make too many assumptions, this is a pretty complex part of GitLab. I think it's a good candidate for UX research.

Great work putting all of this together

Using-facing wise, we will focus on the merged, closed, and lock states for the re-design. @dimitrieh will focus on the the re-design of those, and in future releases, we will work on the open state, and also any further design changes.

Technical-wise, @fatihacet + @smcgivern will do some awesome work so that everything continues to work and allows us to move forward, minimizing risk along the way.

mentioned in issue #27870 (closed)

removed milestone

Removing 9.0 from this issue. this issue will be a meta issue. Look at the description for issues that will be delivered in each milestone.

added meta label

cc @dzaporozhets

added ~122770 label

added code review label

@fatihacet @dzaporozhets I hope I have all states included. If not let me know!

There are some things mixed in, like

showing if issues are going to be closed yes or no or only are mentioned
Pipelines
Review apps

Ah well, I can improve upon clearness later

Design spec url: ~~https://gitlab-org.gitlab.io/gitlab-design/progress/dimitrie/Object-views-issues-mr-pipelines-builds-commits/25424-Refine-MR-Widget/25424-Refine-MR-Widget-spec-previews/~~

new spec url: https://gitlab-org.gitlab.io/gitlab-design/hosted/25424-Refine-MR-Widgets-spec-previews/

Merged

Old	New

failed removing source branch
process of removing source branch


Will merge when pipeline succeeds

Closed & Locked

Old	New

Able to merge

Old	New


Able to merge when pipeline succeeds

Not able to merge

Old	New










merge conflicts



rebasing required


Need approvals

@fatihacet assigned you for now :)

assigned to @fatihacet

added frontend and removed ~19492 labels

added ~901060 label

note to self: perhaps adjust approved mr yourself screen with a checked button or something.. or something to make it clearer that you yourself have already approved this MR

@dimitrieh "The job for this merge request failed" -> "The pipeline for this merge request failed". Is that really what it says currently? /cc @filipa

@dimitrieh Do we have the design source file already available in somewhere?

@fatihacet I'm guessing this is it: https://gitlab.com/gitlab-org/gitlab-design/tree/master/progress/dimitrie/Object-views-issues-mr-pipelines-builds-commits/25424-Refine-MR-Widget

@awhildy Oh right! I should have check the design repo before asking. Thanks

mentioned in issue #28041 (closed)

@markpundsack I took that screenshot from the gdk-ee .. have adjusted the comment above with an updated visual for that particular one

@fatihacet I created a spec-export so you don't need the design file or sketch ;) it will be available here after the pipeline of the design repository succeeds: https://gitlab-org.gitlab.io/gitlab-design/progress/dimitrie/Object-views-issues-mr-pipelines-builds-commits/25424-Refine-MR-Widget/25424-Refine-MR-Widget-spec-previews/

i added this link to the comment with the designs as well

changed milestone to %9.0

Closed https://gitlab.com/gitlab-org/gitlab-ce/issues/27870 and assigned this issue 9.0 for now as we will work with this issue and a WIP Merge request

mentioned in issue #28185 (closed)

mentioned in merge request !9253 (merged)

@dimitrieh Sorry I am crashing this party, I have a remark about the widget when the MR is merged:

I think the "Revert" button screams for too much attention. When I visit a merge request that was merged, at first glimpse this button makes me feel like there is something wrong with the MR. How do feel about using .btn-close?

That is a great point @to1ne, I totally agree. Could we update the designs to use the outlined button for revert @dimitrieh?

@dimitrieh your mockups break the conventions defined here https://gitlab.com/gitlab-org/gitlab-ce/issues/20892

mentioned in issue #28407 (closed)

@to1ne great spot!

@alexispires Yes that is correct. This is the first step in the evolution of that in both style as well as functionality (although for now mostly style).

@tauriedavis @to1ne Updating the mockups!

mentioned in issue #26944 (closed)

updated https://gitlab.com/gitlab-org/gitlab-ce/issues/25424#note_23180548 with closed and locked states (cc: @selfup @fatihacet )

spec sheet should be updated in about 10-15 min: https://gitlab-org.gitlab.io/gitlab-design/progress/dimitrie/Object-views-issues-mr-pipelines-builds-commits/25424-Refine-MR-Widget/25424-Refine-MR-Widget-spec-previews/

Feedback on current MR view from HN:

My only other issue is the the UI in the merge requests. It all seems to run together and it's difficult (for me at least) to get a handle on what's going on with comments and the status at the top.

https://news.ycombinator.com/item?id=13716058

mentioned in merge request !9555 (merged)

mentioned in merge request !9579 (merged)

mentioned in merge request !9609 (merged)

mentioned in merge request !9630 (merged)

@fatihacet seeing the progress on the MR's! wow Will this be mergable for %9.1 ? :) In that sense, perhaps its wise to update this issue with %9.1 milestone!

for all readers: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9352 is the delivery of this issue for milestone %9.0 :)

changed milestone to %9.1

@dimitrieh that's right. There is some progress and more to come. Unfortunatelly this will not be ready for 9.0 but we are working hard to make it happen for 9.1 and I am very possitive for 9.1. Update the milestone.

Thanks @fatihacet + @dimitrieh

mentioned in issue #29169 (closed)

nice!

removed milestone

changed milestone to %9.1

Per @jschatz1 , @fatihacet will continue to work on the FE of this issue. Per @smcgivern , he will assign a BE resource to coordinate with @fatihacet . @dimitrieh I'm sure will continue to watch closely. Thanks @dimitrieh !

Thanks @victorwu

mentioned in merge request !9561 (merged)

mentioned in issue #29320 (closed)

@oswaldo will help out with this! @oswaldo, can you please take a look at the comments in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9630?

mentioned in merge request !9877 (merged)

mentioned in merge request !9963 (merged)

mentioned in merge request !10006 (merged)

mentioned in issue #29641 (moved)

@fatihacet coverage details can be displayed as follows:

I added a checklist to issue description. There are still tons of things to do. Beware that those checklist items are not only items to complete this feature. Those are the items I noted in my notebook so I want to make them public. Maybe I would need to add more items in the near future.

I also created a checklist for backend and added a few things I discussed with @oswaldo.

/cc @victorwu @jschatz1 @dimitrieh @oswaldo

marked the task Implement "Remove WIP status" button in WIP widget as completed

mentioned in merge request !10127 (merged)

marked the task Implement "Remove source branch" in Merged state as completed

marked the task Make sure that all UJSs are removed. @filipa would be upset otherwise as completed

Thank you @fatihacet!

marked the task Handle Deployment info as completed

marked the task Do some styling, make widgets look better as completed

mentioned in issue #30015 (moved)

mentioned in merge request !10219 (merged)

marked the task Add "Couldn't connect to CI service" state into serializer as completed

marked the task Add Coverage info into serializer as completed

@fatihacet a small note: checkmark on approvers, as it is currently on the widget like this:

is unnecessary and therefor also not in the design from this issue, as stated here https://gitlab.com/gitlab-org/gitlab-ee/issues/2004#note_26206657

so approvers are just displayed like this:

marked the task Handle Coverage info as completed

marked the task Handle "Couldn't connect to CI service" state for pipeline as completed

marked the task Handle pipeline succeed with warning state as completed

marked the task Start polling server if the initial state is unchecked as completed

marked the task Handle Koding integration as completed

marked the task Update widget when a discussion is resolved as completed

marked the task Double check Merge button for Pipeline passed with warnings state as completed

marked the task Handle comment added to update widget for unresolved discussion state. as completed

marked the task Fix cosmetic issues SS SS as completed

mentioned in merge request !10319 (merged)

small update on this:

to :

updating spec url ;)

marked the task Implement polling after merge action as completed

marked the task If page have resolved_conflicts=true in query string check merge status. as completed

marked the task "Source branch removed" and "being removed" is visible at the same time when Remove source branch is clicked in Merged state as completed

marked the task Add loading icon for Merge button when while merging as completed

mentioned in merge request !10354 (merged)

marked the task Show loading icon in the merge button in build running state when Merge Immediately option clicked. as completed

marked the task Update merge button text to Merge in progress when Merge Immediately option selected as completed

marked the task Double check Merge immediately and Merge when pipeline succeeds actions as completed

mentioned in merge request !10377 (merged)

mentioned in merge request !10381 (merged)

mentioned in issue #27911 (closed)

marked the task Add polling for CI status as completed

marked the task Update widget when automatically when pipeline succeeds as completed

marked the task Add download attribute to download buttons. as completed

as discussed with @fatihacet

There is a need of a temporary error message status to reflect status:failed. This status can happen if just before the merge or while merging something in the MR changed, preventing the merge. For example someone started a unresolved discussion, which needs resolving in order to be able to merge.

The following design shows an error message for 10 seconds (bonus if counts down) after which it refreshes the widget and shows the updated status. This way we have a simple way of showing some error state in context of what prevented the merge, without adding to much complexity (the more complex solution would be to show the error inside the updated status.. this would be beyond the scope of this issue).

@dimitrieh There you go

You can also refresh the widget manually

@dimitrieh instead of this super long red sentence, can we have something like Merging this request did fail. Auto refresh in X seconds.

Moreover, something is not crystal clear to me actually. Why would it be refreshed automatically? Will it try to merge it again in X seconds? If so, message should be clearer.

@regisF message is already updated, see the screenshot below.

If merge failed this means that MR status became unmergeable so trying to merge again automatically won't work.

marked the task Handle merge error when merge failed. For example a new discussion added before merge. as completed

@fatihacet Sorry for not tracking entire discussion, so I'm probably missing something, but is there a reason that we can't reload merge request widget instantly after someone clicked "merge" to avoid this red text saying we will reload in X seconds?

@grzesiek that's what I initially proposed to @dimitrieh but I think he thought that this would be better UX. @dimitrieh can you chime in please?

marked the task Implement "Stop environment" action. as completed

marked the task Implement polling for Deployments. as completed

mentioned in merge request !10423 (merged)

@grzesiek @regisF the widget is actually showing the correct status after pressing merge, as the merge did fail. However just saying that the merge did fail over and over again doesn't say much. The opposite is also true.. say you merged and the widget instantly states the new status.. unmerged (with for example a new unresolved discussion). This will confuse the user, as he/she just pressed merge....

In order to avoid confusion we delay the refreshing of the widget temp in order to show that the merge has failed. It says it will show the updated status after this refreshing.. which will result in the status of the merge request needing for example have another discussion being resolved.

This way a use cannot spam the merge button or get unexpected changes of state which do not make sense without a proper warning.

for more info see https://gitlab.com/gitlab-org/gitlab-ce/issues/25424#note_26730624

marked the task Revert and Cherry Pick buttons doesn't work when widget state changed to merged. as completed

@dimitrieh Sorry, I still do not see the point of the 10...0 countdown. Can't we just show "merge failed, because there were unresolved discussions"?

Implement Geo features.

@victorwu Is this a new feature? I can't find anything in the current MR widget for Geo status (the design). There's no data for it currently being exposed either. Just want to make sure I'm not missing something.

cc: @fatihacet

mentioned in merge request !10457 (merged)

Can't we just show "merge failed, because there were unresolved discussions"?

@grzesiek This is what we ultimately want, but in my discussion with @fatihacet this is not yet clear at the moment of failure, but only after updating the widget again showing the new status of "unresolved discussions".

This measure is to avoid extra added complexity and work for this release of the merge request, so that it maybe improved upon in a separate release. As showing the error in direct context of the new status, requires a new kind of mixed status, correct @fatihacet ?

@brycepj that's an EE only feature correct? I don't know if its in already, but i believe I found this in gdk-EE image

and created the design accordingly

@dimitrieh that was exactly what I needed to find the code -- not sure how I missed it! Thanks

cc: @victorwu

This measure is to avoid extra added complexity and work for this release of the merge request, so that it maybe improved upon in a separate release. As showing the error in direct context of the new status, requires a new kind of mixed status, correct @fatihacet ?

Exactly. We can later make this better and show the proper error message instead of counter text etc. For now this is the most boring solution we should follow. /cc @dimitrieh @grzesiek

marked the task Revert and Cherry Pick buttons are visible for anonymous user. as completed

mentioned in merge request !10504 (merged)

marked the task Styling bugs and missing pieces SS SS as completed

marked the task Write unit tests. as completed

marked the task Testing as completed

marked the task Testing as incomplete

marked the task EE features as completed

marked the task Implement Squash commits. as completed

marked the task Implement Geo features. as completed

marked the task Implement Approvals. as completed

marked the task Remove existing widget and delete all related files. as completed

marked the task Check for deprecated CSS and remove if possible. as completed

marked the task Display Prometheus sparkline. #26944 (closed) as completed

mentioned in merge request !10543 (merged)

mentioned in merge request !10548 (merged)

mentioned in issue #30733 (moved)

mentioned in issue #30896 (closed)

changed milestone to %9.2

marked the checklist item Fix existing Rspec tests. as completed

marked the checklist item Double check existing HAML template logics and make sure we covered every possible case in related widget states. as completed

marked the checklist item Perform Cross Browser test. as completed

marked the checklist item Perform Cross Browser test. as incomplete

marked the checklist item Double check MRs to make sure that there is no unresolved discussions. as completed

marked the checklist item Double check MRs to make sure that there is no unresolved discussions. as incomplete

marked the checklist item Make sure that all TODO and FIXME comments are implemented and removed. as completed

marked the checklist item Make sure that all requests are covered with a proper catch statement. as completed

marked the checklist item Double check MRs to make sure that there is no unresolved discussions. as completed

marked the checklist item Final checks and clean up as completed

closed via merge request !10319 (merged)

mentioned in commit 8db76243

mentioned in commit 1ba06c8c

marked the checklist item Testing as completed

marked the checklist item Perform Cross Browser test. as completed

mentioned in merge request !11379 (closed)

mentioned in issue #32521 (closed)

mentioned in issue #32614 (closed)

mentioned in issue #36487

mentioned in issue gitlab#8957

Refine merge request widget

Resources

Latest designs

Problems and scope

WIP

Frontend and Backend Checklist

Frontend

Backend

Designs

Child items ...

Activity

Additional changes

Updated Designs

A new angle

States

Personas

Goals/jobs

Restating the pain points described previously

The list

Plan of action

9.0

9.1

9.1 + beyond

Merged

Closed & Locked

Able to merge

Not able to merge

Admin message

Admin message

Refine merge request widget

Resources

Latest designs

Problems and scope

WIP

Frontend and Backend Checklist

Frontend

Backend

Activity

Additional changes

Updated Designs

A new angle

States

Personas

Goals/jobs

Restating the pain points described previously

The list

Plan of action

9.0

9.1

9.1 + beyond

Merged

Closed & Locked

Able to merge

Not able to merge