From 3940b153fe70a090dd1bc6c44ab3c43e2727cbb1 Mon Sep 17 00:00:00 2001
From: stanislavh <stanislav.honcharov@sonarsource.com>
Date: Mon, 16 Jan 2023 12:19:52 +0100
Subject: [PATCH] SONAR-18147 Status message not automatically announced

---
 .../src/main/js/app/styles/init/misc.css      | 12 ++---
 .../changelog/ChangelogContainer.tsx          |  3 +-
 .../ChangelogContainer-test.tsx.snap          |  8 ++++
 .../__tests__/ActivityGraph-it.tsx            |  2 +-
 .../js/components/controls/ListFooter.tsx     |  2 +-
 .../controls/__tests__/Checkbox-test.tsx      |  2 +-
 .../__snapshots__/Checkbox-test.tsx.snap      | 40 +++++++++-------
 .../__snapshots__/ListFooter-test.tsx.snap    | 20 ++++++++
 .../main/js/components/ui/DeferredSpinner.tsx | 47 +++++++++++--------
 .../ui/__tests__/DeferredSpinner-test.tsx     | 15 ++----
 10 files changed, 95 insertions(+), 56 deletions(-)

diff --git a/server/sonar-web/src/main/js/app/styles/init/misc.css b/server/sonar-web/src/main/js/app/styles/init/misc.css
index 9441ebbaab4..526a667115d 100644
--- a/server/sonar-web/src/main/js/app/styles/init/misc.css
+++ b/server/sonar-web/src/main/js/app/styles/init/misc.css
@@ -40,12 +40,12 @@ th.hide-overflow {
 }
 
 .a11y-hidden {
-  position: absolute;
-  left: -10000px;
-  top: auto;
-  width: 1px;
-  height: 1px;
-  overflow: hidden;
+  position: absolute !important;
+  left: -10000px !important;
+  top: auto !important;
+  width: 1px !important;
+  height: 1px !important;
+  overflow: hidden !important;
 }
 
 .invisible {
diff --git a/server/sonar-web/src/main/js/apps/quality-profiles/changelog/ChangelogContainer.tsx b/server/sonar-web/src/main/js/apps/quality-profiles/changelog/ChangelogContainer.tsx
index 725d459607d..17f0cf3ce28 100644
--- a/server/sonar-web/src/main/js/apps/quality-profiles/changelog/ChangelogContainer.tsx
+++ b/server/sonar-web/src/main/js/apps/quality-profiles/changelog/ChangelogContainer.tsx
@@ -20,6 +20,7 @@
 import * as React from 'react';
 import { getProfileChangelog } from '../../../api/quality-profiles';
 import { Location, Router, withRouter } from '../../../components/hoc/withRouter';
+import DeferredSpinner from '../../../components/ui/DeferredSpinner';
 import { parseDate, toShortNotSoISOString } from '../../../helpers/dates';
 import { translate } from '../../../helpers/l10n';
 import { withQualityProfilesContext } from '../qualityProfilesContext';
@@ -147,7 +148,7 @@ export class ChangelogContainer extends React.PureComponent<Props, State> {
             onReset={this.handleReset}
           />
 
-          {this.state.loading && <i className="spinner spacer-left" />}
+          <DeferredSpinner loading={this.state.loading} className="spacer-left" />
         </header>
 
         {this.state.events != null && this.state.events.length === 0 && <ChangelogEmpty />}
diff --git a/server/sonar-web/src/main/js/apps/quality-profiles/changelog/__tests__/__snapshots__/ChangelogContainer-test.tsx.snap b/server/sonar-web/src/main/js/apps/quality-profiles/changelog/__tests__/__snapshots__/ChangelogContainer-test.tsx.snap
index e7347b5911f..a42e82c92c5 100644
--- a/server/sonar-web/src/main/js/apps/quality-profiles/changelog/__tests__/__snapshots__/ChangelogContainer-test.tsx.snap
+++ b/server/sonar-web/src/main/js/apps/quality-profiles/changelog/__tests__/__snapshots__/ChangelogContainer-test.tsx.snap
@@ -17,6 +17,10 @@ exports[`should render correctly 1`] = `
       onDateRangeChange={[Function]}
       onReset={[Function]}
     />
+    <DeferredSpinner
+      className="spacer-left"
+      loading={false}
+    />
   </header>
   <Changelog
     events={
@@ -81,6 +85,10 @@ exports[`should render correctly without events 1`] = `
       onDateRangeChange={[Function]}
       onReset={[Function]}
     />
+    <DeferredSpinner
+      className="spacer-left"
+      loading={false}
+    />
   </header>
   <ChangelogEmpty />
 </div>
diff --git a/server/sonar-web/src/main/js/components/activity-graph/__tests__/ActivityGraph-it.tsx b/server/sonar-web/src/main/js/components/activity-graph/__tests__/ActivityGraph-it.tsx
index 3bd02af9ed6..4b9ac8eb468 100644
--- a/server/sonar-web/src/main/js/components/activity-graph/__tests__/ActivityGraph-it.tsx
+++ b/server/sonar-web/src/main/js/components/activity-graph/__tests__/ActivityGraph-it.tsx
@@ -42,7 +42,7 @@ const START_DATE = '2016-01-01T00:00:00+0200';
 
 it('should render correctly when loading', async () => {
   renderActivityGraph({ loading: true });
-  expect(await screen.findByLabelText('loading')).toBeInTheDocument();
+  expect(await screen.findByText('loading')).toBeInTheDocument();
 });
 
 it('should show the correct legend items', async () => {
diff --git a/server/sonar-web/src/main/js/components/controls/ListFooter.tsx b/server/sonar-web/src/main/js/components/controls/ListFooter.tsx
index 30ae1741a89..e5ada28e176 100644
--- a/server/sonar-web/src/main/js/components/controls/ListFooter.tsx
+++ b/server/sonar-web/src/main/js/components/controls/ListFooter.tsx
@@ -110,7 +110,7 @@ export default function ListFooter(props: ListFooterProps) {
           : translateWithParameters('x_show', formatMeasure(count, 'INT', null))}
       </span>
       {button}
-      {loading && <DeferredSpinner className="text-bottom spacer-left position-absolute" />}
+      {<DeferredSpinner loading={loading} className="text-bottom spacer-left position-absolute" />}
     </div>
   );
 }
diff --git a/server/sonar-web/src/main/js/components/controls/__tests__/Checkbox-test.tsx b/server/sonar-web/src/main/js/components/controls/__tests__/Checkbox-test.tsx
index 18231d65295..6d1ab824f32 100644
--- a/server/sonar-web/src/main/js/components/controls/__tests__/Checkbox-test.tsx
+++ b/server/sonar-web/src/main/js/components/controls/__tests__/Checkbox-test.tsx
@@ -55,7 +55,7 @@ describe.each([
     jest.useFakeTimers();
     renderCheckbox({ label: 'me', children, loading: true });
     jest.runAllTimers();
-    expect(screen.getByLabelText('me')).toMatchSnapshot();
+    expect(screen.getByTestId('deferred-spinner')).toMatchSnapshot();
     jest.useRealTimers();
   });
 });
