feat(theme): put Element Call on the OrisoScheme contract, show the scheme in Storybook - #902
Conversation
The call UI runs on Compound's --cpd-* roles while ORISO speaks --m3-*. Nothing bridged the two, so the fork carried hand-written colours that could not follow a re-tune of the engine — the same drift that left the Figma export 11 roles out of date. Map the Compound semantic roles onto engine roles and emit the stylesheet the fork ships. Compound declares `@layer cpd-semantic, custom, cpd-base` for exactly this, so the override needs no !important and no selector games. The contrast assertions caught a real defect while writing them: --cpd-color-border-focused mapped onto --m3-primary-outline, a role tuned to stay dark on light brand surfaces, rendered the focus ring at 1.5:1 on the dark canvas. --m3-primary flips with the scheme and clears 3:1 in both. vite-node needs the same `noExternal` treatment the vitest config already documents for @material/material-color-utilities. Refs #897 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lint-staged reformatted the artefact on commit, so the checked-in file no longer matched the generator and the drift test failed — correctly. A generated file should not be subject to formatting churn: the generator owns its bytes, and the test compares them exactly. Refs #897 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two of the mapped roles (--cpd-color-text-primary-alpha and --cpd-color-text-secondary-alpha) do not exist in Compound — I had guessed them from the naming pattern. Checked against the real token list, they are gone. --cpd-color-text-placeholder is not a Compound role either, but the fork reads it in input/Input.module.css, so it stays with that reason recorded. Also assert the defect that caused the epic: Compound derives --cpd-color-icon-primary-alpha from --cpd-color-alpha-gray-1400, which is near-white in the dark theme. The fork had flattened it to opaque black for video tiles, taking every icon on that role with it. The new pairs fail if those two concerns are ever conflated again. Refs #897 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The widget URL carried no theme parameter, so the call was dark only because Element Call's own fallback picks dark when nothing is asked for. That made our appearance an accident of an upstream default, with nothing in our code to point at if it ever changed. The call is deliberately dark while the app around it stays light; app-wide dark remains a separate decision (ACTIVE_SCHEMES). No visual change — this pins what already happens. Refs #900 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was no place to see the platform's colours. The Figma file was meant to be that place and drifted eleven roles out of date, because it was a copy. These pages compute every swatch from the engine as they render, so they cannot go stale the same way. - Colour roles: light, dark and inverted side by side, each role with its hex and — where a role has a defined partner — its contrast ratio, flagged in red when it misses AA. - Element Call mapping: the --cpd-* ← --m3-* bridge, read from the same table the generator writes, so this page cannot disagree with the stylesheet the fork ships. - Live embed: the deployed call in an iframe with a switchable theme. Deliberately not imported components — the call's screens pull matrix-js-sdk, livekit-client and widget state, and the fork runs React 19; mocking that in would show mocks, not the design, and would break on every upstream merge. - Toolbar scheme switch that repaints any story in any scheme. The switch is a Storybook capability only: ACTIVE_SCHEMES is untouched and the app still ships light. Components will look wrong in dark — that is the inventory this makes visible, and fixing them belongs to a future app-dark decision, not here. Refs #898 Refs #899 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughThe PR adds an ORISO-to-Element Call theme bridge, deterministic CSS generation with drift tests, Storybook token and live-embed views, and explicit dark-mode widget configuration. ChangesElement Call theming
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Storybook
participant DesignTokens
participant callTheme
participant ElementCall
Storybook->>DesignTokens: render ORISO roles and Compound mappings
DesignTokens->>callTheme: read palette roles and CALL_TOKEN_MAP
callTheme-->>DesignTokens: return mapped values and swatches
Storybook->>ElementCall: load configured iframe URL
ElementCall-->>Storybook: render the selected theme
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/generate-call-theme.ts`:
- Around line 26-34: Centralize the generated theme artefact path in a shared
named constant, exporting it from the existing theme module or a small shared
configuration module. In scripts/generate-call-theme.ts at lines 26-34, replace
the local path.join construction with that constant; in
src/utils/theme/callTheme.test.ts at lines 83-87, import and reuse the same
constant instead of rebuilding the path.
In `@src/components/call/widget/useElementCallWidget.ts`:
- Around line 164-169: Align the ready-time theme update in the api.on('ready',
...) handler with the embedded call URL by changing the
ClientWidgetApi.updateTheme payload from light to dark, or remove the update so
theme=dark remains the single source of truth.
In `@src/components/designTokens/DesignTokens.stories.tsx`:
- Around line 102-163: Update RoleTable to compute each scheme’s palette once
and pass its tokens to SchemeColumn instead of having SchemeColumn call
computeOrisoPalette for every cell. Change SchemeColumn’s props and value lookup
to use the supplied tokens, while preserving the existing role iteration, scheme
columns, and contrast behavior.
In `@src/components/designTokens/ElementCallEmbed.stories.tsx`:
- Around line 1-16: Update the introductory rationale in the story comment above
ElementCallEmbed so it no longer claims the fork uses React 19 while the app
does not. Preserve the explanation that the iframe avoids bundling the call’s
dependencies and conflicting design systems, rewriting the React-version wording
only if needed to accurately support the deliberate iframe choice.
In `@src/utils/theme/callTheme.test.ts`:
- Around line 38-51: Add an exhaustive exact-value test in callTheme.test.ts
that iterates over every entry in CALL_TOKEN_MAP, reads the generated token for
both dark and light themes, and asserts each value equals the corresponding
mapped --m3-* role in the matching engine palette. Keep the existing focused
tests and contrast/foreign-colour checks unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 345faf89-49b0-4cb3-95d0-ee90aebb2563
⛔ Files ignored due to path filters (6)
.prettierignoreis excluded by none and included by none.storybook/preview.tsxis excluded by none and included by none.storybook/withOrisoScheme.tsxis excluded by none and included by nonepackage.jsonis excluded by none and included by nonesrc/utils/theme/generated/element-call-theme.cssis excluded by!**/generated/**and included bysrc/**vitest.config.mtsis excluded by none and included by none
📒 Files selected for processing (7)
scripts/generate-call-theme.tssrc/components/call/widget/useElementCallWidget.test.tsxsrc/components/call/widget/useElementCallWidget.tssrc/components/designTokens/DesignTokens.stories.tsxsrc/components/designTokens/ElementCallEmbed.stories.tsxsrc/utils/theme/callTheme.test.tssrc/utils/theme/callTheme.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: lint, type-check, build and Storybook Docker validation
- GitHub Check: lint, type-check, test, build and Docker validation
- GitHub Check: lint, type-check, build and test frontend
- GitHub Check: lint, type-check and build frontend
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Keep shared behavior in hooks/utilities when multiple screens need the same state, route, selection, or formatting logic.
UI changes must have accessible focus/keyboard behavior and must not rely on color alone.
Files:
scripts/generate-call-theme.tssrc/components/designTokens/DesignTokens.stories.tsxsrc/components/call/widget/useElementCallWidget.tssrc/utils/theme/callTheme.test.tssrc/components/designTokens/ElementCallEmbed.stories.tsxsrc/components/call/widget/useElementCallWidget.test.tsxsrc/utils/theme/callTheme.ts
**/*.{scss,sass,css,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Reuse existing design tokens, Sass mixins, and component patterns; avoid one-off hardcoded CSS for controls, active list states, focus rings, and responsive layout.
Files:
scripts/generate-call-theme.tssrc/components/designTokens/DesignTokens.stories.tsxsrc/components/call/widget/useElementCallWidget.tssrc/utils/theme/callTheme.test.tssrc/components/designTokens/ElementCallEmbed.stories.tsxsrc/components/call/widget/useElementCallWidget.test.tsxsrc/utils/theme/callTheme.ts
src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx,js,jsx}: Check behavior against the surrounding ORISO flow, not only types.
Flag duplicated route/state logic, missing error/loading handling,
privacy regressions around Matrix/chat/drafts, and changes that should
be expressed through shared hooks or utilities instead of local flags.
Files:
src/components/designTokens/DesignTokens.stories.tsxsrc/components/call/widget/useElementCallWidget.tssrc/utils/theme/callTheme.test.tssrc/components/designTokens/ElementCallEmbed.stories.tsxsrc/components/call/widget/useElementCallWidget.test.tsxsrc/utils/theme/callTheme.ts
🧠 Learnings (1)
📚 Learning: 2026-07-18T03:56:46.133Z
Learnt from: kiodreambau
Repo: OpenResilienceInitiative/ORISO-Frontend PR: 516
File: src/components/groupChat/waitingClock/WaitingAreaCountdown.stories.tsx:0-0
Timestamp: 2026-07-18T03:56:46.133Z
Learning: When reviewing Storybook stories (e.g., `*.stories.tsx`), treat story-specific styling done via isolated inline decorators/fixtures as Storybook simulation, not as production design tokens or focus-style behavior. Do not flag the story’s styles as production focus/regression issues unless the change involves the actual production components (not the story wrapper/fixture).
Applied to files:
src/components/designTokens/DesignTokens.stories.tsxsrc/components/designTokens/ElementCallEmbed.stories.tsx
🪛 ast-grep (0.45.0)
scripts/generate-call-theme.ts
[warning] 46-46: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(out, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 59-59: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(out, css, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
src/utils/theme/callTheme.test.ts
[warning] 18-21: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(
\\.cpd-theme-${scheme}[^{]*\\{([\\s\\S]*?)\\n\\},
'm'
)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 23-23: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${token}:\\s*([^;]+);)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 93-93: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(artefact, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🪛 OpenGrep (1.26.0)
src/utils/theme/callTheme.test.ts
[ERROR] 19-22: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 24-24: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (8)
src/components/call/widget/useElementCallWidget.test.tsx (1)
163-167: LGTM!src/utils/theme/callTheme.ts (1)
1-229: LGTM!scripts/generate-call-theme.ts (1)
1-25: LGTM!Also applies to: 36-62
src/utils/theme/callTheme.test.ts (1)
1-37: LGTM!Also applies to: 53-82, 98-161
src/components/designTokens/DesignTokens.stories.tsx (1)
1-100: LGTM!Also applies to: 165-201, 203-261, 263-285
src/components/designTokens/ElementCallEmbed.stories.tsx (3)
18-22: LGTM!
24-68: 🔒 Security & Privacy | ⚡ Quick winReuse the shared URL utility, confirm param parity, and add defense-in-depth origin validation.
Three points on this segment:
- Line 25 reimplements trailing-slash stripping locally.
getElementCallBaseUrlalready composesstripTrailingSlashes(context snippet 1). Export and reuse that helper here instead of a duplicate regex, so the two call sites stay in sync.- The header comment claims this story shows the call "the way
useElementCallWidgetembeds it." TheuseElementCallWidget.tsfile is not in this batch. Confirm thatroomId,theme,confineToRoom,header,skipLobby,intent, andcallIntentmatch the actual widget hook's parameter set, so this documentation page cannot silently drift from production behaviour the way the Figma export did for colour roles.baseUrlis a free-text control used directly as the iframesrcorigin, and the iframe grantscamera,microphone,display-capture, andclipboard-writeviaallow. React 19 rejectsjavascript:URLs insrc/href, and Storybook restricts URL-drivenargsvalues to alphanumeric characters, spaces, underscores, and dashes, which blocks a crafted external link from injecting a full attacker origin. Given both platform-level mitigations, add an explicithttp/httpsprotocol check before buildingurlas defense-in-depth, in case a developer pastes an arbitrary origin directly into the control.As per path instructions, "changes that should be expressed through shared hooks or utilities instead of local flags" — reusing `stripTrailingSlashes` instead of a duplicate regex satisfies this.🔒 Proposed defense-in-depth check
const Embed: React.FC<EmbedArgs> = ({ baseUrl, theme, room }) => { const origin = (baseUrl || '').replace(/\/+$/, ''); - if (!origin) { + const isHttpOrigin = /^https?:\/\//i.test(origin); + if (!origin || !isHttpOrigin) { return ( <p style={{ padding: 24, maxWidth: '65ch' }}> - No Element Call origin configured. Set{' '} + No valid Element Call origin configured. Set{' '} <code>REACT_APP_ELEMENT_CALL_BASE_URL</code>, or type an origin into the <code>baseUrl</code> control. </p> ); }Source: Path instructions
70-88: LGTM!
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Align the ready-time updateTheme with the theme=dark URL pin (the light payload flipped the embedded call to light after launch) - Share the generated-artefact location: export CALL_THEME_ARTEFACT_PATH from callTheme.ts and use it in both the generator CLI and the drift test instead of two independent path.join constructions - Assert every CALL_TOKEN_MAP entry against the engine palette in both schemes, so a mapping pointed at the wrong --m3-* role fails a test - Compute each scheme's palette once in RoleTable instead of once per table cell - Drop the stale React-19 rationale from the ElementCallEmbed story comment (this app is on React 19 too now) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Pushed cfa3080 addressing all five review findings (all verified as valid against the code), plus a clean merge of
Local verification: |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/utils/theme/callTheme.ts (2)
215-237: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake custom-seed provenance reproducible.
buildCallThemeCss(seed)accepts custom seeds, but the generated header always says(ORISO default tenant)and always gives the default regeneration command. A stylesheet generated with--seedcan therefore contain false provenance and cannot be reproduced from its own metadata. Make the tenant label and regeneration instruction conditional on the supplied seed. (raw.githubusercontent.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/theme/callTheme.ts` around lines 215 - 237, Update buildCallThemeCss to derive the header’s tenant label and regeneration command from the supplied seed: retain the ORISO default tenant metadata and default command for the default seed, but emit custom-seed provenance and a command that includes --seed with the actual seed for other values. Ensure the generated metadata can reproduce the stylesheet.
68-75: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix low-contrast status mappings and test the rendered pairs.
The generated light theme renders
--cpd-color-text-info-primaryas#4d5660on--cpd-color-bg-info-subtle#656e79, about 1.44:1. The critical subtle background maps to#de0077instead of an error-container-like surface, so--cpd-color-text-critical-primaryalso renders at 1.44:1 in light mode. Updatesrc/utils/theme/callTheme.tsto map these status roles to engine roles that have guaranteed contrast, and add the real status pair assertions toTEXT_PAIRS.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/theme/callTheme.ts` around lines 68 - 75, Update the status role mappings in src/utils/theme/callTheme.ts at lines 68-75 and 120-132 so the info and critical text/background pairs use engine roles with guaranteed contrast, including an error-container-like surface for critical backgrounds. In src/utils/theme/callTheme.test.ts lines 143-182, add assertions for the rendered info and critical status pairs to TEXT_PAIRS and verify their light-theme contrast.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/generate-call-theme.ts`:
- Around line 29-31: Update the argument-value handling in argValue so it
rejects missing next arguments and any value beginning with "--", including
cases such as --out --check and --seed --check, instead of treating the flag as
a path or seed. Preserve normal value parsing for valid non-flag arguments and
ensure invalid usage reports the existing argument error.
In `@src/components/call/widget/useElementCallWidget.test.tsx`:
- Around line 316-318: Strengthen the ready-handler assertion around
api.updateTheme by verifying it is called exactly once, in addition to checking
the dark theme payload. Keep the existing dark update expectation and prevent
any light or contradictory update from passing the test.
---
Outside diff comments:
In `@src/utils/theme/callTheme.ts`:
- Around line 215-237: Update buildCallThemeCss to derive the header’s tenant
label and regeneration command from the supplied seed: retain the ORISO default
tenant metadata and default command for the default seed, but emit custom-seed
provenance and a command that includes --seed with the actual seed for other
values. Ensure the generated metadata can reproduce the stylesheet.
- Around line 68-75: Update the status role mappings in
src/utils/theme/callTheme.ts at lines 68-75 and 120-132 so the info and critical
text/background pairs use engine roles with guaranteed contrast, including an
error-container-like surface for critical backgrounds. In
src/utils/theme/callTheme.test.ts lines 143-182, add assertions for the rendered
info and critical status pairs to TEXT_PAIRS and verify their light-theme
contrast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 526b4ac8-7c54-411a-a8c7-7416fe3a8a13
⛔ Files ignored due to path filters (1)
.storybook/preview.tsxis excluded by none and included by none
📒 Files selected for processing (7)
scripts/generate-call-theme.tssrc/components/call/widget/useElementCallWidget.test.tsxsrc/components/call/widget/useElementCallWidget.tssrc/components/designTokens/DesignTokens.stories.tsxsrc/components/designTokens/ElementCallEmbed.stories.tsxsrc/utils/theme/callTheme.test.tssrc/utils/theme/callTheme.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: lint, type-check, build and Storybook Docker validation
- GitHub Check: lint, type-check, test, build and Docker validation
- GitHub Check: lint, type-check, build and test frontend
- GitHub Check: lint, type-check and build frontend
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Keep shared behavior in hooks/utilities when multiple screens need the same state, route, selection, or formatting logic.
UI changes must have accessible focus/keyboard behavior and must not rely on color alone.
Files:
src/components/call/widget/useElementCallWidget.test.tsxsrc/components/call/widget/useElementCallWidget.tsscripts/generate-call-theme.tssrc/components/designTokens/ElementCallEmbed.stories.tsxsrc/components/designTokens/DesignTokens.stories.tsxsrc/utils/theme/callTheme.test.tssrc/utils/theme/callTheme.ts
**/*.{scss,sass,css,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Reuse existing design tokens, Sass mixins, and component patterns; avoid one-off hardcoded CSS for controls, active list states, focus rings, and responsive layout.
Files:
src/components/call/widget/useElementCallWidget.test.tsxsrc/components/call/widget/useElementCallWidget.tsscripts/generate-call-theme.tssrc/components/designTokens/ElementCallEmbed.stories.tsxsrc/components/designTokens/DesignTokens.stories.tsxsrc/utils/theme/callTheme.test.tssrc/utils/theme/callTheme.ts
src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx,js,jsx}: Check behavior against the surrounding ORISO flow, not only types.
Flag duplicated route/state logic, missing error/loading handling,
privacy regressions around Matrix/chat/drafts, and changes that should
be expressed through shared hooks or utilities instead of local flags.
Files:
src/components/call/widget/useElementCallWidget.test.tsxsrc/components/call/widget/useElementCallWidget.tssrc/components/designTokens/ElementCallEmbed.stories.tsxsrc/components/designTokens/DesignTokens.stories.tsxsrc/utils/theme/callTheme.test.tssrc/utils/theme/callTheme.ts
🧠 Learnings (2)
📚 Learning: 2026-07-18T03:56:46.133Z
Learnt from: kiodreambau
Repo: OpenResilienceInitiative/ORISO-Frontend PR: 516
File: src/components/groupChat/waitingClock/WaitingAreaCountdown.stories.tsx:0-0
Timestamp: 2026-07-18T03:56:46.133Z
Learning: When reviewing Storybook stories (e.g., `*.stories.tsx`), treat story-specific styling done via isolated inline decorators/fixtures as Storybook simulation, not as production design tokens or focus-style behavior. Do not flag the story’s styles as production focus/regression issues unless the change involves the actual production components (not the story wrapper/fixture).
Applied to files:
src/components/designTokens/ElementCallEmbed.stories.tsxsrc/components/designTokens/DesignTokens.stories.tsx
📚 Learning: 2026-08-03T08:28:22.927Z
Learnt from: Storypapst
Repo: OpenResilienceInitiative/ORISO-Frontend PR: 849
File: src/components/pseudonym/BotMessageAnimation.stories.tsx:0-0
Timestamp: 2026-08-03T08:28:22.927Z
Learning: In the ORISO frontend Storybook 10 setup, select viewports through Storybook `globals`, not `parameters.viewport.defaultViewport`. The built-in `mobile1` viewport is not registered. For stories claiming 375px or 390px widths, use the custom `phone375` (375×812) or `phone390` (390×844) viewports registered in `.storybook/preview.tsx`, via the exported `phone375Globals` and `phone390Globals` helpers.
Applied to files:
src/components/designTokens/ElementCallEmbed.stories.tsxsrc/components/designTokens/DesignTokens.stories.tsx
🔇 Additional comments (9)
src/components/call/widget/useElementCallWidget.ts (2)
164-169: LGTM!Also applies to: 284-287
171-177: 🎯 Functional CorrectnessVerify the
appConfiglifecycle for the E2EE kill-switch.
preparereads module-levelappConfigwhen it builds the URL, but this effect does not rerun whensetAppConfigmutates that value. If configuration loads or changes after URL construction, the call can keep the wrongperParticipantE2EEsetting. Confirm that configuration is initialized before this hook and remains immutable, or expose the effective toggle through reactive state.Source: Path instructions
src/components/call/widget/useElementCallWidget.test.tsx (2)
163-167: LGTM!
172-195: 🎯 Functional CorrectnessIsolate the module-level
appConfigstate in these tests.The positive assertion does not set an explicit enabled configuration, while the following test sets
enableCallMediaE2EEtofalse. Confirm thatbeforeEachorafterEachrestores the configuration. Otherwise, test order can change the generated URL and mask the default behavior.Source: Path instructions
src/utils/theme/callTheme.ts (1)
1-18: LGTM!Also applies to: 19-26, 27-31, 33-57, 58-67, 76-119, 133-170, 172-178, 180-197, 199-213
scripts/generate-call-theme.ts (1)
1-28: LGTM!Also applies to: 32-60
src/utils/theme/callTheme.test.ts (1)
1-56: LGTM!Also applies to: 57-92, 94-117, 119-141
src/components/designTokens/DesignTokens.stories.tsx (1)
102-105: LGTM!Also applies to: 127-131, 156-161
src/components/designTokens/ElementCallEmbed.stories.tsx (1)
8-12: LGTM!
| */ | ||
| const DEFAULT_OUT = path.join(__dirname, '..', CALL_THEME_ARTEFACT_PATH); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject flags with missing values.
argValue returns the next argument without checking it. With --out --check, the generator treats --check as the output path and checks the wrong file. With --seed --check, it passes a flag as a colour seed and reports a confusing palette error. Reject a missing value or a next argument beginning with --. (raw.githubusercontent.com)
Suggested validation
const argValue = (flag: string): string | undefined => {
const i = process.argv.indexOf(flag);
- return i === -1 ? undefined : process.argv[i + 1];
+ if (i === -1) return undefined;
+ const value = process.argv[i + 1];
+ if (!value || value.startsWith('--')) {
+ throw new Error(`Missing value for ${flag}`);
+ }
+ return value;
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/generate-call-theme.ts` around lines 29 - 31, Update the
argument-value handling in argValue so it rejects missing next arguments and any
value beginning with "--", including cases such as --out --check and --seed
--check, instead of treating the flag as a path or seed. Preserve normal value
parsing for valid non-flag arguments and ensure invalid usage reports the
existing argument error.
| // Must match the `theme=dark` pin in the widget URL — a ready-time | ||
| // update to any other scheme would flip the call after launch. | ||
| expect(api.updateTheme).toHaveBeenCalledWith({ name: 'dark' }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that the ready handler sends only the dark update.
toHaveBeenCalledWith passes if the mock receives both light and dark updates. Also assert toHaveBeenCalledTimes(1) or compare the complete call list to prevent a contradictory update from passing the test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/call/widget/useElementCallWidget.test.tsx` around lines 316 -
318, Strengthen the ready-handler assertion around api.updateTheme by verifying
it is called exactly once, in addition to checking the dark theme payload. Keep
the existing dark update expectation and prevent any light or contradictory
update from passing the test.
QA verification — Element Call ORISO theming (predev + Storybook)Ran through the full reviewer test plan against ✅ Item 1 — Storybook color tokens page 📎 ✅ Item 2 — Light values match the running app 📎 ✅ Item 3 — Toolbar scheme switch repaints live 📎 ✅ Item 4 — Call mapping page 📎 ✅ Item 5 — Live embed theme control ✅ Item 6 — Real group call opens dark, URL pinned 📎 ✅ Item 7 — Rest of the app stays light 📎 * ✅ Item 8 — Drift guard works All 8 items from the reviewer test plan verified. Also resolves the question I raised in code review about the old |
3 participants
Refs #896
Closes #897
Closes #898
Closes #899
Closes #900
Pairs with OpenResilienceInitiative/ORISO-ElementCall#40, which consumes the stylesheet this generates.
Why
Element Call was the last surface not on the ORISO colour contract, and there was no place anywhere to see the platform's colours. The Figma export was meant to be that place and had drifted eleven roles out of date — because it was a copy, and copies drift. Everything here is built so the same thing cannot happen again.
The reconciliation behind that number is in #896. Short version: 30 of 41 roles still matched; the differences were systematic (Figma reused light containers in dark, and used a warm seed-tinted background that the engine's own test suite would reject). Decision: the engine wins, in full.
What changed
Generator (#897) —
src/utils/theme/callTheme.tsmaps Compound's semantic--cpd-*roles onto engine--m3-*roles;scripts/generate-call-theme.tsemits the stylesheet the fork ships. A golden test compares the checked-in artefact against fresh engine output, so a re-tune that forgets to regenerate fails CI rather than shipping a stale call.Roles left unmapped on purpose, and why, are recorded in the module: the six-hue
decorativeset tells participant avatars apart and has no ORISO counterpart — collapsing it onto one brand colour would make every participant look identical.Storybook (#898, #899) — a colour-role page (light/dark/inverted, hex, contrast, AA failures flagged in red), the call mapping page, a live embed of the deployed call with a switchable theme, and a toolbar scheme switch that repaints any story.
Every swatch calls
computeOrisoPaletteat render time. There is no hex list to maintain, so the page cannot be wrong.Theme pin (#900) — the widget URL now carries
theme=darkexplicitly. The call was dark only because Element Call's own fallback picks dark when nothing is asked for; that made our appearance an accident of an upstream default. No visual change — this pins what already happens.Two findings worth your attention
The contrast assertions caught a real defect before it shipped.
--cpd-color-border-focusedinitially mapped onto--m3-primary-outline, a role the engine tunes to stay dark on light brand surfaces. On the dark canvas that renders the focus ring at 1.5:1 — a keyboard focus indicator nobody can see. Eyeballing a screenshot would have passed it.I had guessed two token names.
--cpd-color-text-primary-alphaand--cpd-color-text-secondary-alphafollow Compound's naming pattern but do not exist. Checked against the real token list and removed.--cpd-color-text-placeholderis also not a Compound role, but the fork reads it ininput/Input.module.css, so it stays — with that reason recorded next to it.Scope held deliberately
ACTIVE_SCHEMES.darkis stillfalse. The app ships light; only the embedded call is dark. Components will look wrong when you switch Storybook to dark — that is the inventory this makes visible, not a regression it introduces. Fixing them is a future app-dark decision.Follow-ups noted on #896: runtime tenant-token injection into the call (so a tenant's brand colour reaches the call UI instead of the default-tenant snapshot), extracting the engine as a shared package instead of generate-and-copy, and Playwright screenshot tests inside the fork.
Incidental
lint-stagedreformatted the generated artefact on commit and the drift test failed — correctly. Generated files are now in.prettierignore; the generator owns their bytes.vite-nodealso needed thenoExternaltreatment the vitest config already documents for@material/material-color-utilities.Verification
orisoTuning.tsmakes it fail; restoring it makes it pass.light #fcf9f9/dark #131314/inverted #303031).Reviewer test plan
themecontrol → the embedded call changes schemetheme=darkorisoTuning.tsand runnpm run test:unit→ the drift test fails and names the generatorSummary by CodeRabbit
New Features
Bug Fixes
Documentation