Skip to content

Latest commit

 

History

History
1114 lines (877 loc) · 34.1 KB

style.md

File metadata and controls

1114 lines (877 loc) · 34.1 KB

Carbon Style Guide

Table of Contents

Introduction

This guide covers how we prefer to write code for the Carbon Design System. As a result, this document will evolve and change over time as we explore new concepts, learn from our mistakes, and grow the number of languages that we target or support as a design system.

Guidelines or practices outlined in this document are meant to help us as a group build stable and reliable software. What this means to the developer may change depending on the environment in which one writes code.

At the end of the day, we as a group hold the following values about writing code:

  • No abstraction is better than the wrong abstraction
  • Prefer learning patterns over libraries or frameworks
  • Prefer few abstractions and lots of repetition
  • When creating abstractions, only fix or update code if repetition or patterns lead to bugs. Make sure the abstraction is worth its weight

Inspired by Minimal API Surface Area

JavaScript

Style

Be explicit

UnpreferredPreferred
const add = (a, b) => a + b;
const add = (a, b) => {
  return a + b;
};

Certain features in JavaScript have implicit behavior. One of these that we see most often is the implicit return behavior of arrow function expressions, for example:

const add = (a, b) => a + b;

We've found that, while this style is terse and compact, it can be at odds with the fact that code is revisited often and that developers need to peak inside sometimes to see what is going on. For example, if we needed to debug a specific value in the function above then we would go through the following steps:

// Step 1. The code as originally authored
const add = (a, b) => a + b;

// Step 2. Update the code to no longer use the implicit return
const add = (a, b) => {
  return a + b;
};

// Step 3. Add any debugging code or ways to introspect its values
const add = (a, b) => {
  console.log(a);
  return a + b;
};

// Step 4. Undo these changes and bring back to original format
const add = (a, b) => a + b;

If instead we had written this code without the implicit return then we would have saved three out of the four steps above. As a result, we tend to favor being explicit in how JavaScript is written instead of relying on implicit behavior.

React

Guidelines

Writing a component

In general, we prefer to author components using functions instead of classes. This means that we take advantage of built-in and custom hooks to provide state-based behavior inside of a component.

At a high-level, the structure of a component will mirror the following:

import PropTypes from 'prop-types';
import React, { useState, useEffect } from 'react';

function MyComponent({
  // Prefer default argument values to `defaultProps`
  initialValue = 0,
}) {
  // State-related behavior
  const [state, setState] = useState(initialValue);
  // Constants and other variables
  const value = 1;

  // Handlers
  function onClick() {
    // ...
  }

  // Effects
  useEffect(() => {
    // ...
  }, []);

  // Output
  return <button onClick={onClick}>Output</button>;
}

MyComponent.propTypes = {
  /**
   * Description of what this prop is for
   */
  initialValue: PropTypes.number,
};

Note: not every component will mirror the structure above. Some will need to incorporate useEffect, some will not. You can think of the outline above as slots that you can fill if you need this functionality in a component.

When to use React.ForwardRef

From the react docs,

Ref forwarding is an opt-in feature that lets some components take a ref they receive, and pass it further down (in other words, “forward” it) to a child.

For the most part, components should utilize React.ForwardRef so that consumers can impact or control the managing of focus, selection, or animations.

Cases where a component may not need to forward a ref include components that render static content or do not render elements that are focusable, interactive, or animatable.

Note that adding a forwarded ref to a component should be considered a breaking change. When creating a new component, even if you do not anticipate an explicit need to provide a forwarded ref, it's likely still worthwhile to include one to avoid unecessary breaking changes in the future.

className, data-testid, and ...rest

Where possible, the following should be placed on the outermost, parent, or root element within a component:

  • The className prop
  • Additional props spread via ...rest
  • data-testid attributes
function MyComponent({ className, ...rest }) {
  return (
    <div className={className} {...rest}>
      <div>
        <div></div>
      </div>
    </div>
  );
}

The location and placement of what elements these props are placed on should be stable across major, minor, and patch versions. This may not always be possible, but movement of any of these to different elements should ideally only happen within a major version change. In some rare cases to fix critical bugs we can consider moving placement in a minor version, but we'll need to be highly communicative of the change on all of our support channels. Consumers rely on the placement of these within the DOM and any changes can cause tests, functionality, and custom styling to break within consuming applications.

Stable selectors

We also support the placement of data-testid attributes on components as a "stable selector" for locating elements for testing when all other options are exhausted. The location and placement of these in the DOM should remain stable between versions. This can be accomplished by explicitly placing this prop on the outermost element, or it can be accomplished by having ...rest spread on the outermost element.

