-
Notifications
You must be signed in to change notification settings - Fork 21
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
RSC-1772: Vulnerability Indicator and Vulnerability Indicator Legend #1199
base: FEATURE_VULNERABILITY_COMPONENTS
Are you sure you want to change the base?
RSC-1772: Vulnerability Indicator and Vulnerability Indicator Legend #1199
Conversation
…rability_indicator
…rability_indicator
…rability_indicator
…rability_indicator_legend
…rability_indicator
…rability_indicator_legend
…rability_indicator
…rability_indicator
…rability_indicator_legend
…773_vulnerability_indicator_legend
…o RSC-1774_vulnerability_score
…773_vulnerability_indicator_legend
…o RSC-1774_vulnerability_score
…773_vulnerability_indicator_legend
…o RSC-1774_vulnerability_score
…yIndicatorPage.tsx Co-authored-by: Ross Pokorny <[email protected]>
lib/src/components/NxVulnerabilityIndicator/NxVulnerabilityIndicator.tsx
Outdated
Show resolved
Hide resolved
…yIndicatorPage.tsx Co-authored-by: Ross Pokorny <[email protected]>
…abilityIndicatorLegendPage.tsx Co-authored-by: Ross Pokorny <[email protected]>
expect(vulnerabilityIndicator).toBeInTheDocument(); | ||
}); | ||
|
||
it('takes precedence of severityLevel if both props are provided', async function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something's wrong if this is passing; the prop is called severityRating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed tests for NxVulnerabilityIndicator
here: 3be4a99
}); | ||
|
||
it('sets the accessible name based on the severity level', async function() { | ||
const el = quickRender({ score: '9.0', severityLevel: 'low' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
level -> rating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in: bcf6d2d
return tooltip.textContent; | ||
}; | ||
|
||
describe('when vulnerabilityScore prop is provided', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fewer tests of values in the middle of each range, more tests of values at the edges of each range.
}); | ||
}); | ||
|
||
describe('when vulnerabilitySeverityLevel prop is provided', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in: bcf6d2d
@use '../../scss-shared/nx-dark-mode-helpers'; | ||
|
||
.nx-vulnerability-indicator-legend { | ||
$max-grid-columns: 6; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be 5 now
const { | ||
critical, | ||
high, | ||
medium, | ||
low, | ||
none, | ||
vertical, | ||
header, | ||
className, | ||
...attrs | ||
} = props; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in: 39456a8
.../components/NxVulnerabilityIndicatorLegend/__tests__/NxVulnerabilityIndicatorLegend.test.tsx
Outdated
Show resolved
Hide resolved
…stExample.tsx Co-authored-by: Ross Pokorny <[email protected]>
…/sonatype/sonatype-react-shared-components into RSC-1772_vulnerability_indicator
Co-authored-by: Ross Pokorny <[email protected]>
…icator.scss Co-authored-by: Ross Pokorny <[email protected]>
…VulnerabilityIndicatorLegend.test.tsx Co-authored-by: Ross Pokorny <[email protected]>
lib/src/components/NxVulnerabilityIndicator/NxVulnerabilityIndicator.tsx
Outdated
Show resolved
Hide resolved
…icator.tsx Co-authored-by: Ross Pokorny <[email protected]>
$-vertical-alignment: -0.1em; | ||
|
||
.nx-vulnerability-indicator { | ||
font-size: var(--nx-font-size-s); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The visible shape of this fontawesome icon doesn't take up the full height of its <svg>
box: the <svg>
is 14px tall but the visible square is only 12.5px tall whereas it's supposed to be 14px. As it happens, this can be fixed by using the regular 16px font size, causing the visible square to come out to the desired 14px. And then, since the "font size" of the icon is the same as that of the adjacent text, we don't have to use a custom vertical-align
.
Jira ticket: https://sonatype.atlassian.net/browse/RSC-1769
Subtasks: