Skip to content
Atmosfearful edited this page Jun 14, 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 = (props: MyButtonProps) => {
  <button onClick={props.onClick}>{props.children}</button>;
};

Use an "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.

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

// 💚
const MyButton = (props: MyButtonProps): 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.

Translation / localization / i18n

For localization we use lingui.js. Lingui auto-generates source string files and writes them to the /locale folder.

Every time a commit is added to staging, a bot will automatically extract strings and sync with our remote translation platform, translation.io. This is where our community of translators do their work.

Every string has a string id, these are auto-generated by lingui. Developers need only wrap a string in a lingui tag!

import { t, Trans } from "@lingui/macro";

<MyComponent
  label={t`I am a tagged string, translate me`}
/>

<button>
  <Trans>I am a tagged string, translate me</Trans>
</button>

All staging, preview and local builds include a language called "Pseudo" which can be used more easily spot untranslated strings, or to identify the underlying string id.

Why am I seeing string_ids in my preview build?

This is a common mistake. Run npm run extract-strings:dev from the repository root. This will update the en and pseudo strings for all packages. When you merge your work to staging, the bot will generate ids for the other locales.

Strings show a string_id fallback placeholder until a translation is added and synced from translation.io-- except for en and pseudo, which are auto-generated from the tags when you run the above command.