In some cases ...rest can not be spread on the outermost element and needs to be spread on other key interior elements, such as inputs. For these cases we cannot rely on data-testid being included in ...rest and it must explicitly be applied to the outermost element.

function MyComponent({ className, ...rest }) {
  return (
    <div className={className} data-testid={rest['data-testid']}>
      <div>
        <input {...rest} />
      </div>
    </div>
  );
}

We highly encourage consuming applications to avoid using data-testid unless absolutely necessary and instead use more stable relative queries focused on accessible roles or HTML5 and ARIA semantics for selecting elements for testing.

Authoring dynamic/inline styles

It's increasingly common for applications to use a Content Security Policy (CSP) header with a style-src directive. When this is configured, inline styles are blocked. Due to this, style={{}} can not be used on any element within the codebase. The react/forbid-component-props eslint rule is configured to flag invalid usages of the style attribute/prop.

Components that need dynamic or inline styles can author these via the CSS Object Model (CSSOM). Dynamic styles can be set via individual properties on the CSSStyleDeclaration interface object provided to HTMLElement. This will usually need to be wrapped in a useIsomorphicEffect hook to ensure compatibility between SSR and browser environments and also to ensure the value is unset if not provided.

function MyComponent({ width }) {
  const ref = useRef();

  useIsomorphicEffect(() => {
    if (width) {
      ref.current.style.width = `${width}px`;
    } else {
      ref.current.style.width = null;
    }
  }, [width]);

  return <div ref={ref} />;
}

Translating a component

Certain components will need to expose a way for the caller to pass in translated strings. For a wide variety of components, this should be done through props. However, if there are situations where props don't make sense or the data that needs to be translated depends on state or is nested you will need to use the following strategy to translate a component.

For component translation, you will need to define a map of translation ids and their corresponding default values, along with a default translateWithId prop. For example:

const translationIds = {
  'carbon.component-name.field': 'Default value',
  'carbon.component-name.other-field': 'Other value',
};

function translateWithId(messageId) {
  return translationIds[messageId];
}

function MyComponent({ translateWithId: t = translateWithId }) {
  return (
    <>
      <span>t('carbon.component-name.field')</span>
      <span>t('carbon.component-name.other-field')</span>
    </>
  );
}

The ids used in translationIds should be consistent between major versions. Changing one will represent a breaking change for the component.

These translation message ids should be specified in the component documentation page.

Working with messages that depend on state

If it seems like your translation requires state in order to be translated correctly, consider creating specific message ids for each state value.

For example, when working with something that can be sorted in ascending or descending order you could create two message ids and choose, based on state, which one to use.

function MyComponent({ translateWithId: t = translateWithId }) {
  const [sortDirection, setSortDirection] = useState('ASC');

  function onClick() {
    if (sortDirection === 'ASC') {
      setSortDirection('DESC');
    } else {
      setSortDirection('ASC');
    }
  }

  return (
    <>
      <span>
        {sortDirection === 'ASC'
          ? t('carbon.component-name.sort.ascending')
          : t('carbon.component-name.sort.descending')}
      </span>
      <button onClick={onClick}>t('carbon.component-name.toggle-sort')</button>
    </>
  );
}

If the message depends on a state value, for example a count, then you should pass along this information as a state argument to translateWithId.

function MyComponent({ translateWithId: t = translateWithId }) {
  const [count, updateCount] = useState(0);
  const translationState = {
    count,
  };

  return (
    <>
      <span>
        The current count is:
        {t('carbon.component-name.display-count', translationState)}
      </span>
      <button onClick={() => updateCount(count + 1)}>
        {t('carbon.component-name.increment-count')}
      </button>
    </>
  );
}

Using useCallback and useMemo

useCallback and useMemo can be incredibly useful tools in certain situations. In general, however, we try to avoid them unless one of the following conditions occur:

  • The identity of a function or object is required as a dependency in a dependency array
  • We have observed performance issues due to allocations that can be reproduced and resolved using these techniques

This practice is to avoid introducing useCallback and useMemo prematurely, which can create extra work for our components to perform.

A rule of thumb for this is to understand how frequently a dependency will update that is given to useCallback or useMemo. If a dependency is likely to update frequently, then React will have to perform comparisons and re-run callback to useCallback and useMemo. This would be slower than creating a new function each render instead.

Hooks that rely on refs

When designing hooks that require a reference to a DOM node (using a ref) you should design the hook to take in a ref as an argument instead of creating a ref on behalf of the caller.

This is important when a caller decides to use multiple hooks that rely on a ref. For example,

