mirror of
https://github.com/vector-im/element-call.git
synced 2026-09-25 22:35:49 +00:00
* Say what earns a comment its place - Add a comments section to the code style: no comment by default, the reason only, and the kinds that go stale. - Point to it from the PR checklist in AGENTS.md. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
4.4 KiB
4.4 KiB
Code style
Clean Code (Robert C. Martin): small functions, intention-revealing names, low noise.
Reuse before you build
Look before you write, in this order:
- Compound Web —
@vector-im/compound-webfor buttons, tooltips, alerts, dialogs, menus, form controls, avatars; icons in@vector-im/compound-design-tokens/assets/web/icons. - This repo —
src/button/,src/components/,src/input/,src/form/,src/tabs/, andModal,Avatar,Toast,Slider,ErrorView,FullScreenViewat the root ofsrc/. - The nearest existing feature — grep for the same shape first. A hook is often what you want to extract, not a component.
If nothing fits, name the new shared component in the PR body and say what you rejected and why — never add one silently. Extend or parameterise before forking, and extract shared logic into a hook both callers use before copying it; if you copy, the PR says so.
Ordering
- Newspaper order. Headline first, detail down. A file opens with what it exists to provide.
- Stepdown rule. Caller above callee. A helper used by one function sits
beneath it, which needs a
functiondeclaration, not aconstarrow. Existing arrow components above their callers are not a pattern to extend — nor to churn. - Suites. Test cases first, helpers below. Older suites invert this; follow the rule in new ones, don't reorder old ones.
- Names. What a thing means, not what it is made of:
naturalLayout$, notcomputedLayout$.
Comments
No comment is the default. One earns its place by giving a reason the code cannot: a platform quirk, an outside constraint, a trap that looks like a simplification.
- Say the reason, then stop. Test each sentence: does the reason still stand without it? If so, cut it. Most reasons fit on a line, so a long comment is worth a second look — it is usually carrying something that isn't one.
- Write about the line the comment sits on, in terms of what the code does there — leave layout to the stylesheet and measurements to the design.
- Name the condition ("where the platform can't route audio"), not the browser or the spec entry.
- Put history — what was tried, removed or rejected, the dev journal — in the commit or the PR description.
- In tests, comment only structure that looks wrong but isn't: batching, timers, why this scroll position.
Before keeping one: would it still be true after a CSS change or a refactor of its caller? If not, cut it.
TypeScript, React and RxJS idiom win on a clash
- Hooks stay unconditional at the top of a component.
useCallback/useMemodependency arrays sometimes force inlining.- An RxJS pipeline stays one expression. Name the behavior, not each operator.
- A long factory is fine when it reads as a list of named behaviors.
CallViewModel.tsis long because the domain is. - Marble test tables are dense on purpose.
Enforced by lint
- Copyright header on every file:
Copyright <current year> Element Creations Ltd.plus the AGPL / commercial SPDX line. loggerfrommatrix-js-sdk/lib/logger, neverconsole, and no top-levellogger.getChild(). Theconsoleban is only enforced undersrc/*/**.- No floating or misused promises; async functions typed
Promise<T>. - Inline type imports (
import { type Foo }), so matrix-js-sdk stays lazily loadable. - Deep-import
matrix-js-sdk/lib/<module>as the codebase does. Banned is the barematrix-js-sdk/lib,lib/indexand anything undersrc/.
CSS
- Compound components and
--cpd-*tokens in CSS modules. A hardcoded colour or px spacing is a design question, not a licence to inline a hex. - Size against the root, not the window:
@container element-call (…)andcqw/cqh, never@media (width)orvw/vh. Media queries stay correct only in standalone-only views — home, login. - Style
[data-element-call-root], neverbodyor:root; the component build makes those stand for the root (component/build/scopeStylesToRoot.ts).
Strings and a11y
- Strings through
t(): add the key, runpnpm i18n, filllocales/en/app.json. Other locales come from Localazy; never hand-edit them. tfromuseTranslation(), orsrc/utils/i18n.tsoutside React. Never thei18nextglobal — several instances share a page.- Accessible names on controls,
aria-pressedon toggles, keyboard reachability.jsx-a11yrules are errors;vitest-axeis available.