Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(conventions): separation of concerns for the internal doc #4295

Merged
merged 4 commits into from
Mar 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions conventions/Accessibility.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Accessibility

Components must be accessible and are required to have tests that focus on a11y. We use the following resources as guides:

- [Google Accessibility Overview](https://developers.google.com/web/fundamentals/accessibility/)
- [WAI-ARIA Authoring Practices](https://www.w3.org/TR/wai-aria-practices-1.1/)
9 changes: 9 additions & 0 deletions conventions/Documentation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Documentation

This project uses [Storybook](https://storybook.js.org/) to provide an interactive showcase of components with accompanying documentation.

For each main component (i.e., one that can be used by itself), there should be a `<component-name>.stories.ts` file in its component folder.

Each story should provide access to relevant [knobs](https://github.com/storybookjs/storybook/tree/next/addons/knobs) so users can test out different properties.

For additional documentation, create a [usage folder](https://github.com/Esri/calcite-components/tree/master/src/components/action/usage) in the component directory with a basic.md and optionally an advanced.md file (if additional documentation or examples are required) with snippets showing different supported use cases for the component.
29 changes: 29 additions & 0 deletions conventions/Internationalization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Internationalization

Components should require as a few text translations as possible. In general, components should let users supply text values via slots and attributes; this lets a user handle translation within their apps.

If you component involves formatting numbers or dates use the [`Intl` APIs](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl) for formatting the display of numbers and dates in your component.

To add right-to-left (RTL) support to your components you should use [CSS Logical properties](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Logical_Properties) within CSS. If you need to know direction in JavaScript, use the internal `getElementDir` helper in the dom utility.

Some CSS properties do not have logical equivalents. Such as...

- box-shadow
- text-shadow
- transform
- background-position

For these properties, you should use the internal `getElementDir` helper to apply the `CSS_UTILITY.rtl` class to your component.

## Translated strings

In future it will likely become necessary to provide string translations for components. An example would be the `aria-label` for the `<calcite-modal>` close button. Initial research looks promising and we could likely implement one of these approaches and set a `lang` for each component.

- https://medium.com/stencil-tricks/implementing-internationalisation-i18n-with-stencil-5e6559554117 and https://codesandbox.io/s/43pmx55vo9
- https://github.com/ionic-team/ionic-stencil-conference-app/issues/69

Until we implement a `lang` facility and set up translations for all components, we have been allowing a small number of strings to be passed in as props. Props that represent translated strings should have the syntax: `text-label-x`, where `x` is the name for the string. For example, when providing a string from "Close", use the prop name `text-label-close`. In the component, these props should default to their English equivalent (this is useful for non-localized apps):

```
@Prop() textLabelClose: string = 'Close';
```
256 changes: 0 additions & 256 deletions conventions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,43 +2,6 @@

This is a living document defining our best practices and reasoning for authoring Calcite Components.

<!-- TOC depthFrom:2 -->

- [Component Guidelines](#component-guidelines)
- [General Guidelines](#general-guidelines)
- [Structure](#structure)
- [Styling](#styling)
- [Avoid complex CSS selectors](#avoid-complex-css-selectors)
- [Color](#color)
- [Light Theme/Dark Theme](#light-themedark-theme)
- [Custom Themes](#custom-themes)
- [Typography](#typography)
- [Palette](#palette)
- [Component Responsibilities](#component-responsibilities)
- [Events](#events)
- [Event Names](#event-names)
- [Private/Internal Events](#privateinternal-events)
- [Event Details](#event-details)
- [Props](#props)
- [Focus support](#focus-support)
- [CSS Class Names](#css-class-names)
- [assets](#assets)
- [a11y](#a11y)
- [i18n](#i18n)
- [Translated strings](#translated-strings)
- [Bundling and Loading](#bundling-and-loading)
- [Unique IDs for Components](#unique-ids-for-components)
- [Prerendering and SSR](#prerendering-and-ssr)
- [Cleaning up resources](#cleaning-up-resources)
- [Timeouts](#timeouts)
- [Tests](#tests)
- [Writing Tests](#writing-tests)
- [Prevent logging unnecessary messaging in the build](#prevent-logging-unnecessary-messaging-in-the-build)
- [Unstable Tests](#unstable-tests)
- [Documentation](#documentation)

<!-- /TOC -->

## General Guidelines

Generally adhere to and follow these best practices for authoring components.
Expand All @@ -50,154 +13,6 @@ Generally adhere to and follow these best practices for authoring components.

We follow Stencil's suggested component structure. See their [style guide](https://github.com/ionic-team/stencil/blob/master/STYLE_GUIDE.md#file-structure) for more details.

## Styling

Be sure to set `shadow: true` in Stencil's `@Component` options to make sure styles are encapsulated in our Calcite design system. This helps keep our components consistent across applications.

### Avoid complex CSS selectors

Avoid complex CSS selectors and move logic into the component. As a general rule, if using more than 1 attribute in the CSS selector, use a class and move the logic into the component.

For example, instead of a complex CSS selector as demonstrated below:

```css
[alignment="icon-end-space-between"]:not([width="auto"]) {
/* ... */
}
```

Add a class to handle the logic in the component class.

```tsx
<div
class={{
[CSS.myClass]: alignment === "icon-end-space-between" && width !== "auto"
}}
/>
```

```css
.myClass {
/* ... */
}
```

## Color

If a component has multiple color themes (for example Blue, Red, Green, and Yellow) representing various state implement a `color` prop and reflect it to an attributes.

```tsx
enum Colors {
red = "red",
blue = "blue",
green = "green",
yellow = "yellow",
}

export class Component {

// ...

@Prop({ reflect: true }) color: Colors = 'blue'

// ...
```

You can then use the `:host()` selector to define your custom colors:

```scss
:host([color="blue"]) {
.something {
// make it blue
}
}

:host([color="red"]) {
.something {
// make it red
}
}
```

**Discussed In**:

- https://github.com/Esri/calcite-components/pull/24/files/3446c89010e3ef0421803d68d627aba2e7c4bfa0#r289427838

## Light Theme/Dark Theme

In the [global CSS file](https://github.com/Esri/calcite-components/blob/master/src/assets/styles/global.scss), we specify the values of each color for both light and dark theme. This enables theming to be inherited throughout a component tree. Consider this valid example:

```html
<div class="calcite-theme-dark">
<calcite-button>Button text</calcite-button>
<calcite-date-picker></calcite-date-picker>
</div>
```

This will cause both the button and the date picker to use the dark theme color variables declared in the global file. This makes it very easy for developers to move an entire app from light to dark and vice versa.

To make this work, inside a component's SASS file, _you must use colors from the theme variables_. For example

```scss
// 🙅‍♀️ using the sass var will not correctly inherit or change in light/dark mode
:host {
color: $ui-brand-light;
}

// 👍 using the CSS var will inherit correctly
:host {
color: var(--calcite-ui-brand);
}
```

## Custom Themes

Since Calcite Components might be used in many different contexts such as configurable apps, multiple themes and appearances need to be supported. The most common use case for custom themes are applications where the end user needs to be able to customize brand colors and typography. To this end custom theming can be accomplished by overriding the [CSS Custom Properties (CSS Variables)](https://developer.mozilla.org/en-US/docs/Web/CSS/--*) from the main light and dark themes with new values:

```css
:root {
--calcite-ui-brand: red;
}
```

You can apply these overrides to individual components as well:

```css
calcite-slider {
--calcite-ui-brand: red;
}
```

Or, add a class to the specific instance:

```css
.my-custom-theme {
--calcite-ui-brand: red;
}
```

```html
<calcite-slider class="my-custom-theme"></calcite-slider>
```

### Typography

All components have been constructed to inherit their `font-family`. This enables you to change the font much like changing the colors:

```css
:root {
font-family: "Comic Sans";
}
```

### Palette

The current light theme colors and their hex values can be found [here](https://esri.github.io/calcite-colors/).

**Discussed In**:

- https://github.com/Esri/calcite-components/issues/507

## Component Responsibilities

Calcite Components broadly targets two groups of projects inside Esri:
Expand Down Expand Up @@ -374,43 +189,6 @@ const assetPath = getAssetPath(`./assets/my-component/asset.json`);

This is required in order to have a unified assets folder in the distributable.

## a11y

Components must be accessible and are required to have tests that focus on a11y. We use the following resources as guides:

- [Google Accessibility Overview](https://developers.google.com/web/fundamentals/accessibility/)
- [WAI-ARIA Authoring Practices](https://www.w3.org/TR/wai-aria-practices-1.1/)

## i18n

Components should require as a few text translations as possible. In general, components should let users supply text values via slots and attributes; this lets a user handle translation within their apps.

If you component involves formatting numbers or dates use the [`Intl` APIs](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl) for formatting the display of numbers and dates in your component.

To add right-to-left (RTL) support to your components you should use [CSS Logical properties](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Logical_Properties) within CSS. If you need to know direction in JavaScript, use the internal `getElementDir` helper in the dom utility.

Some CSS properties do not have logical equivalents. Such as...

- box-shadow
- text-shadow
- transform
- background-position

For these properties, you should use the internal `getElementDir` helper to apply the `CSS_UTILITY.rtl` class to your component.

### Translated strings

In future it will likely become necessary to provide string translations for components. An example would be the `aria-label` for the `<calcite-modal>` close button. Initial research looks promising and we could likely implement one of these approaches and set a `lang` for each component.

- https://medium.com/stencil-tricks/implementing-internationalisation-i18n-with-stencil-5e6559554117 and https://codesandbox.io/s/43pmx55vo9
- https://github.com/ionic-team/ionic-stencil-conference-app/issues/69

Until we implement a `lang` facility and set up translations for all components, we have been allowing a small number of strings to be passed in as props. Props that represent translated strings should have the syntax: `text-label-x`, where `x` is the name for the string. For example, when providing a string from "Close", use the prop name `text-label-close`. In the component, these props should default to their English equivalent (this is useful for non-localized apps):

```
@Prop() textLabelClose: string = 'Close';
```

## Bundling and Loading

Stencil has the capability to build and distribute a large variety of outputs based on our needs. You can read more about this in the [output targets](https://github.com/ionic-team/stencil/blob/cc55401555ff5c28757cf99edf372dcada2c0b25/src/compiler/output-targets/readme.md) documentation.
Expand Down Expand Up @@ -493,37 +271,3 @@ focusMenu(): void => {
this.menuFocusTimeout = window.setTimeout(() => focusElement(this.menuEl), 100);
}
```

## Tests

Components should have an automated test for any incoming features or bug fixes. Make sure all tests pass as PRs will not be allowed to merge if there is a single test failure.

We encourage writing expressive test cases and code that indicates intent. Use comments sparingly when the aforementioned can't be fully achieved. Keep it clean!

Please see Stencil's doc for more info on [end-to-end](https://stenciljs.com/docs/end-to-end-testing) testing. See one of our test examples [here](https://github.com/Esri/calcite-components/blob/master/src/components/block/block.e2e.ts).

### Writing Tests

#### Prevent logging unnecessary messaging in the build

**This is only necessary if a component's test will produce a lot of console messages in a test run.**

As a best practice when writing tests, prevent emitting console warnings by stubbing them. Depending on the tested component, this may also apply to other console APIs.

Console warnings can end up polluting the build output messaging that makes it more difficult to identify real issues. By stubbing `console.warn`, you can prevent warning messages from displaying in the build. See [`color.e2e`](https://github.com/Esri/calcite-components/blob/af0c6cb/src/components/color/color.e2e.ts#L9-L17) for an example.

### Unstable Tests

If you notice that a test fails intermittently during local or CI test runs, it is unstable and must be skipped to avoid holding up test runs, builds and deployments.

To skip a test, use the `skip` method that's available on [tests, or suites](https://jestjs.io/docs/en/api#methods) and submit a pull request. Once that's done, please create a follow-up issue by [choosing](https://github.com/Esri/calcite-components/issues/new/choose) the unstable test template and filling it out.

## Documentation

This project uses [Storybook](https://storybook.js.org/) to provide an interactive showcase of components with accompanying documentation.

For each main component (i.e., one that can be used by itself), there should be a `<component-name>.stories.ts` file in its component folder.

Each story should provide access to relevant [knobs](https://github.com/storybookjs/storybook/tree/next/addons/knobs) so users can test out different properties.

For additional documentation, create a [usage folder](https://github.com/Esri/calcite-components/tree/master/src/components/action/usage) in the component directory with a basic.md and optionally an advanced.md file (if additional documentation or examples are required) with snippets showing different supported use cases for the component.
Loading