function MyComponent() {
  const [ref1, isHovering] = useHover();
  const [ref2, isDragging] = useDrag();

  // How should the caller merge these two refs?
}

If, instead, these hooks took in a ref we could have the caller manage the ref and pass it into the hooks.

function MyComponent() {
  const ref = useRef(null);
  const isHovering = useHover(ref);
  const isDragging = useDrag(ref);

  // Caller has to add `ref` to a node below
}

Hooks that use a callback

Often times, you will want to author a hook that executes a given function when something happens. For example, we could have a hook called useEvent that will execute a function whenever the event is triggered:

useEvent(window, 'click', (event) => {
  // Called when the click event fires
});

When you write a hook that uses a pattern like this, you may run into a problem where you want to call the callback in a useEffect block, but you don't want that effect to fire every time the callback changes.

From our useEvent hook above, this would come up when adding the event listener to the document:

function useEvent(element, eventName, callback) {
  // ...

  useEffect(() => {
    element.addEventListener(eventName, callback);
    return () => {
      element.removeEventListener(eventName, callback);
    };
  }, [element, eventName, callback]);

  // ...
}

In the code snippet above, the effect specified in useEffect will trigger any time the element changes, the event changes, or the callback changes. However, we only would want the listener re-attached any time the element or event name changes, not when the callback changes.

To separate out the callback changes from changes in our effect's dependencies, you can use the saved callback pattern:

function useEvent(element, eventName, callback) {
  const savedCallback = useRef(callback);

  useEffect(() => {
    savedCallback.current = callback;
  });

  useEffect(() => {
    function listener(event) {
      savedCallback.current(event);
    }
    element.addEventListener(eventName, listener);
    return () => {
      element.removeEventListener(eventName, listener);
    };
  }, [element, eventName]);
}

By saving our callback in a ref, we're able to keep track of changes to the callback that we receive without having to re-run our useEffect block every time it changes.

Style

Naming event handlers

UnpreferredPreferred
function MyComponent() {
  function click() {
    // ...
  }
  return <button onClick={click} />;
}
function MyComponent() {
  function onClick() {
    // ...
  }
  return <button onClick={onClick} />;
}
function MyComponent({ onClick }) {
  function handleClick(event) {
    // ...
    onClick(event);
  }
  return <button onClick={handleClick} />;
}
function MyComponent({ onClick }) {
  function handleOnClick(event) {
    // ...
    onClick(event);
  }
  return <button onClick={handleOnClick} />;
}

When writing event handlers, we prefer using the exact name, onClick to a shorthand. If that name is already being used in a given scope, which often happens if the component supports a prop onClick, then we prefer to specify the function as handleOnClick.

Naming experimental code

See Experimental Code;

Testing

Strategy

In general we aim to test components from a user-focused perspective. This means avoiding testing implementation details, and instead focusing on writing tests that closely resemble how the components are used. The various testing-library packages are used to encourage this mindset when writing and composing test suites.

Organization

Every component should have tests covering a series of Categories

  • General component functionality/API
  • Accessibility
  • End to end tests
  • Server side rendering

Each of these are separated into individual files. In some cases the syntax may be slightly different and separate files make this easier to understand. Additionally separate file types can be more easily globbed to only run certain tests in certain environments (local, CI, Pre-release checks, etc).

File name Category
ComponentName-test.js General component functionality
ComponentName-test.a11y.js Accessibility testing
ComponentName-test.e2e.js End to end tests
ComponentName-test.ssr.js Server side rendering

There are corresponding commands to run all categories, individual categories, or a combination. Depending on your shell, modifiers can be used to run two commands one after another. Refer to the documentation of your shell.

Command Corresponding test category
yarn test All categories
yarn test:unit Only component unit tests
yarn test:a11y Only accessibility tests
yarn test:ssr Only server side tests
yarn test:a11y && yarn test:e2e In bash via &&: Run the a11y tests, and if they succeed, run e2e

Recipes

Below are some common recipes for component testing. Many of the pattern/syntax details contained within these recipes are enforced via eslint rules declared in eslint-config-carbon.

ComponentName-test.js
  • Use @testing-library/react
  • Format with describe/it blocks
  • Use jest-dom matchers for assertions
import { render, screen, findByLabel } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { ComponentName } from '../ComponentName';

