Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cve): details page v1 parity #192

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kahboom
Copy link
Contributor

@kahboom kahboom commented Oct 10, 2024

implements #190

Changes

  • Rearrange properties in Overview component to reflect v1
  • Adapt labels in tables according to this guide
  • Move tabs + Overview into dedicated area outside of DetailPage
  • Remove DetailPage, as we are no longer using those breadcrumbs, built-in tabs, or actions

Comparison of v1 and v2

v1v2

New state

Advisories

SBOMs

Packages

Detail popover

TBD

  • Keep or ditch the "Related Products" tab from v1
  • Keep or ditch "Packages" tab from v2
  • Do we want to show properties not shown (available?) in v1 ("non-normative", "CWE", "withdrawn", "released")?
  • What happened to the "Reserved" property in v1, was that renamed? maybe to one of the above?

@kahboom kahboom marked this pull request as ready for review October 10, 2024 12:57
<DescriptionListGroup>
<DescriptionListTerm>CWE</DescriptionListTerm>
<DescriptionListDescription>
{"vulnerability.cwe"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the property is cwes. And this is a mistake in the current main branch as it is rendering a string and not the value of the variable

Copy link
Member

@carlosthe19916 carlosthe19916 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @kahboom here are some minor points I noticed:

  • The new Vulnerability Details page does not have the Breadcrumb to navigate to the previous page as the other pages has.
  • The new Tabs have a grey background and not white as other pages. I am not saying the white one is right or wrong but perhaps we need to make an agreement on how all details pages should look like (either white or gray + any other common patterns) so all pages are consistent. We could do that in another PR surely so we don't block this PR.
  • Because of the gray background there is also a grey space between the table and the bottom pagination. That doesn't look nice IMHO.

Screenshot from 2024-10-11 08-59-28

And to your questions:

Do we want to show properties not shown (available?) in v1 ("non-normative", "CWE", "withdrawn", "released")?

I don't know. I would suggest to keep that information. Show it to UX and PM, take a decision and move on.

What happened to the "Reserved" property in v1, was that renamed? maybe to one of the above?

Good question. Unfortunately, I do not know the answer. We need to ask this question to the backend guys as this is a technical thing. Perhaps the "reserved" property is now either "Modified" or "Withdrawn"

@kahboom
Copy link
Contributor Author

kahboom commented Oct 11, 2024

@carlosthe19916 sorry for the confusion, I thought we were also wanting it to be visually the same as it was before? like this:

Screenshot 2024-10-11 at 11 28 03 AM

That's why I removed the breadcrumbs and the DetailPage etc. but I can revert that. What you showed is the SBOM "related vulnerabilities", so we should be using that same style/layout for CVEs/vulnerabilities detail as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants