]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-18147 Status message not automatically announced
authorstanislavh <stanislav.honcharov@sonarsource.com>
Mon, 16 Jan 2023 11:19:52 +0000 (12:19 +0100)
committersonartech <sonartech@sonarsource.com>
Mon, 16 Jan 2023 20:03:43 +0000 (20:03 +0000)
server/sonar-web/src/main/js/app/styles/init/misc.css
server/sonar-web/src/main/js/apps/quality-profiles/changelog/ChangelogContainer.tsx
server/sonar-web/src/main/js/apps/quality-profiles/changelog/__tests__/__snapshots__/ChangelogContainer-test.tsx.snap
server/sonar-web/src/main/js/components/activity-graph/__tests__/ActivityGraph-it.tsx
server/sonar-web/src/main/js/components/controls/ListFooter.tsx
server/sonar-web/src/main/js/components/controls/__tests__/Checkbox-test.tsx
server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/Checkbox-test.tsx.snap
server/sonar-web/src/main/js/components/controls/__tests__/__snapshots__/ListFooter-test.tsx.snap
server/sonar-web/src/main/js/components/ui/DeferredSpinner.tsx
server/sonar-web/src/main/js/components/ui/__tests__/DeferredSpinner-test.tsx

index 9441ebbaab497ca97c00ebdf1948c9b69120a54c..526a667115de9bd50953a24a459616034122ce3b 100644 (file)
@@ -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 {
index 725d459607d44b45d028a5cd2bdca768f2e3a727..17f0cf3ce2865630b07ae92449f29d18aff4b0f1 100644 (file)
@@ -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 />}
index e7347b5911f6689f6f2ee631bdccdd2dffd43306..a42e82c92c5a14a5d130cd3fb309001cf9d03da1 100644 (file)
@@ -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>
index 3bd02af9ed6ed0ae08886128207a589b706aeaf9..4b9ac8eb46859f3f027e1574ecd4b28b3eeb8b77 100644 (file)
@@ -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 () => {
index 30ae1741a89d5c3ee8e0ca97d2faf13890b73b43..e5ada28e17672c7aeeacc16376a837a6311160b7 100644 (file)
@@ -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>
   );
 }
index 18231d65295e85176c8d47a853a2a0349afcb0fa..6d1ab824f32d19cf8333132fe61e79e6580911e9 100644 (file)
@@ -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();
   });
 });
index 30df5c3539c56d1e9171e14da554c4710d5f83e1..ea68408df358fe0b196cf16ca7c478fde971369b 100644 (file)
@@ -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"
   />
index d16290f172b255e92ef6b1e4377ed019084cd418..0bd10c7be1d0f602b07f3e5af972a4e97442961f 100644 (file)
@@ -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>
 `;
index 7a0136fc3425be02a73ba0adad786bb7e8892768..5b9b8550be91a99b4e2344a08805ef30442d93c6 100644 (file)
@@ -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}
+      </>
     );
   }
 }
index 023e3c8ee47f4157f01e600bdef74cb04deb8851..6c7909d2523f4f05a854e29cb86e6ad576992bdb 100644 (file)
@@ -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']> = {}) {