[MR Widget] V2
## Introduction
Starting from the `Summary` section, the description contains the original proposal. It has been discussed during a sync meeting to move forward with this epic. The idea is to create a Vue Component that handles the MR Widget presentational logic. This component will try to be as much as compatible with the V1 logic to minimise the work required for the refactoring of the existing widgets.
Recording: https://www.youtube.com/watch?v=uJRbKct69_s&feature=youtu.be
## Summary
This issue is created to propose a V2 of the MR Widget. Below, I'm going to list the issues I faced while developing the MR Security Widget and finally propose a solution.
## Issues
### 1. Html injection
The current implementation requires us to inject HTML to modify the UI. As seen on the image, we display information about the number of `Critical`, `High` and `Other` vulnerabilities in different colors. In order to do that, the current code does the following:
```javascript
summary() {
return `
${this.$options.i18n.detected(
this.$options.i18n.securityScanning,
this.totalVulnerabilities(),
)}
<br />
${this.$options.i18n.highlights(this.highlights())}`;
},
```
Where `i18n.highlights` is a method that looks like:
```javascript
highlights: ({ critical, high, other }) => {
return sprintf(
s__(
'ciReport|%{critical_start}%{critical_vulns} critical%{critical_end}, %{danger_start}%{high_vulns} high%{danger_end} and %{strong_start}%{other_vulns} others%{strong_end}',
),
{
critical_vulns: critical || 0,
high_vulns: high || 0,
other_vulns: other || 0,
},
);
},
```
This code has a few issues:
1. Note how we inject `<br />` to move the text in a separate line. This is also harder to manipulate for mobile views.
2. Computed properties do not work as expected. The `totalVulnerabilities` call could easily be a `computed property` but for some reason it's not reactive. The workaround was to move the method into the `methods` field. This is not a big deal, but I had to spend time to figure out why this was happening as I thought the problem was in my code.
3. We cannot re-use existing logic. For instance, the third point shown in the image is a link that opens a modal. I have to replicate the whole logic ([which is very complex](https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue#L793)) and unfortunately I cannot use existing components.
### SprintF needs magic to work
Because the `summary` method is expected to return a text and we cannot make use of the `gl-sprintf` component, we have to do magic in order to simply make a text bold in the summary field.
For instance, in the code above, `danger_start` and `danger_end` are defined by the extension API in the following file: https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/assets/javascripts/vue_merge_request_widget/components/extensions/utils.js. The developer needs to know the existence of this file and the variables exported when they simply want to color a text. Another problem is that if we need a different color, we need to extend that file to include another injection.
### base.vue is doomed to grow
The extension API centralizes the whole logic in the `base.vue` component. This file is already 400 lines and eventually will end up being larger as we introduce more logic. For instance, the security widget needs to display a popover next to the action items as shown below:
To do this, we need to extend the `base.vue` add a `popover` component and extend the extensions to accept a popover configuration.
### Learning curve
New developers coming aboard will have to learn the extension API to work on the MR widgets. Although not steep, this still adds an overhead on the developer.
To sum up, we're sacrificing **efficiency** in the name of **consistency**. In my opinion, we're fighting the problem with wrong tools at hand.
## Alright, Savas, we got this! What is the proposed solution though?
Across GitLab, we already solve a similar problem. The [Pajamas Design System](https://design.gitlab.com/) was introduced to provide consistency across the tool and we're using [gitlab-ui](https://gitlab.com/gitlab-org/gitlab-ui) to implement the design system.
Similarly, we can come up with a `Widget` component, that is purely Vue.js, and can be re-used and extended easily. So the code above becomes:
```vue
<gl-mr-widget :fetch-full-data="fetchFullData" :fetch-collapsed-data:"fetchCollapsedData">
<template #summary>
<gl-sprintf :message="$options.i18n.detectedMessage">
<template #bold="{ content }">
<b>{{ content }}</b>
</template>
</gl-sprintf>
<br />
<gl-sprintf :message="$options.i18n.highlights>
<template #critical>
<severity-badge severity="critical" class="gl-text-center!" />
</template>
<template #high>
<severity-badge severity="high" class="gl-text-center!" />
</template>
<template #others="{ content }">
<b>{{ content }}</b>
</template>
</gl-sprintf>
</template>
</gl-mr-widget>
```
With this approach we have several significant benefits:
1. This is purely Vue and learning curve should be non-existent.
2. Since this is purely Vue, it's easier to maintain and review. Even though code reviewers may not be from the Working Group, they'll still be familiar with the changes as it's similar to the rest of the application.
3. We don't have to inject HTML anymore and come up with workarounds to implement basic functionality.
4. If there is logic that is specific to the widget, it's easier to implement it without breaking existing functionality and it doesn't have to go in a centralised logic like `base.vue`.
## What about consistency?
The basic idea behind the strictness of the current extension API is that it enforces the developer to be **consistent** across different widgets.
Unfortunately, as seen above, the current implementation allows injecting HTML which gives the developer the freedom they want, it's just that we can't use the tools that'd like to use.
I'm proposing to move the consistency check to **Code Review Guidelines** and **Design Guidelines**.
## This is not a complete refactor
Please don't get this wrong! This is not a complete refactor. It's more like breaking up the `base.vue` into multiple components and re-using those components to develop the widgets. The existing logic (`fetchFullData`, `fetchCollapsed` etc... should still be re-usable through the new components.
## Implementation plan
- [ ] Start with the security widget as it's currently too hard to migrate it to the extension api. This could be the PoC
- [ ] Move other widgets one by one through a feature flag
## Figma Design Specs
https://www.figma.com/file/qEddyqCrI7kPSBjGmwkZzQ/%F0%9F%93%99-Component-library?node-id=40462%3A43785
epic