From fd8e03601e66967a4f83f303f772e4a32fea14cf Mon Sep 17 00:00:00 2001
From: 7PH <benjamin.raymond@sonarsource.com>
Date: Tue, 23 May 2023 16:29:20 +0200
Subject: [PATCH] SONAR-19236 Move hotspot location list to new design system

---
 .../subnavigation/SubnavigationItem.tsx       | 12 +--
 .../__tests__/SubnavigationItem-test.tsx      |  6 +-
 .../design-system/src/theme/light.ts          |  3 +-
 .../js/apps/issues/__tests__/IssuesApp-it.tsx | 23 +++---
 .../components/HotspotCategory.tsx            | 23 +++---
 .../components/HotspotListItem.tsx            | 80 ++++++++++++++++---
 .../js/components/common/LocationMessage.css  | 36 ---------
 .../js/components/common/LocationMessage.tsx  |  1 -
 .../locations/SingleFileLocationNavigator.tsx | 43 +++++++---
 .../resources/org/sonar/l10n/core.properties  |  2 +
 10 files changed, 139 insertions(+), 90 deletions(-)
 delete mode 100644 server/sonar-web/src/main/js/components/common/LocationMessage.css

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 eade58670ab..68bbb8593d2 100644
--- a/server/sonar-web/design-system/src/components/subnavigation/SubnavigationItem.tsx
+++ b/server/sonar-web/design-system/src/components/subnavigation/SubnavigationItem.tsx
@@ -22,13 +22,12 @@ import classNames from 'classnames';
 import { ReactNode, useCallback } from 'react';
 import tw, { theme as twTheme } from 'twin.macro';
 import { themeBorder, themeColor, themeContrast } from '../../helpers/theme';
-import { BareButton } from '../buttons';
 
 interface Props {
   active?: boolean;
   children: ReactNode;
   className?: string;
-  innerRef?: (node: HTMLButtonElement) => void;
+  innerRef?: (node: HTMLDivElement) => void;
   onClick: (value?: string) => void;
   value?: string;
 }
@@ -39,18 +38,19 @@ export function SubnavigationItem(props: Props) {
     onClick(value);
   }, [onClick, value]);
   return (
-    <SubnavigationItemStyled
+    <StyledSubnavigationItem
       aria-current={active}
-      className={classNames('js-subnavigation-item', { active }, className)}
+      className={classNames({ active }, className)}
+      data-testid="js-subnavigation-item"
       onClick={handleClick}
       ref={innerRef}
     >
       {children}
-    </SubnavigationItemStyled>
+    </StyledSubnavigationItem>
   );
 }
 
-const SubnavigationItemStyled = styled(BareButton)`
+const StyledSubnavigationItem = styled.div`
   ${tw`sw-flex sw-items-center sw-justify-between`}
   ${tw`sw-box-border`}
   ${tw`sw-body-sm`}
diff --git a/server/sonar-web/design-system/src/components/subnavigation/__tests__/SubnavigationItem-test.tsx b/server/sonar-web/design-system/src/components/subnavigation/__tests__/SubnavigationItem-test.tsx
index 3646eee2b70..66c776e79f7 100644
--- a/server/sonar-web/design-system/src/components/subnavigation/__tests__/SubnavigationItem-test.tsx
+++ b/server/sonar-web/design-system/src/components/subnavigation/__tests__/SubnavigationItem-test.tsx
@@ -25,20 +25,20 @@ import { SubnavigationItem } from '../SubnavigationItem';
 it('should render correctly', () => {
   setupWithProps();
 
-  expect(screen.getByRole('button', { current: false })).toBeVisible();
+  expect(screen.getByTestId('js-subnavigation-item')).toHaveAttribute('aria-current', 'false');
 });
 
 it('should display selected', () => {
   setupWithProps({ active: true });
 
-  expect(screen.getByRole('button', { current: true })).toBeVisible();
+  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.getByRole('button'));
+  await user.click(screen.getByTestId('js-subnavigation-item'));
   expect(onClick).toHaveBeenCalledWith('foo');
 });
 
