SONAR-20023 Update guide styling to comply with Spotlight from design system

7 files changed, 134 insertions, 66 deletions

diff --git a/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-Filtering-it.tsx b/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-Filtering-it.tsx
index d8df224e24d..965d39f8913 100644
--- a/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-Filtering-it.tsx
+++ b/server/sonar-web/src/main/js/apps/issues/__tests__/IssuesApp-Filtering-it.tsx
@@ -23,6 +23,7 @@ import userEvent from '@testing-library/user-event';
 import React from 'react';
 import { renderOwaspTop102021Category } from '../../../helpers/security-standard';
 import { mockLoggedInUser, mockRawIssue } from '../../../helpers/testMocks';
+import { NoticeType } from '../../../types/users';
 import IssuesList from '../components/IssuesList';
 import {
   branchHandler,
@@ -200,7 +201,7 @@ describe('issues app filtering', () => {
 
   it('should allow to set creation date', async () => {
     const user = userEvent.setup();
-    const currentUser = mockLoggedInUser();
+    const currentUser = mockLoggedInUser({ dismissedNotices: { [NoticeType.ISSUE_GUIDE]: true } });
     issuesHandler.setCurrentUser(currentUser);
 
     renderIssueApp(currentUser);
@@ -237,7 +238,7 @@ describe('issues app filtering', () => {
 
   it('should allow to only show my issues', async () => {
     const user = userEvent.setup();
-    const currentUser = mockLoggedInUser();
+    const currentUser = mockLoggedInUser({ dismissedNotices: { [NoticeType.ISSUE_GUIDE]: true } });
     issuesHandler.setCurrentUser(currentUser);
     renderIssueApp(currentUser);
     await waitOnDataLoaded();
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 79845e3c7d3..2c74b149a70 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
@@ -38,7 +38,7 @@ import {
 
 jest.mock('../sidebar/Sidebar', () => {
   const fakeSidebar = () => {
-    return <div>Sidebar</div>;
+    return <div data-guiding-id="issue-5" />;
   };
   return {
     __esModule: true,
@@ -47,6 +47,18 @@ jest.mock('../sidebar/Sidebar', () => {
   };
 });
 
+jest.mock('../../../components/common/ScreenPositionHelper', () => ({
+  __esModule: true,
+  default: class ScreenPositionHelper extends React.Component<{
+    children: (args: { top: number }) => React.ReactNode;
+  }> {
+    render() {
+      // eslint-disable-next-line testing-library/no-node-access
+      return this.props.children({ top: 10 });
+    }
+  },
+}));
+
 beforeEach(() => {
   issuesHandler.reset();
   componentsHandler.reset();
@@ -79,7 +91,9 @@ describe('issues app', () => {
       renderIssueApp();
 
       // Navigate to 2nd issue
-      await user.keyboard('{ArrowDown}');
+      await act(async () => {
+        await user.keyboard('{ArrowDown}');
+      });
 
       // Select it
       await act(async () => {
@@ -235,7 +249,10 @@ describe('issues app', () => {
       expect(await ui.issueItems.findAll()).toHaveLength(7);
       expect(ui.issueItem8.query()).not.toBeInTheDocument();
 
-      await user.click(screen.getByRole('button', { name: 'show_more' }));
+      await act(async () => {
+        await user.click(screen.getByRole('button', { name: 'show_more' }));
+      });
+
       expect(ui.issueItems.getAll()).toHaveLength(10);
       expect(ui.issueItem8.get()).toBeInTheDocument();
     });
@@ -250,7 +267,11 @@ describe('issues app', () => {
 
       // Check that the bulk button has correct behavior
       expect(screen.getByRole('button', { name: 'bulk_change' })).toBeDisabled();
-      await user.click(screen.getByRole('checkbox', { name: 'issues.select_all_issues' }));
+
+      await act(async () => {
+        await user.click(screen.getByRole('checkbox', { name: 'issues.select_all_issues' }));
+      });
+
       expect(
         screen.getByRole('button', { name: 'issues.bulk_change_X_issues.10' })
       ).toBeInTheDocument();
@@ -477,7 +498,9 @@ describe('issues item', () => {
     const user = userEvent.setup();
     renderIssueApp();
 
-    await user.click(await ui.issueItem4.find());
+    await act(async () => {
+      await user.click(await ui.issueItem4.find());
+    });
 
     expect(
       screen.queryByRole('button', {
@@ -551,7 +574,9 @@ describe('issues item', () => {
     renderIssueApp();
 
     // Select an issue with an advanced rule
-    await user.click(await ui.issueItem5.find());
+    await act(async () => {
+      await user.click(await ui.issueItem5.find());
+    });
 
     // open status popup on key press 'f'
     await user.keyboard('f');
@@ -647,7 +672,7 @@ describe('redirects', () => {
   });
 });
 
-describe('Activity', () => {
+describe('activity', () => {
   it('should be able to add or update comment', async () => {
     const user = userEvent.setup();
     issuesHandler.setIsAdmin(true);
@@ -739,6 +764,19 @@ describe('guide', () => {
 
     expect(await ui.guidePopup.find()).toBeInTheDocument();
 
+    expect(await ui.guidePopup.find()).toBeInTheDocument();
+    expect(ui.guidePopup.get()).toHaveTextContent('guiding.issue_list.1.title');
+    expect(ui.guidePopup.get()).toHaveTextContent('guiding.issue_list.1.content');
+    expect(ui.guidePopup.get()).toHaveTextContent('guiding.step_x_of_y.1.5');
+
+    await user.click(ui.guidePopup.byRole('button', { name: 'next' }).get());
+
+    expect(ui.guidePopup.get()).toHaveTextContent('guiding.issue_list.2.title');
+    expect(ui.guidePopup.get()).toHaveTextContent('guiding.issue_list.2.content');
+    expect(ui.guidePopup.get()).toHaveTextContent('guiding.step_x_of_y.2.5');
+
+    await user.click(ui.guidePopup.byRole('button', { name: 'next' }).get());
+
     expect(ui.guidePopup.get()).toHaveTextContent('guiding.issue_list.3.title');
     expect(ui.guidePopup.get()).toHaveTextContent('guiding.issue_list.3.content');
     expect(ui.guidePopup.get()).toHaveTextContent('guiding.step_x_of_y.3.5');
@@ -762,7 +800,7 @@ describe('guide', () => {
     expect(ui.guidePopup.query()).not.toBeInTheDocument();
   });
 
-  it('should not show Guide for those who dismissed it', async () => {
+  it('should not show guide for those who dismissed it', async () => {
     renderIssueApp(
       mockCurrentUser({ isLoggedIn: true, dismissedNotices: { [NoticeType.ISSUE_GUIDE]: true } })
     );
@@ -776,8 +814,8 @@ describe('guide', () => {
     renderIssueApp(mockCurrentUser({ isLoggedIn: true }));
 
     expect(await ui.guidePopup.find()).toBeInTheDocument();
-    expect(ui.guidePopup.get()).toHaveTextContent('guiding.issue_list.3.title');
-    expect(ui.guidePopup.get()).toHaveTextContent('guiding.step_x_of_y.3.5');
+    expect(ui.guidePopup.get()).toHaveTextContent('guiding.issue_list.1.title');
+    expect(ui.guidePopup.get()).toHaveTextContent('guiding.step_x_of_y.1.5');
 
     await user.click(ui.guidePopup.byRole('button', { name: 'skip' }).get());
 
diff --git a/server/sonar-web/src/main/js/apps/issues/components/IssueGuide.tsx b/server/sonar-web/src/main/js/apps/issues/components/IssueGuide.tsx
index 438394a24fe..2d277d0c49b 100644
--- a/server/sonar-web/src/main/js/apps/issues/components/IssueGuide.tsx
+++ b/server/sonar-web/src/main/js/apps/issues/components/IssueGuide.tsx
@@ -17,24 +17,49 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
+import { SpotlightTour, SpotlightTourStep } from 'design-system';
 import React from 'react';
 import { FormattedMessage } from 'react-intl';
 import { dismissNotice } from '../../../api/users';
 import { CurrentUserContext } from '../../../app/components/current-user/CurrentUserContext';
 import DocLink from '../../../components/common/DocLink';
-import { Guide } from '../../../components/common/Guide';
-import { translate } from '../../../helpers/l10n';
+import { SCREEN_POSITION_COMPUTE_DELAY } from '../../../components/common/ScreenPositionHelper';
+import { translate, translateWithParameters } from '../../../helpers/l10n';
 import { NoticeType } from '../../../types/users';
 
 interface Props {
   run?: boolean;
-  bottom?: boolean;
 }
 
-export default function IssueGuide({ run, bottom }: Props) {
+const PLACEMENT_RIGHT = 'right';
+
+const EXTRA_DELAY = 50;
+
+export default function IssueGuide({ run }: Props) {
   const { currentUser, updateDismissedNotices } = React.useContext(CurrentUserContext);
+  const canRun = currentUser.isLoggedIn && !currentUser.dismissedNotices[NoticeType.ISSUE_GUIDE];
+
+  // IssueGuide can be called within context of a ScreenPositionHelper. When this happens,
+  // React Floater (a lib used by React Joyride, which in turn is what powers SpotlightTour)
+  // gets confused and cannot correctly position the first step. The only way around this is
+  // to delay the rendering of the SpotlightTour until *after* ScreenPositionHelper has
+  // recomputed its positioning. That's what this state + effect is about: if `run` is false,
+  // it means we are not in a state to start running. This could either be because we really don't
+  // want to start the tour at all, in which case `run` will remain false. OR, it means we are
+  // waiting on something else (like ScreenPositionHelper), in which case `run` will turn true
+  // later. We wait on the delay of ScreenPositionHelper + 50ms, and try again. If `run` is still
+  // false, we don't start the tour. If `run` is now true, we start the tour.
+  const [start, setStart] = React.useState(run);
+  React.useEffect(() => {
+    // Only trigger the timeout if start is false.
+    if (!start && canRun) {
+      setTimeout(() => {
+        setStart(run);
+      }, SCREEN_POSITION_COMPUTE_DELAY + EXTRA_DELAY);
+    }
+  }, [canRun, run, start, setStart]);
 
-  if (!currentUser.isLoggedIn || currentUser.dismissedNotices[NoticeType.ISSUE_GUIDE]) {
+  if (!start || !canRun) {
     return null;
   }
 
@@ -64,34 +89,22 @@ export default function IssueGuide({ run, bottom }: Props) {
     </>
   );
 
-  const commonStepProps = {
-    disableScrolling: true,
-    disableBeacon: true,
-    floaterProps: {
-      disableAnimation: true,
-    },
-  };
-
-  const steps = [
+  const steps: SpotlightTourStep[] = [
     {
       target: '[data-guiding-id="issue-1"]',
       content: constructContent('guiding.issue_list.1.content.1', 'guiding.issue_list.1.content.2'),
       title: translate('guiding.issue_list.1.title'),
-      placement: bottom ? ('bottom' as const) : ('right' as const),
-      ...commonStepProps,
+      placement: PLACEMENT_RIGHT,
     },
     {
       target: '[data-guiding-id="issue-2"]',
       content: constructContent('guiding.issue_list.2.content.1', 'guiding.issue_list.2.content.2'),
       title: translate('guiding.issue_list.2.title'),
-      placement: bottom ? ('bottom' as const) : ('right' as const),
-      ...commonStepProps,
     },
     {
       target: '[data-guiding-id="issue-3"]',
       content: constructContent('guiding.issue_list.3.content.1', 'guiding.issue_list.3.content.2'),
       title: translate('guiding.issue_list.3.title'),
-      ...commonStepProps,
     },
     {
       target: '[data-guiding-id="issue-4"]',
@@ -105,10 +118,9 @@ export default function IssueGuide({ run, bottom }: Props) {
         </ul>
       ),
       title: translate('guiding.issue_list.4.title'),
-      ...commonStepProps,
     },
     {
-      target: 'body',
+      target: '[data-guiding-id="issue-5"]',
       content: (
         <FormattedMessage
           id="guiding.issue_list.5.content"
@@ -123,10 +135,20 @@ export default function IssueGuide({ run, bottom }: Props) {
         />
       ),
       title: translate('guiding.issue_list.5.title'),
-      placement: 'center' as const,
-      ...commonStepProps,
     },
   ];
 
-  return <Guide callback={onToggle} steps={steps} run={run} continuous />;
+  return (
+    <SpotlightTour
+      callback={onToggle}
+      steps={steps}
+      run={run}
+      continuous
+      skipLabel={translate('skip')}
+      backLabel={translate('go_back')}
+      closeLabel={translate('close')}
+      nextLabel={translate('next')}
+      stepXofYLabel={(x: number, y: number) => translateWithParameters('guiding.step_x_of_y', x, y)}
+    />
+  );
 }
diff --git a/server/sonar-web/src/main/js/apps/issues/components/IssueHeader.tsx b/server/sonar-web/src/main/js/apps/issues/components/IssueHeader.tsx
index ec36bbd01a8..95aeb66c064 100644
--- a/server/sonar-web/src/main/js/apps/issues/components/IssueHeader.tsx
+++ b/server/sonar-web/src/main/js/apps/issues/components/IssueHeader.tsx
@@ -170,12 +170,10 @@ export default class IssueHeader extends React.PureComponent<Props, State> {
     return (
       <header className="sw-flex sw-mb-6">
         <div className="sw-mr-8 sw-flex-1">
-          <div data-guiding-id="issue-1" className="sw-w-fit">
-            <CleanCodeAttributePill
-              cleanCodeAttributeCategory={issue.cleanCodeAttributeCategory}
-              cleanCodeAttribute={issue.cleanCodeAttribute}
-            />
-          </div>
+          <CleanCodeAttributePill
+            cleanCodeAttributeCategory={issue.cleanCodeAttributeCategory}
+            cleanCodeAttribute={issue.cleanCodeAttribute}
+          />
 
           <div className="sw-flex sw-items-center sw-my-2">
             <PageContentFontWrapper className="sw-body-md-highlight" as="h1">
diff --git a/server/sonar-web/src/main/js/apps/issues/sidebar/AttributeCategoryFacet.tsx b/server/sonar-web/src/main/js/apps/issues/sidebar/AttributeCategoryFacet.tsx
index e9a92e2469a..4797ecb36ad 100644
--- a/server/sonar-web/src/main/js/apps/issues/sidebar/AttributeCategoryFacet.tsx
+++ b/server/sonar-web/src/main/js/apps/issues/sidebar/AttributeCategoryFacet.tsx
@@ -32,14 +32,12 @@ export function AttributeCategoryFacet(props: Props) {
   const { categories = [], ...rest } = props;
 
   return (
-    <div data-guiding-id="issue-1">
-      <SimpleListStyleFacet
-        property="cleanCodeAttributeCategory"
-        itemNamePrefix="issue.clean_code_attribute_category"
-        listItems={CATEGORIES}
-        selectedItems={categories}
-        {...rest}
-      />
-    </div>
+    <SimpleListStyleFacet
+      property="cleanCodeAttributeCategory"
+      itemNamePrefix="issue.clean_code_attribute_category"
+      listItems={CATEGORIES}
+      selectedItems={categories}
+      {...rest}
+    />
   );
 }
diff --git a/server/sonar-web/src/main/js/apps/issues/sidebar/SoftwareQualityFacet.tsx b/server/sonar-web/src/main/js/apps/issues/sidebar/SoftwareQualityFacet.tsx
index 92e47f10a79..2851cc06a60 100644
--- a/server/sonar-web/src/main/js/apps/issues/sidebar/SoftwareQualityFacet.tsx
+++ b/server/sonar-web/src/main/js/apps/issues/sidebar/SoftwareQualityFacet.tsx
@@ -32,14 +32,12 @@ export function SoftwareQualityFacet(props: Props) {
   const { qualities = [], ...rest } = props;
 
   return (
-    <div data-guiding-id="issue-2">
-      <SimpleListStyleFacet
-        property="impactSoftwareQuality"
-        itemNamePrefix="issue.software_quality"
-        listItems={QUALITIES}
-        selectedItems={qualities}
-        {...rest}
-      />
-    </div>
+    <SimpleListStyleFacet
+      property="impactSoftwareQuality"
+      itemNamePrefix="issue.software_quality"
+      listItems={QUALITIES}
+      selectedItems={qualities}
+      {...rest}
+    />
   );
 }
diff --git a/server/sonar-web/src/main/js/apps/issues/test-utils.tsx b/server/sonar-web/src/main/js/apps/issues/test-utils.tsx
index 3308b61f4e6..6fba1751ba3 100644
--- a/server/sonar-web/src/main/js/apps/issues/test-utils.tsx
+++ b/server/sonar-web/src/main/js/apps/issues/test-utils.tsx
@@ -19,6 +19,7 @@
  */
 import { waitFor } from '@testing-library/react';
 import React from 'react';
+import { Outlet, Route } from 'react-router-dom';
 import BranchesServiceMock from '../../api/mocks/BranchesServiceMock';
 import ComponentsServiceMock from '../../api/mocks/ComponentsServiceMock';
 import IssuesServiceMock from '../../api/mocks/IssuesServiceMock';
@@ -32,7 +33,7 @@ import {
   SoftwareQuality,
 } from '../../types/issues';
 import { Component } from '../../types/types';
-import { CurrentUser } from '../../types/users';
+import { NoticeType } from '../../types/users';
 import IssuesApp from './components/IssuesApp';
 import { projectIssuesRoutes } from './routes';
 
@@ -137,19 +138,31 @@ export async function waitOnDataLoaded() {
   });
 }
 
-export function renderIssueApp(currentUser?: CurrentUser) {
-  renderApp('issues', <IssuesApp />, { currentUser: mockCurrentUser(currentUser) });
+export function renderIssueApp(
+  currentUser = mockCurrentUser({ dismissedNotices: { [NoticeType.ISSUE_GUIDE]: true } })
+) {
+  renderApp('project/issues', <IssuesApp />, { currentUser });
 }
 
 export function renderProjectIssuesApp(
   navigateTo?: string,
   overrides?: Partial<Component>,
-  currentUser?: CurrentUser
+  currentUser = mockCurrentUser({ dismissedNotices: { [NoticeType.ISSUE_GUIDE]: true } })
 ) {
   renderAppWithComponentContext(
     'project/issues',
-    projectIssuesRoutes,
-    { navigateTo, currentUser: mockCurrentUser(currentUser) },
+    () => (
+      <Route
+        element={
+          <div data-guiding-id="issue-5">
+            <Outlet />
+          </div>
+        }
+      >
+        {projectIssuesRoutes()}
+      </Route>
+    ),
+    { navigateTo, currentUser },
     { component: mockComponent(overrides) }
   );
 }