Skip to content
Atmosfearful edited this page Jan 17, 2023 · 35 revisions

Contribution & Style Guide

Code conventions

Margin Considered Harmful

Most layouts can be accomplished with less code using grid. Use <Stack /> components for vertical spacing, columns for horizontal spacing. This also maps very nicely to Figma auto-layouts, works in RTL, and doesn't require overrides for start/end elements.

https://mxstbr.com/thoughts/margin/

By nesting stacks, you can accomplish a clean consistent layout with very minimal css!

// 💚
.contentContainer,
header,
main {
  display: grid;
}
.contentContainer {
  gap: 3.2rem;
}
header {
  gap: 0.8rem;
}
main {
  gap: 1.6rem;
}
// 💚
<div className="contentContainer">
  <header>
    <h1>{title}</h1>
    <p>{subtitle}</h1>
  </header>
  <section>
    {...firstSection}
  <section>
  <section>
    {...secondSection}
  <section>
</div>

Prefer plain CSS

It's easy to do dynamic and conditional styles with cx (classnames) or in some cases, data-attributes.

// example of conditional className
<button
  className={cx({ disabled: !!props.disabled }, styles.button)}
/>;

// example of data driven attributes
<div
  className={styles.colorChanger}
  data-hex={state.hex}
  data-variant={props.variant}
/>;

Composition > configuration

Props and configs might reduce lines of code, but are difficult to refactor and lead to big complex components.

// ❌ bad, configuration via props

<MyPage
  showHeader
  headerTabs={[...]}
  showFooter
  showFooterLogo
>
  {content}
</MyPage>
// 💚 good, composition of reuseable elements! 👍

<PageContainer>
  <Header>
    <Tab />
    <Tab />
  </Header>
  <Main>{content}</Main>
  <Footer>
    <Logo />
  </Footer>
</PageContainer>

Do not de-structure props

This helps to prevent name-clashes, demonstrate where a function or value originates, avoids big blocks of code when a component has many props, and requires less refactoring when things are renamed or removed.

// 💚
const MyButton: FC<Props> = (props) => {
  <button onClick={props.onClick}>{props.children}</button>;
};

Use a onEvent/handleEvent naming convention.

I.e. props start with on and handlers functions start with handle

// 💚
const ClickTracker = (props) => {
  const handleClick = (e) => {
    trackClick(e);
    props.onClick(e);
  };

  return <button onClick={handleClick}>Click Me</button>;
};

Provide a return type for any functions that have a return value.

Example: easy to assume this is a number, but it's not! Explicit return types prevent confusion. For functional React components, use the FC<Props> interface.

// 💚
const getQuantity = (): string => {
  return state.quantity;
};

// 💚
const MyButton = (props): FC<Props> => <button {...props} />;

Prefer params objects over multiple arguments

// 💚
const emitEvent = (params) => {
  props.onEvent({ id: params.id, value: params.value });
};

Prefer named exports over default exports.

See this article for reasons why 'default is bad'.

Process & Workflow

Git conventions

Name your branches using this pattern: [nickname]/[issue#]-[meaningful-branch-name] For example, atmos/123-fix-retirement-bug

Rebase your working branches every day with git pull --rebase origin staging. Rebase is preferred to merge as it makes commit-by-commit code-review easier.

PRs are squash-merged to staging, and staging is rebase-merged to main (no merge commits).

Feature flagging

Use feature flags to keep PRs small and reviewable. It's OK to put unfinished UI on production, just flag it so it's hidden!

// import from app/featureFlags or site/featureFlags
import { features } from "lib/featureFlags"

export const Example = () => {
  if (features.newFeatureFlagKey) {
    return <NewFeature />
  }

  return <ExistingCode />
}

Adding new feature flags to @klimadao/lib/featureFlags should be self-explanatory. Import them and use them to hide unfinished or untested UIs from production users.

QA workflow

When a PR is ready (after you have tested it yourself), QA and code-review can happen in parallel.

  • Tag our QA manager (Shimon) as a reviewer.
  • Add the "QA needed" label to the PR.
  • Tag other code reviewers

The QA manager will add any findings as PR review comments. When the QA manager believes it is production ready, they set the label to "QA done"

Submit dependency changes in a standalone PR

Do not make changes to package.json or package-lock.json in any other PR.