diff --git a/server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/Checkbox-test.tsx.snap b/server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/Checkbox-test.tsx.snap
index 30df5c3539c..ea68408df35 100644
--- a/server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/Checkbox-test.tsx.snap
+++ b/server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/Checkbox-test.tsx.snap
@@ -1,28 +1,31 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`Checkbox with children should render loading state 1`] = `
-<a
-  aria-checked="true"
-  aria-label="me"
-  class="link-checkbox"
-  href="#"
-  role="checkbox"
+<i
+  aria-live="polite"
+  class="deferred-spinner is-loading"
+  data-testid="deferred-spinner"
 >
-  <i
-    class="deferred-spinner"
-  />
-  <a>
-    child
-  </a>
-</a>
+  <span
+    class="a11y-hidden"
+  >
+    loading
+  </span>
+</i>
 `;
 
 exports[`Checkbox with no children should render loading state 1`] = `
 <i
-  aria-label="me"
   aria-live="polite"
-  class="deferred-spinner"
-/>
+  class="deferred-spinner is-loading"
+  data-testid="deferred-spinner"
+>
+  <span
+    class="a11y-hidden"
+  >
+    me
+  </span>
+</i>
 `;
 
 exports[`should render the checkbox on the right 1`] = `
@@ -36,6 +39,11 @@ exports[`should render the checkbox on the right 1`] = `
   <a>
     child
   </a>