describe('ComponentName', () => {
  describe('API', () => {
    it('should provide a data-testid attribute on the outermost DOM node', () => {
      const { container } = render(<ComponentName className="test" />);
      expect(screen.getByTestId('component-test-id')).toBeInTheDocument();
      expect(container.firstChild).toHaveAttribute('class', 'test');
    });

    it('should place the `className` prop on the outermost DOM node', () => {
      const { container } = render(<ComponentName className="test" />);
      expect(container.firstChild).toHaveAttribute('class', 'test');
    });

    it('should place extra props on the outermost DOM node', () => {
      const { container } = render(<ComponentName data-testid="test" />);
      expect(container.firstChild).toHaveAttribute('data-testid', 'test');
    });

    describe('i18n', () => {
      // ... ensure when each prop string is configured it is rendered to the DOM
    });

    // id
    // -----
    // When a component accepts an id prop, it's important
    // that the node on which the id is placed is consistent
    // between minor versions. As a result, tests that you
    // write for id should make assertions around id being
    // placed on the same node.
    it('should place the `id` prop on the same DOM node between minor versions', () => {
      const { container } = render(<ComponentName data-testid="test" />);
      expect(container.firstChild).toHaveAttribute('id', 'test');
    });

    // Event Handlers
    // -----
    // When a component accepts an `onClick` or `onChange` prop
    // it can be helpful to make assertions about when these
    // props are called and what they are called with in order
    // to test the Public API of the component.
    // To make assertions on a function, such as whether its
    // been called or what it has been called with, we can make
    // use of Jest's `jest.fn()` method to create mock
    // functions. We can then make assertions on these mock
    // functions.
    it('should call `onClick` when the trigger element is pressed', () => {
      const onClick = jest.fn();

      render(<TestComponent onClick={onClick} />);

      const trigger = screen.getByText('trigger');
      userEvent.click(trigger);
      expect(onClick).toHaveBeenCalled();
    });

    // Optional ref tests
    // A component that accepts a ref falls in one of three scenarios:
    // 1. A class component
    // 2. A component that uses React.forwardRef and placed
    //    it on an HTML element
    // 3. A component that uses React.forwardRef and uses
    //    useImperativeHandle to decorate the ref (this is
    //    uncommon but can come up)
  });
});
ComponentName-test.a11y.js
  • Use accessibility-checker and axe
  • Optionally configure common props to ensure component variants do not contain accessibility errors.
  • Always use the destructured container from render() to ensure the entire DOM tree is validated before and after interaction.
describe('ComponentName AVT1', () => {
  it('should have no aXe violations', async () => {
    const { container } = render(<ComponentName />);
    await expect(container).toHaveNoAxeViolations();
  });

  it('should have no AC violations', async () => {
    const { container } = render(<ComponentName />);
    await expect(container).toHaveNoACViolations('ComponentName');
  });
});
ComponentName-test.server.js
/**
 * @jest-environment node
 */
import ReactDOMServer from 'react-dom/server';
import { ComponentName } from '../ComponentName';

describe('ComponentName - SSR', () => {
  it('should import ComponentName in a node/server environment', () => {
    expect(ComponentName).not.toThrow();
  });

  it('should not use document/window/etc', () => {
    expect(ReactDOMServer.renderToStaticMarkup(ComponentName)).not.toThrow();
  });
});
Notes on manual testing

Sass

Guidelines

Author component styles using mixins

When authoring the styles for a component, it's important that we use mixins to allow developers to control when the CSS for a specific component gets emitted. For example:

// src/components/accordion/_accordion.scss

/// Accordion
/// @access private
/// @group accordion
@mixin accordion {
  .#{$prefix}--accordion {
    // ...
  }
}

Authoring component styles under a mixin allows the design system to:

  • Control when the CSS for accordion gets emitted, or not emitted, from the library
  • Allows us to author experimental or future styles in a separate mixin and toggle its inclusion through feature flags
  • Could allow developers consuming the design system to control when styles get emitted

Use design tokens where appropriate

We have a number of Sass variables available in the project to be used as design tokens when building out a component. Almost always you will want to leverage these instead of hard coding values for colors, type, or even spacing. You can visit the following SassDoc links to view all of the design tokens relevant to this project:

Avoid magic numbers

In addition to using design tokens where appropriate, when authoring values for margin, padding, size, or similar, avoid using magic numbers.

"A magic number is a value that is used ‘because it just works’."

Magic numbers should be replaced with a value derived from its discrete parts that have been added together or combined. For example:

text-input-style-structure-fixed

If we were trying to apply a padding-inline-end to the input to ensure the input text does not flow behind the icon, we could add up the individual parts of this that use spacing tokens, contextual layout tokens, or other constants/variables within the system that will inherently explain what the final number is composed of.

- padding-inline-end: to-rem(32px);
+ padding-inline-end: calc(layout.density('padding-inline') + $icon-size-01);

