We move the current internals to the drawer design then address specific parts of the UI that needs to change in future issues.
My thoughts:
There are a few differences we'll need to account for right away to maintain parity with what we'll already have implemented by this is ready for release.
Add optional dismissal comment will have to be migrated to this design from the beginning. I've designed the new experience in a way we can reuse most of the components with slight adjustments to make this easier.
Slight tweak of the Resolve with merge request button language since it's a giant button and won't fit in the header. It's not optimal but it is acceptable for 1 or 2 milestones. See MVC w/solution below:
Looking over the design, that pretty much it in terms of accounting for features we will have available once the MVC work for this is underway.
MVC w/no activity
MVC w/issue
MVC Dismissed
MVC Dismissed w/comment
MVC w/solution
Desired state comparison:
Desired
MVC
Develop the UI components within gitlab-ui and then bring them all together in one release.
There's pros and con to both approaches, here's the main ones:
Option 1
Pro: We can get the sidebar design shipped and ready to be iterated on relatively quickly
Con: We might have to shoehorn some of the old modal stuff into the sidebar to just throw it away in a future release.
Option 2
Pro: We're a lot more likely to add these components to GitLab UI if we take this approach
Con: It might be a while before we see any of the sidebar changes on GitLab
I've flip-flopped between these two approaches a couple of times (even when writing this comment). Which makes me think that either approach is fine, but maybe option 1 is a little better since we see results sooner.
@andyvolpe Sorry for jumping in so late. I kinda agree with the approach, but we a should have a look what Drawer we have available in the Issue dashboards. Maybe we can already re-use.
Another thing, doesn't the drawer clash with the Merge request sidebar?
@leipert could you please double-check if there is any backend need on this? I assume this has to be done after &1425 (closed) so that you have a consistent JSON structure across all our endpoints. But I don't see any other specific backend work for this.
Looking at Scope, we either should start with MR (+Pipeline) OR the Security Dashboard
@leipert don't you plan on having the same components since we are unifying the API so providing the exact same content for all these places? That was the reasoning behind my comment as in such case it would be more work to not apply this everywhere.
AFAICT, 1, 2 and 3 are well underway, but do block this issue, to various extents. 4 and 5 are more tangential; they change the places the modal is used, but also possibly refactor some underlying architecture.
What @markrian said is spot on so I'll just add my own thoughts on the points he raised:
The idea behind this refactor was to make this migration easier. In theory, we should be able to update the modal component to this new design and keep most of the inner workings the same.
Again, the logic (actions, mutations, etc) should remain basically the same. We'll just need to update the UI to match the newer designs.
I don't think we need to change this at all as the toasts will behave in exactly the same way and should tie in to the same Vuex actions.
This should be number 1. It's a huge change to the components, Vuex stores, architecture and everything. In theory, once this is done, it will make replacing the modal a million times easier as everywhere will use the security dashboard app.
This will probably be mostly redundant due to #12896 (closed) I still need to look into this but it's looking like the backend work done here is still useful, but the frontend work will be scrapped
tl;dr: Everything is overlapping here. Doing #12896 (closed) first would make sense as the alternative is to get this working in an app that we're about to replace. The work on the other issues does overlap, but most of it is reusable.
Great point @markrian I'm not sure if there is, though this change would be pretty trivial now that we have the correct endpoints. @andyvolpe is there a UX discovery for doing this? If not I can throw something together as a POC
@samdbeckham@markrian I don't believe an issue exists to tackle the pipeline view but the intent of the sidebar design is to replace vuln modals globally. @samdbeckham could you whip up an issue for this?
After conversation with @andyvolpe (and everyone really), We are moving this to %12.4 and are going to tackle a Drawer implementation in GitLab UI in %12.3.
@NicoleSchwartz minor modifications to the design are required based on the evolution of the drawer component. I've removed the ~"UX ready" label. I propose we push this out to %12.6 for a few reasons.
We will be adjusting the modal slightly as a result of first-class vulnerabilities so work there may have an adverse impact here.
The drawer component is not in gitlab-ui and the documentation is incomplete. This work is scheduled for %12.5
First-class vulns will be introducing a few changes to the experience and we can reduce risk by keeping the modal for another milestone while the dust settles.
@andyvolpe@leipert@NicoleSchwartz isn't that also blocked by the redesign of the MR page #12896 (closed) (currently planned for %12.6 like this one)? I assume it would be a simpler implementation for frontend (and less conflict) if we add this inline design after aligning UI between dashboards and MR page.
@gonzoyumo@leipert@NicoleSchwartz Perhaps We could limit the scope to our dashboards and then address the MR page when we are ready, if that is an acceptable course.
This is about both the MR workflow (security reports label) which is currently under "group::composition analysis" responsibility and the Dashboards ("security dashboard" label) which is under groupdynamic analysis responsibility If you consider this is also including vulnerability interaction then groupstatic analysis comes into the party indeed.
That being said, this is a pure frontend change so which group it ends up in doesn't really matter to me.
Though the proposed change looks radically different from the vision of the UI with first-class vulnerabilities (click on vuln links to vulnerability page on dashboards) so I'm wondering if we shouldn't revisit this? @andyvolpe?
@gonzoyumo yep, the current requirements will change since we are removing the modal from the dashboards. We will still need this new feature in places where we have a modal since it is a much better experience. For now, we can consider the scope to be in the MR and in the Pipeline view.
I will prep designs for the pipeline page and get this issue ~"UX ready" prior to 12.6
@matt_wilson Let's hold on this until after #13561 (closed) ships and post-implementation FE priorities are addressed. To me, this feature is definitely more on the complete side of the maturity model since users will still be able to interact with vulnerabilities without it. As for when to schedule this, I am unsure, maybe Q3/Q4 but I don't see the maturity moving past minimal in 2020 https://about.gitlab.com/direction/maturity/#defend which doesn't seem correct. Regardless %Backlog might be a good bet for now.
@andyvolpe Agreed and backlogged. RE: the maturity timeline, that might change as we "unlock" a lot of capabilities once first class vulnerabilities are complete, but for now, I'll leave this where it is.
RE: the maturity timeline, that might change as we "unlock" a lot of capabilities once first class vulnerabilities are complete, but for now, I'll leave this where it is.
sorry, mistakenly hit comment/close button. @andyvolpe this could also be helpful for the general display used for alert details. Question: this isn't seen anywhere yet, right?
@andyvolpe ah, yes sorry, I meant the one in the description for vulns review. Will it display with the new Security tab section, that is tr onclick => displays drawers? I'm looking to mirror the layout as close as possible for alert details - question, do you prefer the header title to be above the CTAs or below?
@kmann this has not been implemented for vulnerabilities just yet and with the moving pieces in Category:Vulnerability Management I'm not too sure when it will be unless @cam.x has a need for it.
I'm looking to mirror the layout as close as possible for alert details - question, do you prefer the header title to be above the CTAs or below?
Actions should go below the header title, here is the preview of the drawer component though I don't think it got migrated to figma.
Andy Volpechanged title from Inline vulnerability management MVC to Design: Vulnerability Details drawer. Formerly: Inline vulnerability management MVC
changed title from Inline vulnerability management MVC to Design: Vulnerability Details drawer. Formerly: Inline vulnerability management MVC
Andy Volpechanged title from Design: Vulnerability Details drawer. Formerly: Inline vulnerability management MVC to Design: Vulnerability Details drawer.
changed title from Design: Vulnerability Details drawer. Formerly: Inline vulnerability management MVC to Design: Vulnerability Details drawer.