Skip to content

Commit

Permalink
Editing some markdown files around contributions (#2827)
Browse files Browse the repository at this point in the history
  • Loading branch information
cchaos authored Feb 6, 2020
1 parent aca04c7 commit e88c8c6
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 60 deletions.
16 changes: 12 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@

🙌 Thanks for your interest in contributing to EUI! 🙌

## New Components, Features, and Bug Fixes
## New components, features, and bug fixes

When creating new components, adding new features, or fixing bugs, please refer to the [Component Development guidelines][docs-components]. If there isn't an associated issue on the bug tracker yet, consider creating one so that you get a chance to discuss the changes you have in mind with the rest of the team.

### Feel free to submit pull requests in draft stages

EUI has strict quality and testing standards due to its large downstream footprint and accessibility requirements. Don't feel intimidated and think you need to submit perfect PRs because of this. We welcome draft PRs to show conceptual ideas or enhancements you would like to see. The EUI team normally engages on these PRs in one of two ways, which is largely up to you.
1. We can provide review and guidance for how to get the PR up to the library's standards. (slower, but you might enjoy this)
2. We can commit directly to your PR to get it over the finish line. (faster)

If you have a preference, let us know when you make your PR, but never feel guilty about just handing it off. We're here to help.

### Adding icons

EUI provides an ever-growing set of [icons][icons], but our set can be incomplete. If you find you need an icon that does not exist, create a new issue and tag it with the *icons* label. A designer from the EUI team will respond to discuss your needs.
Expand All @@ -14,18 +22,18 @@ If you are willing and able to design the icon yourself, then please refer to th

## Documentation

Always remember to update [documentation site][docs] and the [`CHANGELOG.md`](CHANGELOG.md) in the same PR that contains functional changes. We do this in tandem to prevent our examples from going out of sync with the actual components. In this sense, treat documentation no different than how you would treat tests.
Always remember to update [documentation site][docs] via the `src-docs` folder and the [`CHANGELOG.md`](CHANGELOG.md) in the same PR that contains functional changes. We do this in tandem to prevent our examples from going out of sync with the actual components. In this sense, treat documentation no different than how you would treat tests.

Here are our guidelines for updating the `CHANGELOG.md` file:

* Append your changes to the `master` sub-heading of [`CHANGELOG.md`](CHANGELOG.md).
* Add a list item for each significant change in the PR: bugs that were fixed, new features, new components, or changes to the public API
* In the list item, always link to any relevant Pull Requests, commit ranges, or individual commits
* In the list item, always link to any relevant pull requests
* Add a short summary of what has changed, making sure it's informative to consumers who might be unaware of implementation details
* Avoid documenting internal implementation changes that don't have an effect on the public interface
* Write your entry in the past tense, starting with a verb (e.g. Added... , Fixed...)

## Software Releases
## Releases

When we are ready to create a new release, we follow the [Release Process][docs-releases] documentation.

Expand Down
98 changes: 46 additions & 52 deletions wiki/component-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,68 +13,55 @@ We use abbreviations to refer to sizes, e.g. `xxl`, `xl`, `l`, `m`, `s`, `xs`, a
We define objects which map enums to corresponding values, typically CSS classes. For example,
here's how we would define maps for colors and sizes in a fictional `MegaMenu` component.

```jsx
// We first define the map for getting the appropriate class for each enum value.
const colorToClassNameMap = {
```tsx
// We first define the enum values as a type
type EuiMegaMenuColor = 'primary' | 'secondary' | 'warning' | 'danger';

// Then we define the map for getting the appropriate class for each enum value.
const colorToClassNameMap: { [color in EuiMegaMenuColor]: string } = {
primary: 'euiMegaMenu--primary',
secondary: 'euiMegaMenu--secondary',
warning: 'euiMegaMenu--warning',
danger: 'euiMegaMenu--danger',
};

// Then we generate the enums themselves by pulling out the keys.
export const COLORS = Object.keys(colorToClassNameMap);
export const COLORS = keysOf(colorToClassNameMap);
```

// We can repeat this pattern for other things, e.g. sizes.
const sizeToClassNameMap = {
s: 'euiMegaMenu--small',
l: 'euiMegaMenu--large',
};
This is how we define the prop types using the enums we generated in Typescript:

export const SIZES = Object.keys(sizeToClassNameMap);
```tsx
// We can refer to the enums objects for the prop types.
export type EuiMegaMenuProps = {
color: EuiMegaMenuColor;
isDisabled?: boolean;
/* ... */
};
```

We use the maps to generate the classname for the component:
For the default props we can just specify the enum values we want to use in the constructor and then use the maps to generate the classname for the component:

```jsx
export const MegaMenu = ({
```tsx
export const EuiMegaMenu: FunctionComponent<EuiMegaMenuProps> = ({
children,
className,
color,
size,
color = 'primary',
className,
isDisabled,
isDisabled = false,
...rest
}) => {
const classes = classNames(
'euiMegaMenu',
colorToClassNameMap[color],
sizeToClassNameMap[size],
className,
{
'euiMegaMenu--isDisabled': isDisabled,
},
);

/* ... */
```
This is how we define the prop types using the enums we generated:
```jsx
// We can refer to the enums objects for the prop types.
EuiMegaMenu.propTypes = {
color: PropTypes.oneOf(COLORS),
size: PropTypes.oneOf(SIZES),
/* ... */
};

// For the default props we can just specify the enum values we want to use.
EuiMegaMenu.defaultProps = {
color: 'primary',
size: 'l',
/* ... */
};
}
```

## Pass-through props
Expand All @@ -88,33 +75,47 @@ The main benefit behind this practice is that the consumer can specify any of
the [DOM attributes](https://reactjs.org/docs/dom-elements.html) supported by React, including
custom ones with the `data-` prefix.

In Typescript, it makes sense to then extend the props of that element when declaring the component's type. EUI also provides a shortlist of commonly used props like `className`, `aria-label`, and `data-test-subj` that you should extend as well.

```jsx
export const EuiMegaMenu = ({
import { HTMLAttributes, FunctionComponent } from 'react';
import { CommonProps } from '../common';

export type EuiMegaMenuProps = HTMLAttributes<HTMLDivElement> &
CommonProps & {
color: EuiMegaMenuColor;
isDisabled?: boolean;
/* ... */
};

export const EuiMegaMenu: FunctionComponent<EuiMegaMenuProps> = ({
children,
className,
color,
color = 'primary',
size,
className,
isDisabled,
isDisabled = false,
...rest
}) => {

// Anything else specified by the consumer will be applied to the div as a DOM attribute.
return (
<div
{...rest}
>
<div {...rest}>
{/* ... */}
</div>
);

/* ... */
}
```

## Naming props

### Enums

String literals should be used wherever possible and prioritized over booleans. This allows for the most extensibility when it comes to adding more features/options in the future. For example, instead of the prop `isHorizontal: boolean` use `layout: 'horizontal' | 'vertical'`.

### Booleans

Generally, boolean props should have an `is` prefix, e.g. `isPlaceholder` or `isReadOnly`. The exception to this is when the prop matches an existing HTML attribute such as `disabled`; to avoid confusion prop name should align with the HTML specification. This type of mirroring the attributes makes the most sense when the component is a thin wrapper around an existing HTML element, e.g. EuiButton -> button and EuiRadio -> <input type="radio">.
Generally, boolean props should have an `is` prefix, e.g. `isPlaceholder` or `isReadOnly`. The exception to this is when the prop matches an existing HTML attribute such as `disabled`; to avoid confusion the prop name should align with the HTML specification. Mirroring the attributes this way makes the most sense when the component is a thin wrapper around an existing HTML element, e.g. EuiButton -> `<button>` and EuiRadio -> `<input type="radio">`.

### Event handlers

Expand All @@ -126,13 +127,6 @@ Try to leverage the `children` prop wherever possible. This will create a simple
API throughout our components.

We also [require some props](../src/test/reqiured_props.js) to be supported by all components, as
reflected in our tests; for example, `className`.
```jsx
EuiMegaMenu.propTypes = {
children: PropTypes.node,
className: PropTypes.string,
}
```
reflected in our tests; for example, `className`. These are easily added via the `CommonProps` mentioned above.

[component-development]: component-development.md
7 changes: 3 additions & 4 deletions wiki/component-development.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,22 +66,21 @@ If a component has subcomponents (`<EuiToolBar>` and `<EuiToolBarSearch>`), tigh

### Writing CSS

We follow Kibana's [CSS style guide][kibana-css] and [SCSS style guide][kibana-scss].
Refer to the [SASS page][sass] of our documentation site for a guide to writing styles.

[component-design]: component-design.md
[docs]: https://elastic.github.io/eui/
[docs-yeoman]: creating-components-yeoman.md
[docs-manual]: creating-components-manually.md
[kibana-css]: https://github.com/elastic/kibana/blob/master/style_guides/css_style_guide.md
[kibana-scss]: https://github.com/elastic/kibana/blob/master/style_guides/scss_style_guide.md
[sass]: https://elastic.github.io/eui/#/guidelines/sass

## TypeScript definitions

### Pass-through props

Many of our components use `rest parameters` and the `spread` operator to pass props through to an underlying DOM element. In those instances the component's TypeScript definition needs to properly include the target DOM element's props.

A `Foo` component that passes `...rest` through to a `button` element would have the props interface
A `Foo` component that passes `...rest` through to a `button` element would have the props interface

```
// passes extra props to a button
Expand Down

0 comments on commit e88c8c6

Please sign in to comment.