Merge branch 'dev' into fix-checkbox-skeleton

.github/CONTRIBUTING.md

@@ -42,6 +42,8 @@ Once it's done building, you can start editing source code or creating new compo
 
 ## 5. Test your JavaScript code
 
+Please review the [testing guidelines](./docs/testing) for details on how and what to test.
+
 If you're contributing to our JavaScript code, test your changes by running our test command:
 
 ```bash
@@ -52,8 +54,9 @@ npm test
 yarn test
 ```
 
-If you add any features to our JavaScript code, make sure to add tests so that
-your code is covered. Tests are written in
+New tests are written using [React Testing Library](https://testing-library.com/).
+
+Legacy tests are written in
 [Jest](https://jestjs.io) / [Enzyme](https://airbnb.io/enzyme). You can see if your code
 is covered by looking at `coverage/lcov-report/*/index.html` after
 running test.

docs/testing/README.md

@@ -0,0 +1,189 @@
+# Testing
+
+## How to test
+
+Follow the [Guiding Principles of React Testing Library](https://github.com/testing-library/react-testing-library#guiding-principles) when writing tests.
+
+## What to test
+
+### Accessibility
+
+Automated accessibility tests are required for significant component variations and should use existing `toHaveNoAxeViolations()` and `toHaveNoDAPViolations()` Jest matchers.
+
+Use this structure for automated accessibility tests:
+
+```jsx
+test('should have no Axe or DAP violations', async () => {
+  const main = document.createElement('main');
+  render(<Component />, {
+    // DAP requires a landmark '<main>' in the DOM:
+    container: document.body.appendChild(main),
+  });
+
+  await expect(document.body).toHaveNoAxeViolations();
+  await expect(document.body).toHaveNoDAPViolations('ComponentName');
+});
+
+test('should have no Axe or DAP violations with component variation', async () => {
+  const main = document.createElement('main');
+  render(<Component variationProp={value} />, {
+    // DAP requires a landmark '<main>' in the DOM:
+    container: document.body.appendChild(main),
+  });
+
+  await expect(document.body).toHaveNoAxeViolations();
+  await expect(document.body).toHaveNoDAPViolations(
+    'ComponentName with variation'
+  );
+});
+```
+
+#### Test structure
+
+- Each significant variation of a component should have its own test block.
+- Use an **asynchronous** test block, and **await** each accessibility rule matcher.
+
+#### DAP requirements
+
+- DAP requires that components be rendered inside a valid landmark element, which is why the `container` is a `main` node. This extra step may not be required if the component is already wrapped in a `main` or another significant HTML landmark element.
+- DAP requires a unique id per test within a given component. (Hence, "ComponentName" and "ComponentName with variation")
+
+## User events
+
+### Click and key press events
+
+Events (`onClick`, `onKeyPress`, etc) that are important to component functionality should be tested using mocks.
+
+```jsx
+test('should invoke close mock when close button is clicked', () => {
+  const onCloseMock = jest.fn();
+  const { getByLabelText } = render(
+    <Component labelText="test close" onClick={onCloseMock} />
+  );
+
+  userEvent.click(getByLabelText(/test close/i));
+  expect(onCloseMock).toHaveBeenCalledTimes(1);
+});
+```
+
+### Tabbing
+
+Write a series of tab cycle tests in order to verify that a user can tab through the interactive pieces of the component in a predictable way. Tab cycle tests are very useful for finding hidden "focus traps".
+
+Each tab cycle test should traverse completely through the ideal tab order, in a loop that begins and ends with the first interactive element that receives focus.
+
+Similar to accessibility tests, a tab cycle test should be written for each significant component variation, especially if a variation adds or removes interactive elements from the component.
+
+Example of a tab cycle test for `PanelV2`:
+
+```jsx
+test('should cycle panel elements in tab order', () => {
+  const { getByLabelText, getByText } = render(
+    <PanelV2
+      closeButton={{
+        label: 'test close',
+      }}
+      renderFooter={() => (<Button>test footer button</Button>)}
+    >
+      <PanelContent>
+        test content text
+        <Button>test content button</Button>
+      </PanelContent>
+    </PanelV2>
+  );
+
+  userEvent.tab();
+
+  // The close button:
+  expect(getByLabelText(/test close/i)).toHaveFocus();
+
+  userEvent.tab();
+
+  // The button inside the `PanelContent` wrapper:
+  expect(getByText(/test content button/i)).toHaveFocus();
+
+  userEvent.tab();
+
+  // The footer button:
+  expect(getByText(/test footer button/i)).toHaveFocus();
+
+  userEvent.tab();
+
+  // Loop complete.
+  // The close button:
+  expect(getByLabelText(/test close/i)).toHaveFocus();
+});
+```
+
+If the component is a single interactive element, like a single `<a>` or `<button>` then a tab cycle test may not be necessary. The `TrendingCard` is an example of a component that does not require a tab cycle test.
+
+## Props
+
+### Custom class names
+
+Ensure that custom class names are checked:
+
+```jsx
+test('should add custom class', () => {
+  const { getByText } = render(
+    <ExternalLink href="https://www.ibm.com/security" className="custom-class">
+      test link
+    </ExternalLink>
+  );
+  expect(getByText(/test link/i).closest('a')).toHaveClass('custom-class');
+});
+```
+
+### Spread attribute
+
+If a component includes a spread attribute (i.e., `...other` in the props definition), include a test to check that extra props can be passed through to the component.
+
+You can use a testing attribute like `data-testid` to test for this:
+
+```jsx
+test('should pass through extra props via spread attribute', () => {
+  const { queryByTestId } = render(
+    <ExternalLink href="https://www.ibm.com/security" data-testid="test-id">
+      test link
+    </ExternalLink>
+  );
+  expect(queryByTestId('test-id')).toBeInTheDocument();
+});
+```
+
+### Object or array of significant values
+
+If an object or array has significant values, then those values should be tested individually. "Significant values" in this case means any values that render completely different nodes, or that individually could cause an error.
+
+For example, with the `ICA`'s `locale` prop, it makes sense to verify that each possible `locale` value does not throw an error:
+
+```jsx
+// Note: Locales is an array of values.
+Locales.forEach(locale =>
+  test(`should accept '${locale}' locale`, () => {
+    const { container } = render(
+      <ICA label="test ICA" value={10} locale={locale} />
+    );
+    expect(() => container).not.toThrow();
+  })
+);
+```
+
+And in another example for the `StatusIcon`'s `status` prop, it makes sense to run an accessibility test on each type of `status` because each `status` renders completely different Nodes:
+
+```jsx
+// Note: STATUS is an array of values.
+STATUS.forEach(status =>
+  test(`should have no Axe or DAP violations when \`status\` is  '${status}'`, async () => {
+    const main = document.createElement('main');
+    render(<StatusIcon status={status} message="test message" />, {
+      // DAP requires a landmark '<main>' in the DOM:
+      container: document.body.appendChild(main),
+    });
+    await expect(document.body).toHaveNoAxeViolations();
+    await expect(document.body).toHaveNoDAPViolations(
+      `StatusIcon with ${status} status`
+    );
+  })
+);
+```

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@carbon/ibm-security",
-  "version": "1.18.1-prerelease.1",
+  "version": "1.18.1-prerelease.3",
   "description": "Carbon for IBM Security",
   "license": "Apache-2.0",
   "main": "lib/index.js",
@@ -99,9 +99,9 @@
     "@storybook/addons": "^5.2.4",
     "@storybook/react": "^5.2.4",
     "@storybook/theming": "^5.2.4",
-    "@testing-library/jest-dom": "^4.2.4",
-    "@testing-library/react": "^9.3.2",
-    "@testing-library/user-event": "^8.1.0",
+    "@testing-library/jest-dom": "4.2.4",
+    "@testing-library/react": "9.3.2",
+    "@testing-library/user-event": "8.1.0",
     "autoprefixer": "^9.4.10",
     "axe-core": "^3.4.1",
     "babel-eslint": "^10.0.1",

src/components/SummaryCard/SummaryCardAction/SummaryCardAction.js

@@ -83,6 +83,8 @@ class SummaryCardAction extends Component {
                       iconDescription={closeButtonIconDescription}
                       onClick={this.toggleOpen}
                       kind="ghost"
+                      title={closeButtonIconDescription}
+                      aria-label={closeButtonIconDescription}
                     />
                   </div>
                   <div className={`${namespace}-overlay__content`}>

src/components/SummaryCard/SummaryCardAction/__tests__/SummaryCardAction.spec.js

@@ -0,0 +1,136 @@
+/**
+ * @file Summary card tests.
+ * @copyright IBM Security 2020
+ */
+
+import { render } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import React from 'react';
+
+import { SummaryCardAction } from '../../../..';
+
+import { carbonPrefix } from '../../../../globals/namespace';
+
+describe('SummaryCardAction', () => {
+  test('should not be interactive when loading', () => {
+    const { getByText } = render(
+      <div>
+        {/* This button is NOT interactive: */}
+        <SummaryCardAction loading>test loading button</SummaryCardAction>
+        {/* This button is interactive: */}
+        <button>test example button</button>
+      </div>
+    );
+
+    // Expect button to have `disabled` attribute:
+    expect(getByText(/test loading button/i).closest('button')).toHaveAttribute(
+      'disabled'
+    );
+
+    userEvent.tab();
+
+    // Expect the loading button, listed first, not to be focussed:
+    expect(getByText(/test loading button/i)).not.toHaveFocus();
+
+    // Expect the example button, listed second, to be focussed:
+    expect(getByText(/test example button/i)).toHaveFocus();
+
+    userEvent.tab();
+
+    // Expect the example button to still be focussed:
+    expect(getByText(/test example button/i)).toHaveFocus();
+  });
+
+  test('should accept `expandedContent` and only show when activated', () => {
+    const { getByText, queryByText, getByTestId } = render(
+      <SummaryCardAction
+        closeButtonIconDescription="test close button"
+        expandedContent="test expanded action content"
+        data-testid="test-button-id"
+      >
+        test button
+      </SummaryCardAction>
+    );
+
+    // Expect expanded content to NOT be in the document:
+    expect(
+      queryByText(/test expanded action content/i)
+    ).not.toBeInTheDocument();
+
+    // Expect `aria-expanded` attribute to be `false`:
+    expect(getByTestId('test-button-id')).toHaveAttribute(
+      'aria-expanded',
+      'false'
+    );
+
+    // Click on the action button to show expanded content:
+    userEvent.click(getByText('test button'));
+
+    // Expect expanded content to be visible:
+    expect(queryByText(/test expanded action content/i)).toBeVisible();
+
+    // Expect `aria-expanded` attribute to be `true`:
+    expect(getByTestId('test-button-id')).toHaveAttribute(
+      'aria-expanded',
+      'true'
+    );
+  });
+
+  test('should accept a custom class name', () => {
+    const { container } = render(
+      <SummaryCardAction className="custom-class" />
+    );
+    expect(container.firstElementChild).toHaveClass('custom-class');
+  });
+
+  test("should add a description to the expanded content's close button via `closeButtonIconDescription`", () => {
+    const { getByText, getAllByLabelText } = render(
+      <SummaryCardAction
+        expandedContent="test content"
+        closeButtonIconDescription="test icon label"
+      >
+        test button
+      </SummaryCardAction>
+    );
+    // Must open the expanded content to see the close button:
+    userEvent.click(getByText('test button'));
+    // Use `getAll*` & check for 2 occurances because the icon button
+    // applies the label to its `title` and `aria-label` attributes:
+    expect(getAllByLabelText(/test icon label/i).length).toBe(2);
+  });
+
+  test('should accept children', () => {
+    const { queryByText } = render(
+      <SummaryCardAction>test content</SummaryCardAction>
+    );
+    expect(queryByText(/test content/i)).toBeVisible();
+  });
+
+  test('should pass through extra props via spread attribute', () => {
+    const { queryByTestId } = render(
+      <SummaryCardAction data-testid="test-id" />
+    );
+    expect(queryByTestId('test-id')).toBeVisible();
+  });
+
+  test('should apply assistive text and icon-only class name when `hasIconOnly` is `true`', () => {
+    const { queryByText } = render(
+      <SummaryCardAction iconDescription="test icon" hasIconOnly />
+    );
+    // Expect icon description to be applied as assistive text:
+    expect(queryByText(/test icon/i)).toBeVisible();
+    // Expect button to have Carbon's icon-only class:
+    expect(queryByText(/test icon/i).closest('button')).toHaveClass(
+      `${carbonPrefix}btn--icon-only`
+    );
+  });
+
+  test('should invoke click mock when button is clicked', () => {
+    const onClickMock = jest.fn();
+    const { getByText } = render(
+      <SummaryCardAction onClick={onClickMock}>test content</SummaryCardAction>
+    );
+    userEvent.click(getByText(/test content/i));
+    expect(onClickMock).toHaveBeenCalledTimes(1);
+  });
+});

src/components/SummaryCard/SummaryCardBatchActions/SummaryCardBatchActions.js

@@ -11,6 +11,11 @@ import { TableBatchActions } from '../../DataTable';
 
 import { namespace as summaryCardNamespace } from '../SummaryCard';
 
+const namespace = appendComponentNamespace(
+  summaryCardNamespace,
+  'batch-actions'
+);
+
 const {
   defaultProps: { translateWithId },
   propTypes,
@@ -25,7 +30,7 @@ const translationKeys = [
 
 const SummaryCardBatchActions = ({ translateWithId: t, ...other }) => (
   <TableBatchActions
-    className={appendComponentNamespace(summaryCardNamespace, 'batch-actions')}
+    className={namespace}
     translateWithId={(id, state) =>
       t
         ? t(translationKeys[carbonTranslationKeys.indexOf(id)], state)