+  <i
+    aria-live="polite"
+    class="deferred-spinner a11y-hidden"
+    data-testid="deferred-spinner"
+  />
   <i
     class="icon-checkbox icon-checkbox-checked"
   />
diff --git a/server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/ListFooter-test.tsx.snap b/server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/ListFooter-test.tsx.snap
index d16290f172b..0bd10c7be1d 100644
--- a/server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/ListFooter-test.tsx.snap
+++ b/server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/ListFooter-test.tsx.snap
@@ -17,6 +17,9 @@ exports[`should render correctly: default 1`] = `
   >
     show_more
   </Button>
+  <DeferredSpinner
+    className="text-bottom spacer-left position-absolute"
+  />
 </div>
 `;
 
@@ -30,6 +33,9 @@ exports[`should render correctly: empty if everything is loaded 1`] = `
   >
     x_of_y_shown.5.5
   </span>
+  <DeferredSpinner
+    className="text-bottom spacer-left position-absolute"
+  />
 </div>
 `;
 
@@ -43,6 +49,9 @@ exports[`should render correctly: empty if no loadMore nor reload props 1`] = `
   >
     x_of_y_shown.3.5
   </span>
+  <DeferredSpinner
+    className="text-bottom spacer-left position-absolute"
+  />
 </div>
 `;
 