diff --git a/server/sonar-web/design-system/src/theme/light.ts b/server/sonar-web/design-system/src/theme/light.ts
index f8522d70256..084037eb643 100644
--- a/server/sonar-web/design-system/src/theme/light.ts
+++ b/server/sonar-web/design-system/src/theme/light.ts
@@ -394,7 +394,8 @@ export const lightTheme = {
 
     // subnavigation sidebar
     subnavigation: COLORS.white,
-    subnavigationHover: COLORS.indigo[50],
+    subnavigationHover: COLORS.blueGrey[50],
+    subnavigationSelected: COLORS.blueGrey[100],
     subnavigationBorder: COLORS.grey[100],
     subnavigationSeparator: COLORS.grey[50],
     subnavigationSubheading: COLORS.blueGrey[25],
diff --git a/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-it.tsx b/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-it.tsx
index c7a6a5f9c81..4baa2e1b120 100644
--- a/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-it.tsx
+++ b/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-it.tsx
@@ -617,22 +617,27 @@ describe('issues item', () => {
 
     // Location navigation
     await user.keyboard('{Alt>}{ArrowDown}{/Alt}');
-    expect(dataLocation1Button).toHaveClass('selected');
+    expect(dataLocation1Button).toHaveAttribute('aria-current', 'location');
     await user.keyboard('{Alt>}{ArrowDown}{/Alt}');
-    expect(dataLocation1Button).not.toHaveClass('selected');
-    expect(dataLocation2Button).toHaveClass('selected');
+    expect(dataLocation1Button).toHaveAttribute('aria-current', 'false');
     await user.keyboard('{Alt>}{ArrowDown}{/Alt}');
-    expect(dataLocation1Button).not.toHaveClass('selected');
-    expect(dataLocation2Button).not.toHaveClass('selected');
+    expect(dataLocation1Button).toHaveAttribute('aria-current', 'false');
+    expect(dataLocation2Button).toHaveAttribute('aria-current', 'false');
     await user.keyboard('{Alt>}{ArrowUp}{/Alt}');
-    expect(dataLocation1Button).not.toHaveClass('selected');
-    expect(dataLocation2Button).toHaveClass('selected');
+    expect(dataLocation1Button).toHaveAttribute('aria-current', 'false');
+    expect(dataLocation2Button).toHaveAttribute('aria-current', 'location');
 
     // Flow navigation
     await user.keyboard('{Alt>}{ArrowRight}{/Alt}');
-    expect(screen.getByRole('button', { name: '1 Execution location 1' })).toHaveClass('selected');
+    expect(screen.getByRole('button', { name: '1 Execution location 1' })).toHaveAttribute(
+      'aria-current',
+      'location'
+    );
     await user.keyboard('{Alt>}{ArrowLeft}{/Alt}');
-    expect(screen.getByRole('button', { name: '1 Data location 1' })).toHaveClass('selected');
+    expect(screen.getByRole('button', { name: '1 Data location 1' })).toHaveAttribute(
+      'aria-current',
+      'location'
+    );
   });
 
   it('should show education principles', async () => {
diff --git a/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotCategory.tsx b/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotCategory.tsx
index e34ad991cf0..4599551ead3 100644
--- a/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotCategory.tsx
+++ b/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotCategory.tsx
@@ -70,16 +70,19 @@ export default function HotspotCategory(props: HotspotCategoryProps) {
       expanded={expanded}
       onSetExpanded={onSetExpanded}
     >
-      {hotspots.map((hotspot) => (
-        <HotspotListItem
-          hotspot={hotspot}
-          key={hotspot.key}
-          onClick={onHotspotClick}
-          selected={hotspot.key === selectedHotspot.key}
-          onLocationClick={onLocationClick}
-          selectedHotspotLocation={selectedHotspotLocation}
-        />
-      ))}
+      <ul>
+        {hotspots.map((hotspot) => (
+          <li key={hotspot.key}>
+            <HotspotListItem
+              hotspot={hotspot}
+              onClick={onHotspotClick}
+              selected={hotspot.key === selectedHotspot.key}
+              onLocationClick={onLocationClick}
+              selectedHotspotLocation={selectedHotspotLocation}
+            />
+          </li>
+        ))}
+      </ul>
     </SubnavigationAccordion>
   );
 }
diff --git a/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotListItem.tsx b/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotListItem.tsx
index 2a4dca91052..d123cf260d7 100644
--- a/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotListItem.tsx
+++ b/server/sonar-web/src/main/js/apps/security-hotspots/components/HotspotListItem.tsx
@@ -17,9 +17,19 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
-import { SubnavigationItem } from 'design-system';
+import { withTheme } from '@emotion/react';
+import styled from '@emotion/styled';
+import {
+  BareButton,
+  ExecutionFlowIcon,
+  SubnavigationItem,
+  themeColor,
+  themeContrast,
+} from 'design-system';
 import React, { useCallback } from 'react';
-import LocationsList from '../../../components/locations/LocationsList';
+import { FormattedMessage } from 'react-intl';
+import SingleFileLocationNavigator from '../../../components/locations/SingleFileLocationNavigator';
+import { translate, translateWithParameters } from '../../../helpers/l10n';
 import { RawHotspot } from '../../../types/security-hotspots';
 import { getLocations } from '../utils';
 
@@ -35,6 +45,9 @@ export default function HotspotListItem(props: HotspotListItemProps) {
   const { hotspot, selected, selectedHotspotLocation } = props;
   const locations = getLocations(hotspot.flows, undefined);
 
+  const locationMessage =
+    locations.length > 1 ? 'hotspot.location.count.plural' : 'hotspot.location.count';
+
   // Use useCallback instead of useEffect/useRef combination to be notified of the ref changes
   const itemRef = useCallback(
     (node) => {
@@ -61,16 +74,61 @@ export default function HotspotListItem(props: HotspotListItemProps) {
       onClick={handleClick}
       className="sw-flex-col sw-items-start"
     >
-      <div>{hotspot.message}</div>
-      {selected && (
-        <LocationsList
-          locations={locations}
-          showCrossFile={false} // To be removed once we support multi file location
-          componentKey={hotspot.component}
-          onLocationSelect={props.onLocationClick}
-          selectedLocationIndex={selectedHotspotLocation}
-        />
+      <StyledHotspotTitle aria-current={selected} role="button">
+        {hotspot.message}
+      </StyledHotspotTitle>
+      {locations.length > 0 && (
+        <StyledHotspotInfo className="sw-flex sw-justify-end sw-w-full">
+          <div className="sw-flex sw-mt-2 sw-items-center sw-justify-center sw-gap-1 sw-overflow-hidden">
+            <ExecutionFlowIcon />
+            <span
+              className="sw-truncate"
+              title={translateWithParameters(locationMessage, locations.length)}
+            >
+              <FormattedMessage
+                id="hotspots.location"
+                defaultMessage={translate(locationMessage)}
+                values={{
+                  0: <span className="sw-body-sm-highlight">{locations.length}</span>,
+                }}
+              />
+            </span>
+          </div>
+        </StyledHotspotInfo>
+      )}
+      {selected && locations.length > 0 && (
+        <>
+          <StyledSeparator className="sw-w-full sw-my-2" />
+          <div className="sw-flex sw-flex-col sw-gap-1 sw-my-2 sw-w-full">
+            {locations.map((location, index) => (
+              <SingleFileLocationNavigator
+                key={index}
+                index={index}
+                concealedMarker={true}
+                message={location.msg}
+                messageFormattings={location.msgFormattings}
+                onClick={props.onLocationClick}
+                selected={index === selectedHotspotLocation}
+              />
+            ))}
+          </div>
+        </>
       )}
     </SubnavigationItem>
   );
 }
+
+const StyledHotspotTitle = styled(BareButton)`
+  &:focus {
+    background-color: ${themeColor('subnavigationSelected')};
+  }
+`;
+
+const StyledHotspotInfo = styled.div`
+  color: ${themeContrast('pageContentLight')};
+`;
+
+const StyledSeparator = withTheme(styled.div`
+  height: 1px;
+  background-color: ${themeColor('subnavigationExecutionFlowBorder')};
+`);
diff --git a/server/sonar-web/src/main/js/components/common/LocationMessage.css b/server/sonar-web/src/main/js/components/common/LocationMessage.css
deleted file mode 100644
index 97ddfffa465..00000000000
--- a/server/sonar-web/src/main/js/components/common/LocationMessage.css
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * SonarQube
- * Copyright (C) 2009-2023 SonarSource SA
- * mailto:info AT sonarsource DOT com
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
- */
-.location-message {
-  display: inline-block;
-  vertical-align: top;
-  line-height: 16px;
-  padding: 0 6px;
-  font-size: var(--smallFontSize);
-  word-break: break-word;
-}
-
-.location-index + .location-message {
-  margin-left: 4px;
-}
-
-.source-line-code .location-message {
-  padding-top: 2px;
-  padding-bottom: 2px;
-}
diff --git a/server/sonar-web/src/main/js/components/common/LocationMessage.tsx b/server/sonar-web/src/main/js/components/common/LocationMessage.tsx
index b0bfa8e9e65..62c4cf19e05 100644
--- a/server/sonar-web/src/main/js/components/common/LocationMessage.tsx
+++ b/server/sonar-web/src/main/js/components/common/LocationMessage.tsx
@@ -18,7 +18,6 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 import * as React from 'react';
-import './LocationMessage.css';
 
 interface Props {
   children?: React.ReactNode;
diff --git a/server/sonar-web/src/main/js/components/locations/SingleFileLocationNavigator.tsx b/server/sonar-web/src/main/js/components/locations/SingleFileLocationNavigator.tsx
index c48b381b948..45703b11791 100644
--- a/server/sonar-web/src/main/js/components/locations/SingleFileLocationNavigator.tsx
+++ b/server/sonar-web/src/main/js/components/locations/SingleFileLocationNavigator.tsx
@@ -17,13 +17,13 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
+import styled from '@emotion/styled';
 import classNames from 'classnames';
+import { LocationMarker, StyledMarker, themeColor } from 'design-system';
 import * as React from 'react';
 import { translateWithParameters } from '../../helpers/l10n';
 import { MessageFormatting } from '../../types/issues';
-import LocationIndex from '../common/LocationIndex';
 import LocationMessage from '../common/LocationMessage';
-import { ButtonPlain } from '../controls/buttons';
 import { IssueMessageHighlighting } from '../issue/IssueMessageHighlighting';
 import './SingleFileLocationNavigator.css';
 
@@ -33,6 +33,7 @@ interface Props {
   messageFormattings?: MessageFormatting[];
   onClick: (index: number) => void;
   selected: boolean;
+  concealedMarker?: boolean;
 }
 
 export default class SingleFileLocationNavigator extends React.PureComponent<Props> {
@@ -63,20 +64,18 @@ export default class SingleFileLocationNavigator extends React.PureComponent<Pro
   };
 
   render() {
-    const { index, message, messageFormattings, selected } = this.props;
+    const { index, concealedMarker, message, messageFormattings, selected } = this.props;
 
     return (
-      <ButtonPlain
-        preventDefault={true}
-        stopPropagation={true}
-        aria-current={selected ? 'location' : false}
-        className={classNames('locations-navigator', { selected })}
-        innerRef={(node) => {
-          this.node = node;
-        }}
+      <StyledButton
         onClick={this.handleClick}
+        aria-current={selected ? 'location' : false}
+        className={classNames('sw-p-1 sw-flex sw-items-center sw-gap-2', {
+          selected,
+        })}
+        ref={(n) => (this.node = n)}
       >
-        <LocationIndex>{index + 1}</LocationIndex>
+        <LocationMarker selected={selected} text={concealedMarker ? undefined : index + 1} />
         <LocationMessage>
           {message ? (
             <IssueMessageHighlighting message={message} messageFormattings={messageFormattings} />
@@ -84,7 +83,25 @@ export default class SingleFileLocationNavigator extends React.PureComponent<Pro
             translateWithParameters('issue.location_x', index + 1)
           )}
         </LocationMessage>
-      </ButtonPlain>
+      </StyledButton>
     );
   }
 }
+
+const StyledButton = styled.button`
+  color: ${themeColor('pageContent')};
+  cursor: pointer;
+  outline: none;
+  border: none;
+  background: transparent;
+
+  &.selected,
+  &:hover,
+  &:focus {
+    background-color: ${themeColor('subnavigationSelected')};
+  }
+
+  &:hover ${StyledMarker} {
+    background-color: ${themeColor('codeLineLocationMarkerSelected')};
+  }
+`;
diff --git a/sonar-core/src/main/resources/org/sonar/l10n/core.properties b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
index 10cdf553426..7eeb7acf95e 100644
--- a/sonar-core/src/main/resources/org/sonar/l10n/core.properties
+++ b/sonar-core/src/main/resources/org/sonar/l10n/core.properties
@@ -856,6 +856,8 @@ hotspot.filters.status.safe=Safe
 hotspot.filters.by_file_or_list_x=Your hotspots are currently filtered, {show_all_link}
 hotspot.filters.show_all=show all hotspots
 hotspot.section.activity=Activity
+hotspot.location.count={0} extra location
+hotspot.location.count.plural={0} extra locations
 
 hotspots.reviewed.tooltip=Percentage of open Security Hotspots that have been reviewed (Acknowledged, Fixed or Safe)
 hotspots.review_hotspot=Review Hotspot
-- 
2.39.5