]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-19391 Update ITs and improve a11y
authorWouter Admiraal <wouter.admiraal@sonarsource.com>
Fri, 2 Jun 2023 10:52:05 +0000 (12:52 +0200)
committersonartech <sonartech@sonarsource.com>
Mon, 5 Jun 2023 20:02:48 +0000 (20:02 +0000)
server/sonar-web/design-system/src/components/TreeMapRect.tsx
server/sonar-web/design-system/src/components/__tests__/__snapshots__/TreeMap-test.tsx.snap
server/sonar-web/design-system/src/components/subnavigation/SubnavigationAccordion.tsx
server/sonar-web/design-system/src/components/subnavigation/SubnavigationItem.tsx
server/sonar-web/design-system/src/components/subnavigation/__tests__/SubnavigationItem-test.tsx
server/sonar-web/src/main/js/apps/component-measures/__tests__/ComponentMeasures-it.tsx
server/sonar-web/src/main/js/apps/component-measures/drilldown/TreeMapView.tsx
server/sonar-web/src/main/js/apps/component-measures/sidebar/Sidebar.tsx

index a52879fd9e50cafa7ad0df540d225454acbefa10..8fb0d3dcc5c48b00b48bf6e64817cf3fa9557783 100644 (file)
@@ -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<HTMLAnchorElement>) {
-    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 (
+    <Tooltip overlay={tooltip} placement={placement ?? PopupPlacement.Left}>
       <StyledCell style={cellStyles}>
-        <StyledCellLink
-          aria-label={props.prefix ? `${props.prefix} ${props.label}` : props.label}
-          onClick={handleRectClick}
-          onKeyDown={handleRectKeyDown}
-          role="link"
-          tabIndex={0}
-        >
-          <StyledCellLabel width={props.width}>
-            {isIconVisible && <span className="shrink-0">{props.icon}</span>}
+        <StyledCellLink href="#" onClick={handleRectClick}>
+          <StyledCellLabel width={width}>
+            {isIconVisible && <span className="shrink-0">{icon}</span>}
             {isTextVisible &&
-              (props.prefix ? (
+              (prefix ? (
                 <span className="treemap-text">
-                  {props.prefix}
+                  {prefix}
                   <br />
-                  {props.label.substring(props.prefix.length)}
+                  {label.substring(prefix.length)}
                 </span>
               ) : (
-                <span className="treemap-text">{props.label}</span>
+                <span className="treemap-text">{label}</span>
               ))}
+            <StyledA11yHidden>{tooltip}</StyledA11yHidden>
           </StyledCellLabel>
         </StyledCellLink>
       </StyledCell>
-    );
-  }
-
-  const { placement, tooltip } = props;
-  return (
-    <Tooltip overlay={tooltip} placement={placement ?? PopupPlacement.Left}>
-      {renderCell()}
     </Tooltip>
   );
 }
@@ -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;
+`;
index 7f3703d9986d858685e413c936fc579c319581d6..f9b2a493f78f2e34012e27e153c72008a2a37e0c 100644 (file)
@@ -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;"
     >
       <a
-        aria-label="SonarQube_Web"
         class="emotion-4 emotion-5"
-        role="link"
-        tabindex="0"
+        href="#"
       >
         <div
           class="emotion-6 emotion-7"
@@ -137,6 +144,9 @@ exports[`should render correctly and forward click event 1`] = `
           >
             SonarQube_Web
           </span>
+          <span
+            class="emotion-8 emotion-9"
+          />
         </div>
       </a>
     </li>
@@ -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;"
     >
       <a
-        aria-label="SonarQube_Search"
         class="emotion-4 emotion-5"
-        role="link"
-        tabindex="0"
+        href="#"
       >
         <div
           class="emotion-6 emotion-7"
@@ -157,6 +165,9 @@ exports[`should render correctly and forward click event 1`] = `
           <span
             class="shrink-0"
           />
+          <span
+            class="emotion-8 emotion-9"
+          />
         </div>
       </a>
     </li>
@@ -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;"
     >
       <a
-        aria-label="SonarQube_Server"
         class="emotion-4 emotion-5"
-        role="link"
-        tabindex="0"
+        href="#"
       >
         <div
-          class="emotion-18 emotion-7"
+          class="emotion-22 emotion-7"
           width="17"
-        />
+        >
+          <span
+            class="emotion-8 emotion-9"
+          />
+        </div>
       </a>
     </li>
   </ul>
index 01e017f100f4c833050c7922f51580945660085c..747d16a31995116b7532697ec9fc6028233b8e02 100644 (file)
@@ -57,12 +57,7 @@ export function SubnavigationAccordion(props: Props) {
   }, [finalExpanded, onSetExpanded]);
 
   return (
-    <SubnavigationGroup
-      aria-labelledby={`${id}-subnavigation-accordion-button`}
-      className={className}
-      id={`${id}-subnavigation-accordion`}
-      role="region"
-    >
+    <SubnavigationGroup className={className}>
       <SubnavigationAccordionItem
         aria-controls={`${id}-subnavigation-accordion`}
         aria-expanded={finalExpanded}
@@ -72,7 +67,14 @@ export function SubnavigationAccordion(props: Props) {
         {header}
         <OpenCloseIndicator open={finalExpanded} />
       </SubnavigationAccordionItem>
-      {finalExpanded && children}
+      {finalExpanded && (
+        <section
+          aria-labelledby={`${id}-subnavigation-accordion-button`}
+          id={`${id}-subnavigation-accordion`}
+        >
+          {children}
+        </section>
+      )}
     </SubnavigationGroup>
   );
 }
index 68bbb8593d24ab0e800276991bd3d5d317284885..54ff544fdaf241b8b31835154e2cdc9f25cb8320 100644 (file)
@@ -39,7 +39,6 @@ export function SubnavigationItem(props: Props) {
   }, [onClick, value]);
   return (
     <StyledSubnavigationItem
-      aria-current={active}
       className={classNames({ active }, className)}
       data-testid="js-subnavigation-item"
       onClick={handleClick}
index 66c776e79f7a682d04d3d7373bf8899a21552a4f..17fb3e4e51dba01453f00ccbe3ec83689f0aa5a2 100644 (file)
@@ -22,30 +22,18 @@ import { render } from '../../../helpers/testUtils';
 import { FCProps } from '../../../types/misc';
 import { SubnavigationItem } from '../SubnavigationItem';
 
-it('should render correctly', () => {
-  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<FCProps<typeof SubnavigationItem>> = {}) {
   return render(
     <SubnavigationItem active={false} onClick={jest.fn()} value="foo" {...props}>
-      Foo
+      <button type="button">Foo</button>
     </SubnavigationItem>
   );
 }
index e03cf2668b9b160a0eceb64f042e4d6b181d2143..88829f455a1721789bab8e596f5709671870c411 100644 (file)
@@ -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/),
index 07e89c39453799597d1ba7f5a887380bc81ab972..2de67a1381e7c4df6d3b1d4cbed2ca4df3e19380 100644 (file)
@@ -205,7 +205,7 @@ export default class TreeMapView extends React.PureComponent<Props, State> {
         ? components[0].measures.find((measure) => measure.metric.key !== metric.key)
         : null;
     return (
-      <div className="measure-details-treemap">
+      <div className="measure-details-treemap" data-testid="treemap">
         <div className="display-flex-start note spacer-bottom">
           <span>
             <strong className="sw-mr-1">{translate('component_measures.legend.color')}</strong>
index caa4aa7f631ac3b58f7ada0ba4e6fe4e264f078b..67075eabf84919c1edfd43b760c89fec02f760d7 100644 (file)
@@ -102,7 +102,7 @@ export default function Sidebar(props: Props) {
           />
         </FlagMessage>
       )}
-      <nav
+      <section
         className="sw-flex sw-flex-col sw-gap-4 sw-p-4"
         aria-label={translate('component_measures.navigation')}
       >
@@ -116,7 +116,7 @@ export default function Sidebar(props: Props) {
             active={isProjectOverview(selectedMetric)}
             onClick={handleProjectOverviewClick}
           >
-            <BareButton>
+            <BareButton aria-current={isProjectOverview(selectedMetric)}>
               {translate('component_measures.overview', PROJECT_OVERVEW, 'subnavigation')}
             </BareButton>
           </SubnavigationItem>
@@ -132,7 +132,7 @@ export default function Sidebar(props: Props) {
             showFullMeasures={showFullMeasures}
           />
         ))}
-      </nav>
+      </section>
     </StyledSidebar>
   );
 }