From 3722d4b6b7409e77da6016308f434a190fe503c0 Mon Sep 17 00:00:00 2001 From: Wouter Admiraal Date: Fri, 2 Jun 2023 12:52:05 +0200 Subject: [PATCH] SONAR-19391 Update ITs and improve a11y --- .../src/components/TreeMapRect.tsx | 99 ++++++++++--------- .../__snapshots__/TreeMap-test.tsx.snap | 39 +++++--- .../subnavigation/SubnavigationAccordion.tsx | 16 +-- .../subnavigation/SubnavigationItem.tsx | 1 - .../__tests__/SubnavigationItem-test.tsx | 16 +-- .../__tests__/ComponentMeasures-it.tsx | 77 +++++++++------ .../drilldown/TreeMapView.tsx | 2 +- .../component-measures/sidebar/Sidebar.tsx | 6 +- 8 files changed, 139 insertions(+), 117 deletions(-) diff --git a/server/sonar-web/design-system/src/components/TreeMapRect.tsx b/server/sonar-web/design-system/src/components/TreeMapRect.tsx index a52879fd9e5..8fb0d3dcc5c 100644 --- a/server/sonar-web/design-system/src/components/TreeMapRect.tsx +++ b/server/sonar-web/design-system/src/components/TreeMapRect.tsx @@ -22,7 +22,6 @@ import { scaleLinear } from 'd3-scale'; import React from 'react'; import tw from 'twin.macro'; import { themeColor } from '../helpers'; -import { Key } from '../helpers/keyboard'; import { BasePlacement, PopupPlacement } from '../helpers/positioning'; import Tooltip from './Tooltip'; @@ -49,63 +48,60 @@ interface Props { } export function TreeMapRect(props: Props) { - function handleRectClick() { - props.onClick(props.itemKey); - } - - function handleRectKeyDown(event: React.KeyboardEvent) { - if (event.key === Key.Enter) { - props.onClick(props.itemKey); - } - } + const { + placement, + tooltip, + onClick, + itemKey, + x, + y, + width, + height, + fill, + gradient, + label, + icon, + prefix, + } = props; + + const handleRectClick = React.useCallback(() => { + onClick(itemKey); + return false; + }, [onClick, itemKey]); + + const cellStyles = { + left: x, + top: y, + width, + height, + backgroundColor: fill, + backgroundImage: gradient, + fontSize: SIZE_SCALE(width / label.length), + lineHeight: `${height}px`, + }; + const isTextVisible = width >= TEXT_VISIBLE_AT_WIDTH && height >= TEXT_VISIBLE_AT_HEIGHT; + const isIconVisible = width >= ICON_VISIBLE_AT_WIDTH && height >= ICON_VISIBLE_AT_HEIGHT; - function renderCell() { - const cellStyles = { - left: props.x, - top: props.y, - width: props.width, - height: props.height, - backgroundColor: props.fill, - backgroundImage: props.gradient, - fontSize: SIZE_SCALE(props.width / props.label.length), - lineHeight: `${props.height}px`, - }; - const isTextVisible = - props.width >= TEXT_VISIBLE_AT_WIDTH && props.height >= TEXT_VISIBLE_AT_HEIGHT; - const isIconVisible = - props.width >= ICON_VISIBLE_AT_WIDTH && props.height >= ICON_VISIBLE_AT_HEIGHT; - - return ( + return ( + - - - {isIconVisible && {props.icon}} + + + {isIconVisible && {icon}} {isTextVisible && - (props.prefix ? ( + (prefix ? ( - {props.prefix} + {prefix}
- {props.label.substring(props.prefix.length)} + {label.substring(prefix.length)} ) : ( - {props.label} + {label} ))} + {tooltip} - ); - } - - const { placement, tooltip } = props; - return ( - - {renderCell()} ); } @@ -148,3 +144,12 @@ const StyledCellLabel = styled.div<{ width: number }>` ${tw`sw-shrink sw-overflow-hidden sw-whitespace-nowrap sw-text-left sw-text-ellipsis`}; } `; + +const StyledA11yHidden = styled.span` + position: absolute !important; + left: -10000px !important; + top: auto !important; + width: 1px !important; + height: 1px !important; + overflow: hidden !important; +`; diff --git a/server/sonar-web/design-system/src/components/__tests__/__snapshots__/TreeMap-test.tsx.snap b/server/sonar-web/design-system/src/components/__tests__/__snapshots__/TreeMap-test.tsx.snap index 7f3703d9986..f9b2a493f78 100644 --- a/server/sonar-web/design-system/src/components/__tests__/__snapshots__/TreeMap-test.tsx.snap +++ b/server/sonar-web/design-system/src/components/__tests__/__snapshots__/TreeMap-test.tsx.snap @@ -78,7 +78,16 @@ exports[`should render correctly and forward click event 1`] = ` text-align: left; } -.emotion-18 { +.emotion-8 { + position: absolute!important; + left: -10000px!important; + top: auto!important; + width: 1px!important; + height: 1px!important; + overflow: hidden!important; +} + +.emotion-22 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; @@ -100,7 +109,7 @@ exports[`should render correctly and forward click event 1`] = ` max-width: 17px; } -.emotion-18 .treemap-text { +.emotion-22 .treemap-text { -webkit-flex-shrink: 1; -ms-flex-negative: 1; flex-shrink: 1; @@ -120,10 +129,8 @@ exports[`should render correctly and forward click event 1`] = ` style="left: 0px; top: 0px; width: 83px; height: 60px; background-color: rgb(119, 119, 119); font-size: 12.974358974358974px; line-height: 60px;" >
SonarQube_Web +
@@ -145,10 +155,8 @@ exports[`should render correctly and forward click event 1`] = ` style="left: 0px; top: 60px; width: 83px; height: 40px; font-size: 12.276041666666668px; line-height: 40px;" >
+
@@ -165,15 +176,17 @@ exports[`should render correctly and forward click event 1`] = ` style="left: 83px; top: 0px; width: 17px; height: 100px; background-color: rgb(119, 119, 119); font-size: 11px; line-height: 100px;" >
+ > + +
diff --git a/server/sonar-web/design-system/src/components/subnavigation/SubnavigationAccordion.tsx b/server/sonar-web/design-system/src/components/subnavigation/SubnavigationAccordion.tsx index 01e017f100f..747d16a3199 100644 --- a/server/sonar-web/design-system/src/components/subnavigation/SubnavigationAccordion.tsx +++ b/server/sonar-web/design-system/src/components/subnavigation/SubnavigationAccordion.tsx @@ -57,12 +57,7 @@ export function SubnavigationAccordion(props: Props) { }, [finalExpanded, onSetExpanded]); return ( - + - {finalExpanded && children} + {finalExpanded && ( +
+ {children} +
+ )} ); } diff --git a/server/sonar-web/design-system/src/components/subnavigation/SubnavigationItem.tsx b/server/sonar-web/design-system/src/components/subnavigation/SubnavigationItem.tsx index 68bbb8593d2..54ff544fdaf 100644 --- a/server/sonar-web/design-system/src/components/subnavigation/SubnavigationItem.tsx +++ b/server/sonar-web/design-system/src/components/subnavigation/SubnavigationItem.tsx @@ -39,7 +39,6 @@ export function SubnavigationItem(props: Props) { }, [onClick, value]); return ( { - setupWithProps(); - - expect(screen.getByTestId('js-subnavigation-item')).toHaveAttribute('aria-current', 'false'); -}); - -it('should display selected', () => { - setupWithProps({ active: true }); - - expect(screen.getByTestId('js-subnavigation-item')).toHaveAttribute('aria-current', 'true'); -}); - it('should call onClick with value when clicked', async () => { const onClick = jest.fn(); const { user } = setupWithProps({ onClick }); - await user.click(screen.getByTestId('js-subnavigation-item')); + await user.click(screen.getByRole('button')); expect(onClick).toHaveBeenCalledWith('foo'); }); function setupWithProps(props: Partial> = {}) { return render( - Foo + ); } diff --git a/server/sonar-web/src/main/js/apps/component-measures/__tests__/ComponentMeasures-it.tsx b/server/sonar-web/src/main/js/apps/component-measures/__tests__/ComponentMeasures-it.tsx index e03cf2668b9..88829f455a1 100644 --- a/server/sonar-web/src/main/js/apps/component-measures/__tests__/ComponentMeasures-it.tsx +++ b/server/sonar-web/src/main/js/apps/component-measures/__tests__/ComponentMeasures-it.tsx @@ -60,7 +60,7 @@ describe('rendering', () => { await ui.appLoaded(); expect(ui.seeDataAsListLink.get()).toBeInTheDocument(); - expect(ui.overviewFacetBtn.get()).toBeChecked(); + expect(ui.overviewFacetBtn.get()).toHaveAttribute('aria-current', 'true'); expect(ui.bubbleChart.get()).toBeInTheDocument(); expect(within(ui.bubbleChart.get()).getAllByRole('link')).toHaveLength(8); expect(ui.newCodePeriodTxt.get()).toBeInTheDocument(); @@ -100,10 +100,10 @@ describe('rendering', () => { renderMeasuresApp('component_measures?id=foo&metric=sqale_rating&view=treemap'); await ui.appLoaded(); - expect(ui.treeMapCells.getAll()).toHaveLength(7); - expect(ui.treeMapCell(/folderA C metric\.sqale_rating\.name/).get()).toBeInTheDocument(); - expect(ui.treeMapCell(/test1\.js B metric\.sqale_rating\.name/).get()).toBeInTheDocument(); - expect(ui.treeMapCell(/index\.tsx A metric\.sqale_rating\.name/).get()).toBeInTheDocument(); + expect(within(ui.treeMap.get()).getAllByRole('link')).toHaveLength(7); + expect(ui.treeMapCell(/folderA .+ Maintainability Rating: C/).get()).toBeInTheDocument(); + expect(ui.treeMapCell(/test1\.js .+ Maintainability Rating: B/).get()).toBeInTheDocument(); + expect(ui.treeMapCell(/index\.tsx .+ Maintainability Rating: A/).get()).toBeInTheDocument(); }); it('should render correctly for an unknown metric', async () => { @@ -202,10 +202,6 @@ describe('rendering', () => { renderMeasuresApp('component_measures?id=foo&metric=sqale_rating&view=list'); await ui.appLoaded(); - await user.click(ui.maintainabilityFacetBtn.get()); - await user.click(ui.metricBtn('Maintainability Rating').get()); - await ui.changeViewToList(); - expect(ui.notShowingAllComponentsTxt.get()).toBeInTheDocument(); await user.click(ui.showAllBtn.get()); expect(ui.notShowingAllComponentsTxt.query()).not.toBeInTheDocument(); @@ -221,7 +217,7 @@ describe('navigation', () => { // Drilldown to the file level. await user.click(ui.maintainabilityFacetBtn.get()); - await user.click(ui.metricBtn('Code Smells').get()); + await user.click(ui.metricBtn('Code Smells 8').get()); expect( within(ui.measuresRow('folderA').get()).getByRole('cell', { name: '3' }) ).toBeInTheDocument(); @@ -252,7 +248,7 @@ describe('navigation', () => { await ui.appLoaded(); await user.click(ui.maintainabilityFacetBtn.get()); - await user.click(ui.metricBtn('Code Smells').get()); + await user.click(ui.metricBtn('Code Smells 8').get()); await ui.changeViewToList(); expect( @@ -272,19 +268,17 @@ describe('navigation', () => { await ui.appLoaded(); await user.click(ui.maintainabilityFacetBtn.get()); - await user.click(ui.metricBtn('Maintainability Rating').get()); + await user.click(ui.metricBtn('Maintainability Rating metric.has_rating_X.E').get()); await ui.changeViewToTreeMap(); expect(ui.treeMapCell(/folderA/).get()).toBeInTheDocument(); expect(ui.treeMapCell(/test1\.js/).get()).toBeInTheDocument(); - // TODO: once the new design is live, change this to target a link rather than clicking on some text. await user.click(ui.treeMapCell(/folderA/).get()); expect(ui.treeMapCell(/out\.tsx/).get()).toBeInTheDocument(); expect(ui.treeMapCell(/in\.tsx/).get()).toBeInTheDocument(); - // TODO: once the new design is live, change this to target a link rather than clicking on some text. await user.click(ui.treeMapCell(/out.tsx/).get()); expect((await ui.sourceCode.findAll()).length).toBeGreaterThan(0); }); @@ -296,7 +290,7 @@ describe('navigation', () => { // Drilldown to the file level. await user.click(ui.maintainabilityFacetBtn.get()); - await user.click(ui.metricBtn('Code Smells').get()); + await user.click(ui.metricBtn('Code Smells 8').get()); // Select "folderA". await ui.arrowDown(); @@ -347,7 +341,7 @@ describe('redirects', () => { const { ui } = getPageObject(); renderMeasuresApp('component_measures/metric/bugs'); await ui.appLoaded(); - expect(ui.metricBtn('Bugs').get()).toBeChecked(); + expect(ui.metricBtn('Bugs 0').get()).toHaveAttribute('aria-current', 'true'); }); it('should redirect old domain route', async () => { @@ -405,27 +399,48 @@ function getPageObject() { ), // Facets - overviewFacetBtn: byRole('checkbox', { - name: 'component_measures.overview.project_overview.facet', + overviewFacetBtn: byRole('button', { + name: 'component_measures.overview.project_overview.subnavigation', + }), + releasabilityFacetBtn: byRole('button', { + name: 'Releasability component_measures.domain_subnavigation.Releasability.help', + }), + reliabilityFacetBtn: byRole('button', { + name: 'Reliability component_measures.domain_subnavigation.Reliability.help', + }), + securityFacetBtn: byRole('button', { + name: 'Security component_measures.domain_subnavigation.Security.help', + }), + securityReviewFacetBtn: byRole('button', { + name: 'SecurityReview component_measures.domain_subnavigation.SecurityReview.help', + }), + maintainabilityFacetBtn: byRole('button', { + name: 'Maintainability component_measures.domain_subnavigation.Maintainability.help', + }), + coverageFacetBtn: byRole('button', { + name: 'Coverage component_measures.domain_subnavigation.Coverage.help', + }), + duplicationsFacetBtn: byRole('button', { + name: 'Duplications component_measures.domain_subnavigation.Duplications.help', + }), + sizeFacetBtn: byRole('button', { + name: 'Size component_measures.domain_subnavigation.Size.help', + }), + complexityFacetBtn: byRole('button', { + name: 'Complexity component_measures.domain_subnavigation.Complexity.help', + }), + issuesFacetBtn: byRole('button', { + name: 'Issues component_measures.domain_subnavigation.Issues.help', }), - releasabilityFacetBtn: byRole('button', { name: 'Releasability' }), - reliabilityFacetBtn: byRole('button', { name: 'Reliability' }), - securityFacetBtn: byRole('button', { name: 'Security' }), - securityReviewFacetBtn: byRole('button', { name: 'SecurityReview' }), - maintainabilityFacetBtn: byRole('button', { name: 'Maintainability' }), - coverageFacetBtn: byRole('button', { name: 'Coverage' }), - duplicationsFacetBtn: byRole('button', { name: 'Duplications' }), - sizeFacetBtn: byRole('button', { name: 'Size' }), - complexityFacetBtn: byRole('button', { name: 'Complexity' }), - issuesFacetBtn: byRole('button', { name: 'Issues' }), - metricBtn: (name: string) => byRole('checkbox', { name }), + metricBtn: (name: string) => byRole('button', { name }), // Measure content measuresTable: byRole('table'), measuresRows: byRole('row'), measuresRow: (name: string) => byRole('row', { name: new RegExp(name) }), - treeMapCells: byRole('treeitem'), - treeMapCell: (name: string | RegExp) => byRole('treeitem', { name }), + treeMap: byTestId('treemap'), + treeMapCells: byRole('link'), + treeMapCell: (name: string | RegExp) => byRole('link', { name }), fileLink: (name: string) => byRole('link', { name }), sourceCode: byText('function Test() {}'), notShowingAllComponentsTxt: byText(/component_measures.hidden_best_score_metrics/), diff --git a/server/sonar-web/src/main/js/apps/component-measures/drilldown/TreeMapView.tsx b/server/sonar-web/src/main/js/apps/component-measures/drilldown/TreeMapView.tsx index 07e89c39453..2de67a1381e 100644 --- a/server/sonar-web/src/main/js/apps/component-measures/drilldown/TreeMapView.tsx +++ b/server/sonar-web/src/main/js/apps/component-measures/drilldown/TreeMapView.tsx @@ -205,7 +205,7 @@ export default class TreeMapView extends React.PureComponent { ? components[0].measures.find((measure) => measure.metric.key !== metric.key) : null; return ( -
+
{translate('component_measures.legend.color')} diff --git a/server/sonar-web/src/main/js/apps/component-measures/sidebar/Sidebar.tsx b/server/sonar-web/src/main/js/apps/component-measures/sidebar/Sidebar.tsx index caa4aa7f631..67075eabf84 100644 --- a/server/sonar-web/src/main/js/apps/component-measures/sidebar/Sidebar.tsx +++ b/server/sonar-web/src/main/js/apps/component-measures/sidebar/Sidebar.tsx @@ -102,7 +102,7 @@ export default function Sidebar(props: Props) { /> )} - + ); } -- 2.39.5