When crafting these combinations, avoid creating file-local constants/variables, especially if they are never reused. Instead:

  1. Check the reusable/global constants for an appropriate one given what is trying to be accomplished.
  2. If one exists, use it. If not, start a conversation with the team as to why no such value currently exists (perhaps challenge the way it was intended to be used in the first place).
  3. Decide to either introduce a new constant to meet the need; or rework the code in question to use other constants (or perhaps none at all).

Avoid nesting selectors

Nesting selectors is often a convenient and fast way to author styles in Sass. Unfortunately, they also add a performance and maintenance burden for the design system. The performance burden is due to the generated nature of selectors which can lead to unexpected CSS bundle bloat. The maintenance burden manifests itself in a way that makes it harder to find specific selectors while working on the codebase. For example, if we are looking for the selector .component:focus in the following file:

// Early on in the file
.component {
  // ...
}

// ...

// Later on in the file
.component {
  &:focus {
    // ...
  }
}

It can be difficult to track down the specific .component:focus selector without having to navigate through the file and relevant matches to see where &:focus is being defined. While this may be hard to visualize in a short code snippet, as file size grows and our Sass is rewritten or updated, this problem becomes increasingly obvious.

Use only as much specificity as needed

It's important that we write selectors that use only as much specificity as needed. Ideally, we would only need one selector per component but this is rarely the case. As a result, adding specificity should be done sparingly or when including it is helpful when building a component. For example, if you would like to enforce a specific element or ARIA attribute then using this attribute in a selector would be appropriate:

button[aria-expanded='false'] {
  // ...
}

If we compared this to a class selector, for example .my-component__button, then we may consider this as adding more specificity than needed. However, for the design system it is more important that the component itself implements this contract for accessibility.

Use the global $prefix variable

When writing selectors, always include the global $prefix variable. This value is used to namespace all of the selectors that we ship in the design system.

UnpreferredPreferred
.my-component {
  // ...
}
.#{$prefix}--my-component {
  // ...
}

Annotate relevant Sass values with SassDoc

When authoring functions, mixins, or values that are intended to be shared, you should leverage SassDoc. The typical format we use includes the following information:

/// <Details about the mixin>
/// @access <public|private>
/// @group <name-of-group>
@mixin my-component {
  // ...
}

Style

Comments

When annotating individual selectors or properties, you should add an inline comment above the piece of code you're commenting.

UnpreferredPreferred
.#{$prefix}--my-component {
  width: 100%; // Comment about why we need 100% width
}
.#{$prefix}--my-component {
  // Comment about why we need 100% width
  width: 100%;
}

When annotating a section of a Sass file, it is helpful to use the following banner comment style:

//----------------------------------------------------------------------------
// Section name
//----------------------------------------------------------------------------

Note: this banner should be formatted to span 80 columns

When writing SassDoc comments, you should use three forward slashes:

/// This is a comment for SassDoc
/// @access public
.#{$prefix}--my-component {
  // ...
}

Testing

We use the @carbon/test-utils package to test our Sass styles in JavaScript. Inside of this package, there is a SassRenderer module that you can bring in that allows you to get values from Sass in JavaScript to be used in test assertions.

The basic template for tests for Sass files will look like:

/**
 * <COPYRIGHT>
 *
 * @jest-environment node
 */

'use strict';

const { SassRenderer } = require('@carbon/test-utils/scss');

const { render } = SassRenderer.create(__dirname);

describe('@carbon/styles/scss/config', () => {
  test('Public API', async () => {
    const { get } = await render(`
      // You can bring in modules using the path from the test file
      @use '../path/to/sass/module';

      $test: true;

      // The `get` helper will let you pass a value from Sass to JavaScript
      $_: get('test', $test);
    `);

    // get('<key>') gives you both the JavaScript representation of a value
    // along with the `nativeValue` which comes from Dart sass. Use `.value`
    // to get the JavaScript value and make assertions
    expect(get('test').value).toBe(true);
  });
});

Recipes

Public API

Sometimes it is useful to assert that a module's Public API matches what is expected or does not change between versions. To do this in a test file, you can use the sass:meta module along with several helpers for getting the variables and functions from a module. Unfortunately, mixins need to be checked by hand using the mixin-exists function from sass:meta.

test('Public API', async () => {
  await render(`
    @use 'sass:meta';
    @use '../path/to/module';

    // Get the variables for the module under the namespace `module`
    $_: get('variables', meta.module-variables('module'));

    // Get the functions for the module under the namespace `module`
    $_: get('variables', meta.module-functions('module'));

    // Verify that a mixin exists, optionally within a module
    $_: get('mixin-name', meta.mixin-exists('mixin-name', 'module');
  `);
});