diff --git a/.gitignore b/.gitignore index e9225072d..14e96439a 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,9 @@ public/config.json backend/synapse_tmp/* backend/synapse_tmp_othersite/* /coverage + +# Transient agent scratch files +/agent-workspace/ config.json # Yarn diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..f0a94112c --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,54 @@ +# AGENTS.md — Element Call + +MatrixRTC (MSC4143) + LiveKit video calling, shipped standalone, as a widget in +Element Web and Element X, as embedded packages, and as a React component in a +host's page. It is the MatrixRTC reference implementation. + +## Non-negotiables + +- Features and UI changes need a pre-approved issue. No issue, no review. +- Exactly one `PR-*` label per PR. CI enforces it; it drives the changelog. +- New behaviour ships with unit tests, a story if it renders, and an e2e spec if it + is user-facing. All three. +- Reuse an existing component. If you genuinely cannot, say so in the PR body and + name what you rejected — never add a new one silently. +- Nothing reads the page. No `window.location`, `document.body`, `window.inner*`, + `@media`, global `i18next` or the `widget` global — take it from a provider. +- The change works standalone, as a widget and as a component. Say what you checked. +- Every gate below is green before you push. +- Hand off after the first implementation, before the quality pass. Then commit on + the user's word where the setup allows it, or hand them the message. + +## Read before you + +| … | … | +| -------------------------------- | ---------------------------------------------------------- | +| start a task of any size | [docs/agents/workflow.md](docs/agents/workflow.md) | +| touch call logic or a view model | [docs/agents/architecture.md](docs/agents/architecture.md) | +| write any code | [docs/agents/code-style.md](docs/agents/code-style.md) | +| write a test, story or e2e spec | [docs/agents/testing.md](docs/agents/testing.md) | +| open a PR | [CONTRIBUTING.md](CONTRIBUTING.md) | +| implement from a feature spec | [FEATURES_SPEC/AGENTS.md](FEATURES_SPEC/AGENTS.md) | + +## Gates + +```sh +pnpm lint # tsc, oxlint, knip, component externals +pnpm format # oxfmt +pnpm test # vitest: unit and storybook projects +pnpm i18n:check +``` + +`knip` fails on dead code; a file deliberately inert ahead of its consumer goes in +`knip.ts` `ignoreFiles` with a reason. + +## A good PR + +- One slice, under ~400 changed lines, green on its own, linked to its issue. +- View model + marble tests, thin view, a story per state, an e2e spec. +- Any new shared component called out explicitly, with why nothing existing fit. +- Template filled for real: what, why, before/after screenshots, repro steps. +- Anything that redraws continuously — an animation frame, a stream — says what it + costs, or better, counts the redraws in a test. +- Branch `/`. Plain imperative commit subjects, no prefixes. +- Once review starts, fix forward. Never force-push a regeneration over a review. diff --git a/docs/README.md b/docs/README.md index c4b60c2fd..8bd148462 100644 --- a/docs/README.md +++ b/docs/README.md @@ -9,3 +9,12 @@ This folder contains documentation for setup, usage, and development of Element - [MatrixRTC modes](./matrix_rtc_modes.md) - [Self-Hosting](./self_hosting.md) - [Developing with linked packages](./linking.md) + +### Writing code for Element Call (agent-readable conventions) + +Entry point: [AGENTS.md](../AGENTS.md). + +- [Agent workflow](./agents/workflow.md) +- [Architecture](./agents/architecture.md) +- [Code style](./agents/code-style.md) +- [Testing](./agents/testing.md) diff --git a/docs/agents/architecture.md b/docs/agents/architecture.md new file mode 100644 index 000000000..92cac2575 --- /dev/null +++ b/docs/agents/architecture.md @@ -0,0 +1,79 @@ +# Architecture + +## Logic lives in view models; components render + +- Call logic lives in `src/state/`. A component that derives call state is + misplaced logic. +- Composing behaviors: a factory taking `(scope, ...deps$)`, returning named + behaviors plus callbacks. Copy `src/state/LayoutSwitchViewModel.ts`. Prefer this + for anything new. +- Owning a resource: a class taking the scope in its constructor — `MediaDevices`, + `MuteStates`, `TileStore`, `Publisher`, `Connection`. Both styles are current; + don't convert one to the other in passing. +- `foo$` is an observable. `Behavior`, an observable with a current value, is + the default for anything a view reads. + +## The view model / view contract + +- Components take `vm: ViewModel` and read state through `useBehavior`. + See `CallFooter`, `InCallView`, `LobbyView`, `SettingsModal`. +- A snapshot is `Actions & State`; every field becomes a `foo$` behavior, and none + is optional. +- An unavailable action is `undefined`, not a separate `canDoThing` flag — the + presence of the callback drives the rendering. +- Subscribe in an effect only to drive a side effect off an event stream, as + `ReactionAudioRenderer` does; never to read state a behavior already holds. + +## Scopes own lifetimes + +- `ObservableScope` bounds every subscription a view model creates. +- Reference only the scope defined in the same function; one captured from an + enclosing scope outlives its owner, and `no-observablescope-leak` rejects it. + +## Nothing reads the page + +Element Call can be mounted several times inside a host's React tree, so it owns +neither window, URL, document nor router. Each seam defaults to the old standalone +and widget behaviour. + +| Never | Use | +| --------------------------------------------- | ------------------------------------------------------------- | +| the `widget` global | `useHostBridge()` — `src/HostBridge.ts` | +| `getUrlParams()`, `window.location` | `useUrlParams()` | +| `useNavigate("/")`, `` | `useLeaveToHome()`, `LeaveToHomeLink` | +| `document.body` | `useRootElement()` | +| `window.innerWidth/Height`, `useMediaQuery` | `useRootSizeMatches()` in views, `windowSize$` in view models | +| global `i18next` | the instance in `src/utils/i18n.ts` | +| config or analytics reading their environment | `Config.initWith()`, `PosthogAnalytics.configure()` | + +- View models take values as options, never `getUrlParams()` + (`callViewModelOptionsFromParams`, `CallViewModelOptions.hostBridge`). +- The host bridge is the only channel to the host: `createWidgetHostBridge(widget)`, + `nullHostBridge` standalone, `useComponentHostBridge`. State a capability + (`supportsReactions`, `supportsProfileChanges`); never infer it from being a + widget. +- Shortcuts and portals attach to the root element, so two instances don't fight. +- Known debt, not precedent: `Grid` measures `window.innerHeight`; `ErrorView`'s + reload and `getAbsoluteRoomUrl` use `window.location`; recaptcha appends to + `document.body` on the standalone login path. + +## Context differences are options, not checks + +- Named options with per-intent defaults (`configurationForIntent`), never a + runtime check for who is hosting. +- `controlledAudioDevices`, not the platform, is what makes `MediaDevices` pick + `AndroidControlledAudioOutput` / `IOSControlledAudioOutput` over the web + `AudioOutput`. Intent presets set it on non-desktop; standalone leaves it off. +- URL params are a published contract: change one, update `docs/url_params.md`. + +## Build targets + +Code that builds in only one is a bug. + +- `build:full` — standalone app, also widget mode. +- `build:embedded` — `@element-hq/element-call-embedded`. +- `build:sdk` — SDK library, entry `sdk/main.ts`. +- `build:component` — `@element-hq/element-call-component`, sources in `component/` + (its own pnpm project; run pnpm from the repo root). Host API is in the README; + `pnpm lint:externals` rejects an import of a `react` / `react-dom` / + `matrix-js-sdk` / `livekit-client` subpath the externals list omits. diff --git a/docs/agents/code-style.md b/docs/agents/code-style.md new file mode 100644 index 000000000..1907aaf82 --- /dev/null +++ b/docs/agents/code-style.md @@ -0,0 +1,74 @@ +# 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: + +1. **Compound Web** — `@vector-im/compound-web` for buttons, tooltips, alerts, + dialogs, menus, form controls, avatars; icons in + `@vector-im/compound-design-tokens/assets/web/icons`. +2. **This repo** — `src/button/`, `src/components/`, `src/input/`, `src/form/`, + `src/tabs/`, and `Modal`, `Avatar`, `Toast`, `Slider`, `ErrorView`, + `FullScreenView` at the root of `src/`. +3. **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 `function` declaration, not a `const` arrow. 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$`, not + `computedLayout$`. Comments explain why, never what. + +## TypeScript, React and RxJS idiom win on a clash + +- Hooks stay unconditional at the top of a component. +- `useCallback` / `useMemo` dependency 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.ts` is long because the domain is. +- Marble test tables are dense on purpose. + +## Enforced by lint + +- Copyright header on every file: `Copyright Element Creations Ltd.` + plus the AGPL / commercial SPDX line. +- `logger` from `matrix-js-sdk/lib/logger`, never `console`, and no top-level + `logger.getChild()`. The `console` ban is only enforced under `src/*/**`. +- No floating or misused promises; async functions typed `Promise`. +- Inline type imports (`import { type Foo }`), so matrix-js-sdk stays lazily + loadable. +- Deep-import `matrix-js-sdk/lib/` as the codebase does. Banned is the bare + `matrix-js-sdk/lib`, `lib/index` and anything under `src/`. + +## 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 (…)` and + `cqw` / `cqh`, never `@media (width)` or `vw` / `vh`. Media queries stay correct + only in standalone-only views — home, login. +- Style `[data-element-call-root]`, never `body` or `:root`; the component build + makes those stand for the root (`component/build/scopeStylesToRoot.ts`). + +## Strings and a11y + +- Strings through `t()`: add the key, run `pnpm i18n`, fill `locales/en/app.json`. + Other locales come from Localazy; never hand-edit them. +- `t` from `useTranslation()`, or `src/utils/i18n.ts` outside React. Never the + `i18next` global — several instances share a page. +- Accessible names on controls, `aria-pressed` on toggles, keyboard reachability. + `jsx-a11y` rules are errors; `vitest-axe` is available. diff --git a/docs/agents/testing.md b/docs/agents/testing.md new file mode 100644 index 000000000..35a9548a8 --- /dev/null +++ b/docs/agents/testing.md @@ -0,0 +1,53 @@ +# Testing + +Three layers; a user-facing feature needs all three. Unit proves the logic, +Storybook proves the rendering, e2e proves the feature. Codecov gates 80% on the +lines a PR touches. + +## Unit + +- View model tests use `withTestScheduler` + `testScope()` from `src/utils/test.ts` + and read as ASCII timelines. Copy `LayoutSwitchViewModel.test.ts`. +- Component tests use Testing Library, queried by role and accessible name. +- Reuse the factories in `src/utils/test.ts` — `mockRemoteParticipant`, + `mockMatrixRoom`, `mockLivekitRoom`, `MockRTCSession`. Hand-rolled mocks drift. +- Snapshots live in `__snapshots__/`; update with `pnpm test -u`. +- How often something redraws is testable: drive the frames and count commits, not + render calls. An effect with no dependency array runs once per commit. +- `component/**/*.test.ts` runs in the same jsdom project as `src`. + +## Storybook + +A deliverable, not documentation: `pnpm test:storybook` runs every story as a real +browser test in the same CI job as the unit suite. **A UI change without a story is +incomplete.** Only three stories exist so far, so copy `CallFooter.stories.tsx`. + +- Drive the component from a snapshot via `useStaticViewModel`, never internals. +- One named story per state that matters — loading, error, empty, denied, mobile. +- Assert in a `play` function with `userEvent` / `expect` from `storybook/test`. +- Mobile is `globals: { viewport: { value: "mobile2" } }`, not a second component. +- Expose interesting props through `argTypes`. + +`.storybook/preview.tsx` supplies `TooltipProvider`, `src/index.css` and English +translations. The browser is Playwright's, so a fresh clone needs +`pnpm playwright install` once. + +## End-to-end + +- `playwright/*.spec.ts` — standalone, on Chromium and Firefox. +- `playwright/widget/` — widget mode, where cross-context bugs surface. +- `playwright/component/` — the component in a host page, via the harness on port + 3001 that Playwright starts as a second web server. Catches container-relative + layout, styles escaping the root, two instances on a page, host-bridge reports. +- `playwright/mobile/` — Pixel 7, `mobile` project only. + +Test what a user observes: the peer sees the change, it survives a reconnect, it is +right after a reload. Reuse `playwright/spa-helpers.ts`, +`playwright/widget/test-helpers.ts`, `playwright/fixtures/`. + +```sh +pnpm test # unit + storybook +pnpm backend # Synapse + LiveKit, required for e2e +pnpm test:playwright # or :open +pnpm dev:component # component harness, port 3001 +``` diff --git a/docs/agents/workflow.md b/docs/agents/workflow.md new file mode 100644 index 000000000..2170f4a8c --- /dev/null +++ b/docs/agents/workflow.md @@ -0,0 +1,50 @@ +# Agent workflow + +## Scratch files live in `agent-workspace/` + +Git-ignored. One kebab-case subfolder per task, matching the branch topic: +`agent-workspace//`. + +| File | Purpose | +| --------------------------- | -------------------------------------------------------------------------------- | +| `plan.md` | The concrete approach — files, symbols, edit order — once direction is confirmed | +| `commit-msg.txt` | `git commit -F agent-workspace//commit-msg.txt` | +| `pr-body.md` | `gh pr create --body-file agent-workspace//pr-body.md` | +| `implementation-summary.md` | What was built, decisions, trade-offs | +| `NN-prompt.md` | Raw prompt text, numbered, when worth keeping | + +- Nothing here is durable. The folder is ignored, so anything worth keeping is + promoted into the PR body, a doc, or a feature spec before the task ends. +- Never write scratch files to the repo root. Stray `load_test_summary.md` and + `config.json_` files are what this folder prevents. +- Start `pr-body.md` from `.github/PULL_REQUEST_TEMPLATE.md` and fill every + section. +- Commit subjects are plain imperative English. No conventional-commits prefixes. +- A `plan.md` and a feature spec sit at different altitudes; neither replaces the + other. A spec in `FEATURES_SPEC/` is durable and deliberately abstract — + behaviour, decisions, acceptance criteria — so the feature can be rebuilt against + a `main` nobody has seen yet. A plan is one slice of it landing on today's + `main`: the paths, symbols and edit order the spec's decisions and criteria must + not name. `FEATURES_SPEC/AGENTS.md` wins wherever the two genuinely overlap. + +## Hand off before the quality pass + +- Implement the change, run the narrowest check that rules out an obviously broken + handoff, then stop and ask whether the direction is right. +- Full `pnpm lint`, the whole suite, coverage and benchmarks come after the + direction is confirmed. +- Wider refactors, extra tests and documentation polish are follow-up work, not + part of the first handoff. +- If a check is needed before feedback, keep it to the touched code and say why. + +## Commit and PR readiness + +- Commit once the user confirms direction, or asks for one. Not before. Where the + setup has no git identity, a sandbox included, write the message to + `commit-msg.txt` and hand it over instead. +- Ready means every gate in [AGENTS.md](../../AGENTS.md#gates) is green and the + change is covered at the layers [testing.md](./testing.md) asks for. Read the diff + against [code-style.md](./code-style.md) first. +- Re-run the whole checklist after any fix. Only on green, either way. +- Once a human has started reviewing, fix forward — never force-push a + regeneration over a review in progress.