Skip to content

Commit

Permalink
docs: update contributor documentation with more defined guidelines (#…
Browse files Browse the repository at this point in the history 
…1329)
  • Loading branch information
@scurker
scurker authored Feb 12, 2024
1 parent 33d6b54 commit b350c52
Showing 1 changed file with 171 additions and 28 deletions.
199 changes: 171 additions & 28 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
@@ -1,60 +1,203 @@
# Contributing

## Code Quality
1. [Getting Started](#getting-started)
- [Discussing Proprietary Features](#discussing-proprietary-features)
- [Setup](#setup)
1. [Developing Components](#developing-components)
- [Tools Used](#tools-used)
- [Structure](#structure)
- [Quality Checklist](#quality-checklist)
- [Server-side Rendering Compatibility](#server-side-rendering-compatibility)
- [Linting](#linting)
- [Icons](#icons)
- [Commit Messages](#commit-messages)
- [Pull Requests](#pull-requests)
- [Testing Strategies](#testing-strategies)
- [Unit Tests](#unit-tests)
- [Accessibility Testing](#accessibility-testing)
1. [Documentation](#documentation)
1. [Breaking Changes](#breaking-changes)
- [Components](#components)
- [Styles](#styles)
- [Deprecating](#deprectating)
- [Removal](#removal)

## Getting Started

Cauldron is Deque's design system / pattern library for building accessible ui as part of our products as Deque. Cauldron consists of several parts, [react](./packages/react), [styles](./packages/styles), and [documentation](./docs).

As Cauldron is first and foremost a design system for building products at Deque, this means we won't necessarily have 1:1 parity with other design systems. Patterns that don't exist in Deque product UI won't be added as a component within the design system.

### Discussing proprietary features

This is a public repo, so care needs to be taken to not include details or screenshots for unannounced or unreleased features in Deque products. Any issues or pull requests need to describe work without including specific business logic or feature details directly related to Deque products. If context is necessary to implement a specific Cauldron feature, please open an issue in the private [cauldron-team repo](https://github.com/dequelabs/cauldron-team/issues) and link to it via the issue or pull request.

### Setup

The files in this project are formatted by Prettier and ESLint. Both are run when code is committed (as a pre-commit hook). Additionally, you can run ESLint manually:
Local development setup is documented in [this project's readme](./README.md#development)

```sh
$ yarn lint
```
## Developing Components

General [component guidelines](https://cauldron.dequelabs.com/component-guidelines) and [style guidelines](https://cauldron.dequelabs.com/style-guidelines) live within our documentation.

## Commit Messages
- Use well-structured semantic markup over `aria-*` attributes
- Don't specify units for zero values, e.g. `padding: 0px;`, instead you can omit the unit, e.g. `padding: 0;`
- Avoid using CSS prefixes (e.g. `-webkit` or `-moz`) unless absolutely necessary. [autoprefixer](https://github.com/postcss/autoprefixer) should automatically handle prefixes.

We follow the [conventional commit specification](https://www.conventionalcommits.org/en/v1.0.0/#summary), please ensure your commit message or pull request title follows the spec.
### Tools Used

## Unit Testing
- [React](https://react.dev/)
- [Typescript](https://www.typescriptlang.org/)
- [Webpack](https://webpack.js.org/)
- [Jest](https://jestjs.io/)
- [axe-core](https://github.com/dequelabs/axe-core)
- [React Testing Library](https://testing-library.com/docs/react-testing-library/intro/)
- [enzymejs](https://enzymejs.github.io/enzyme)
- [MDXJS](https://mdxjs.com/)
- [ESLint](https://eslint.org/)
- [Prettier](https://prettier.io/)
- [Puppeteer](https://pptr.dev/)

It is expected that you fully cover the code changes in your patch with unit tests. The tests can be run manually:
### Structure

```sh
$ yarn test
```
├─ packages/react/src/components
│ └─ Combobox/
│ ├─ index.ts # exports the component and subcomponents
│ ├─ Combobox.tsx # main component file
│ ├─ ComboboxItem.tsx # subcomponent file
│ └─ Combobox.test.tsx # unit/behavior tests
└─ packages/react/styles
├─ index.css # global styles export
└─ combobox.css # component styles
```

## Accessibility Testing
### Quality Checklist

### Ensuring component returns no violations (in unit test)
Cauldron does not have a dedicated quality assurance (QA) individual. Having a full-time dedicated QA individual is a luxury most design systems do not have, ours included. In order to deliver components with high quality it is expected that everyone participating as a part of a component's development lifecycle wear different hats to help ensure we meet that high level of quality. Both an individual developing a component and a reviewer reviewing a component are accountable for considering the following checklist:

We use [jest-axe](https://www.npmjs.com/package/jest-axe) to run axe in our unit tests. It is expected that you run axe in **all states** of your component.
- Does the component follow general [component guidelines](https://cauldron.dequelabs.com/component-guidelines)?
- Does the component follow general [style guidelines](https://cauldron.dequelabs.com/style-guidelines)?
- Does the component cover all of the necessary variations within scope? (i.e. default, primary, secondary)
- Does the component cover all of the necessary interactive states? (i.e. active, focus, expanded, disabled)
- Does the component account for different layouts within different viewports? (i.e. responsiveness, media queries, overflow)
- Does the component API match the current design patterns of components? (i.e. props, events, callbacks)
- Does the component only implement features that fall within our browser support or implement these features via progressive enhancement?
- Does the component support both light and dark themes?
- Does the component have appropriate documentation?
- Does the component support customizing text labels or content for translations?
- Does the component need usability testing?

```js
test('should return no axe violations', async () => {
const wrapper = shallow(<MyCauldronComponent />);
expect(await axe(wrapper.html())).toHaveNoViolations();
});
### Server-side Rendering Compatibility

Every Cauldron component needs to be compatible with server-side rendering (SSR). A component should be able to render in an SSR environment such as [Gatsby.js](https://www.gatsbyjs.com/) or [Next.js](https://nextjs.org/) while avoiding DOM globals like `document` or `window` that are not available in these environments.

Cauldron uses [`eslint-plugin-ssr-friendly`](https://github.com/kopiro/eslint-plugin-ssr-friendly) to help prevent the accidental misuse of DOM globals. An additional [utility](./packages/react/src/utils/is-browser.ts) is available to help guard against using DOM globals:

```tsx
import { isBrowser } from '../../utils/is-browser';
export default function SomeComponent() {
if (!isBrowser()) {
return null;
}
// do browser/DOM things
}
```

## Local Development
### Linting

Local development setup is documented in [this project's readme](./README.md#development)
The files in this project are formatted by Prettier and linted with ESLint. Both are run when code is committed (as a pre-commit hook).

## Styles
#### Running Lint Manually

See [style-guidelines](https://cauldron.dequqelabs.com/style-guidelines)
| Command | Description |
| :---------------- | :-------------------------------------- |
| `yarn lint` | Runs eslint against everything |
| `yarn lint --fix` | Automatically fixes some linting errors |

## Icons
### Icons

Icons are located in [packages/react/src/components/Icon/icons](./packages/react/src/components/Icon/icons). To add a new icon, follow the following steps:

- Change any `fill`, `stroke`, or any color related attributes in the SVG to use `currentColor`.
- Remove any namespace attributes (such as `xmlns:*`) from the SVG.
- Save your icon in the above location with the proposed name of your icon, e.g. `[your-name].svg`. The `your-name` portion will be how the icon is used in the React Component, e.g. `<Icon type="your-name" />`.
- Submit a new `feat` PR with your changes.

## Server-side Rendering
### Commit Messages

Cauldron follows the [conventional commit specification](https://www.conventionalcommits.org/en/v1.0.0/#summary) and is enforced via [semantic-pr-title](https://github.com/dequelabs/semantic-pr-title). When submitting a pull request, please ensure your initial commit message or pull request title follows the spec.

#### Change Types

Cauldron aims to avoid shipping [breaking changes](#breaking-changes) on a regular basis, instead opting to use `fix` or `feat` for a majority of changes mapping to `patch` and `minor` semver bumps respectively. The following changes below map to their corresponding semver bumps:

| Category | Type of Change | Semver |
| :------------------- | :-------------------------------------------------------------------- | :----------------- |
| Components | A component is added | `minor` |
| | A component is deprecated | `minor` |
| | A component is removed | `major` (breaking) |
| | A change is made that does not affect the public API of the component | `patch` |
| Component Props | An optional prop is added | `minor` |
| | A required prop is added | `major` (breaking) |
| | A prop's type is expanded | `minor` |
| | A prop's type is narrowed | `major` (breaking) |
| | A prop is deprecated | `minor` |
| | A prop is removed | `major` (breaking) |
| Component Styles/CSS | Adding a new css selector | `minor` |
| | Removing a css selector | `major` (breaking) |
| | Adding a css variable | `minor` |
| | Removing a css variable | `major` (breaking) |
| | Modifying css properties | `patch` or `minor` |

### Pull Requests

We aim to review pull requests within a day or two. If your changes are time-sensitive, feel free to request an expedited review on GitHub or reach out in the private `#cauldron` slack channel.

Once approved by a member of the Cauldron team, your pull request can be merged at any time but _must_ be squash merged.

#### Previewing Changes

Cauldron documentation is deployed automatically via [amplify web previews for pull requests](https://docs.aws.amazon.com/amplify/latest/userguide/pr-previews.html) with each commit to a PR. To view the preview of your changes, navigate to your PR and find the comment from the `aws-amplify` bot, which will include a link to the preview site for that PR. The preview site will only persist for as long as the PR remains opened and will be deleted when closed.

### Testing Strategies

The testing methodology should account for both testing the interface of a component as input/output testing and how an end-user would interact with the component. Ideally, implementation details are avoided unless necessary. Examples below of things to consider when writing tests for a component:

- Including all variants in tests (i.e. default, primary, secondary)
- Including different interactive states (i.e. focus, expanded, disabled)
- Including testing different behaviors for functionality related to different layouts (i.e. responsiveness, media queries, overflow)
- Component api and events (i.e. callbacks, click / keyboard events)
- Running axe against various variants and states

### Unit Tests

Cauldron uses [Jest](https://jestjs.io/) as its test runner to run unit tests along with [Enzyme](https://enzymejs.github.io/enzyme) (deprecated) and [React Testing Library](https://testing-library.com/docs/react-testing-library/intro/). We are currently in the process of migrating tests away from Enzyme to React Testing Library so no new tests should be using Enzyme.

### Accessibility Testing

#### Ensuring component returns no violations (in unit test)

We use [jest-axe](https://www.npmjs.com/package/jest-axe) to run axe in our unit tests. It is expected that you run axe in **all states** of your component.

```jsx
test('should return no axe violations', async () => {
render(<SomeCauldronComponent ref={ref} />);
const component = screen.getByRole('button');
expect(await axe(component)).toHaveNoViolations();
});
```

#### Running Tests

| Command | Description |
| :------------------------ | :------------------------------------------------- |
| `yarn test` | Runs all unit tests |
| `yarn test ComponentName` | Runs tests matching component name |
| `yarn test:a11y` | Runs e2e accessibility tests against documentation |

Avoid referencing `window` properties, such as `document`, unless you are sure the code will only be executed in a browser environment. For instance, it is safe to reference `document` in an [Effect Hook](https://reactjs.org/docs/hooks-effect.html) or a lifecycle method like `componentDidMount()`, but not in the `render()` method of a class component.
## Documentation

Ensuring you only reference these objects when it is safe to do so will ensure that library consumers can use Cauldron with platforms that use an SSR engine, such as GatsbyJS and NextJS.
Component documentation guidelines are outlined in [docs/readme.md](./docs/readme.md).

## Breaking Changes

Expand Down

0 comments on commit b350c52

Please sign in to comment.