@@ -63,6 +72,9 @@ exports[`should render correctly: force show load more button if count % pageSiz
   >
     show_more
   </Button>
+  <DeferredSpinner
+    className="text-bottom spacer-left position-absolute"
+  />
 </div>
 `;
 
@@ -87,6 +99,7 @@ exports[`should render correctly: loading 1`] = `
   </Button>
   <DeferredSpinner
     className="text-bottom spacer-left position-absolute"
+    loading={true}
   />
 </div>
 `;
@@ -108,6 +121,9 @@ exports[`should render correctly: reload 1`] = `
   >
     reload
   </Button>
+  <DeferredSpinner
+    className="text-bottom spacer-left position-absolute"
+  />
 </div>
 `;
 
@@ -132,6 +148,7 @@ exports[`should render correctly: reload, loading 1`] = `
   </Button>
   <DeferredSpinner
     className="text-bottom spacer-left position-absolute"
+    loading={true}
   />
 </div>
 `;
@@ -146,5 +163,8 @@ exports[`should render correctly: total undefined 1`] = `
   >
     x_show.3
   </span>
+  <DeferredSpinner
+    className="text-bottom spacer-left position-absolute"
+  />
 </div>
 `;
diff --git a/server/sonar-web/src/main/js/components/ui/DeferredSpinner.tsx b/server/sonar-web/src/main/js/components/ui/DeferredSpinner.tsx
index 7a0136fc342..5b9b8550be9 100644
--- a/server/sonar-web/src/main/js/components/ui/DeferredSpinner.tsx
+++ b/server/sonar-web/src/main/js/components/ui/DeferredSpinner.tsx
@@ -19,6 +19,7 @@
  */
 import classNames from 'classnames';
 import * as React from 'react';
+import { translate } from '../../helpers/l10n';
 import './DeferredSpinner.css';
 
 interface Props {
@@ -27,7 +28,6 @@ interface Props {
   className?: string;
   customSpinner?: JSX.Element;
   loading?: boolean;
-  placeholder?: boolean;
   timeout?: number;
 }
 
@@ -37,6 +37,14 @@ interface State {
 
 const DEFAULT_TIMEOUT = 100;
 
+/**
+ * Recommendation: do not render this component conditionally based on any loading state:
+ *   // Don't do this:
+ *   {loading && <DeferredSpinner />}
+ * Instead, pass the loading state as a prop:
+ *   // Do this:
+ *   <DeferredSpinner loading={loading} />
+ */
 export default class DeferredSpinner extends React.PureComponent<Props, State> {
   timer?: number;
 
@@ -75,28 +83,27 @@ export default class DeferredSpinner extends React.PureComponent<Props, State> {
   };
 
   render() {
-    const { ariaLabel, children, className, customSpinner, placeholder } = this.props;
-    if (this.state.showSpinner) {
-      return (
-        customSpinner || (
-          <i
-            className={classNames('deferred-spinner', className)}
-            aria-live={ariaLabel ? 'polite' : undefined}
-            aria-label={ariaLabel}
-          />
-        )
-      );
+    const { ariaLabel = translate('loading'), children, className, customSpinner } = this.props;
+    const { showSpinner } = this.state;
+
+    if (customSpinner) {
+      return showSpinner ? customSpinner : children;
     }
+
     return (
-      children ||
-      (placeholder ? (
+      <>
         <i
-          data-testid="deferred-spinner-placeholder"
-          className={classNames('deferred-spinner-placeholder', className)}
-          aria-live={ariaLabel ? 'polite' : undefined}
-          aria-label={ariaLabel}
-        />
-      ) : null)
+          aria-live="polite"
+          data-testid="deferred-spinner"
+          className={classNames('deferred-spinner', className, {
+            'a11y-hidden': !showSpinner,
+            'is-loading': showSpinner,
+          })}
+        >
+          {showSpinner && <span className="a11y-hidden">{ariaLabel}</span>}
+        </i>
+        {!showSpinner && children}
+      </>
     );
   }
 }
diff --git a/server/sonar-web/src/main/js/components/ui/__tests__/DeferredSpinner-test.tsx b/server/sonar-web/src/main/js/components/ui/__tests__/DeferredSpinner-test.tsx
index 023e3c8ee47..6c7909d2523 100644
--- a/server/sonar-web/src/main/js/components/ui/__tests__/DeferredSpinner-test.tsx
+++ b/server/sonar-web/src/main/js/components/ui/__tests__/DeferredSpinner-test.tsx
@@ -42,29 +42,24 @@ it('renders children before timeout', () => {
 
 it('renders spinner after timeout', () => {
   renderDeferredSpinner();
-  expect(screen.queryByLabelText('loading')).not.toBeInTheDocument();
+  expect(screen.queryByText('loading')).not.toBeInTheDocument();
   jest.runAllTimers();
-  expect(screen.getByLabelText('loading')).toBeInTheDocument();
-});
-
-it('renders a placeholder while waiting', () => {
-  renderDeferredSpinner({ placeholder: true });
-  expect(screen.getByTestId('deferred-spinner-placeholder')).toBeInTheDocument();
+  expect(screen.getByText('loading')).toBeInTheDocument();
 });
 
 it('allows setting a custom class name', () => {
   renderDeferredSpinner({ className: 'foo' });
   jest.runAllTimers();
-  expect(screen.getByLabelText('loading')).toHaveClass('foo');
+  expect(screen.getByTestId('deferred-spinner')).toHaveClass('foo');
 });
 
 it('can be controlled by the loading prop', () => {
   const { rerender } = renderDeferredSpinner({ loading: true });
   jest.runAllTimers();
-  expect(screen.getByLabelText('loading')).toBeInTheDocument();
+  expect(screen.getByText('loading')).toBeInTheDocument();
 
   rerender(prepareDeferredSpinner({ loading: false }));
-  expect(screen.queryByLabelText('loading')).not.toBeInTheDocument();
+  expect(screen.queryByText('loading')).not.toBeInTheDocument();
 });
 
 function renderDeferredSpinner(props: Partial<DeferredSpinner['props']> = {}) {
-- 
2.39.5