Repository navigation
Conversation
The shared web-test-runner config compiled tests without the package
tsconfig, so esbuild emitted standard decorators and Lit's legacy
decorators crashed on import ("Cannot read properties of undefined
(reading 'constructor')"). No design package test could run.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Extend the shared GlobalStyles of all packages: - `surface` attribute (opt-in): the component paints its own background and text color from --ecc-background / --ecc-foreground, so a theme (e.g. dark) works independently of the host page. - --ecc-font-sans and --ecc-font-mono tokens; without them, the page font is still inherited. Tailwind's font-mono now follows the token. - Dark defaults are now set as inherited private --_ecc-default-* properties in a separate :host([dark]) rule. Previously, :host([dark]) shared a selector list with :host-context(), which Firefox does not support, so the whole rule was dropped and the dark attribute did nothing in Firefox. Nested components now inherit dark defaults without needing :host-context(). - Dark --destructive defaults to a lighter red (shadcn's current value) so it can be used as error text on dark backgrounds; dark --destructive-foreground becomes near-white for solid badges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the hard-coded cloud9_day/cloud9_night Ace themes with an "ecc"
theme whose CSS reads the design tokens (background, foreground, muted
gutter, primary keys/keywords, chart-derived strings and numbers,
--ecc-font-mono). The editor now follows custom themes and dark mode in
CSS, so the JavaScript dark-mode detection is removed: it only watched
the class of <html>/<body>, and its this.matches(":host-context(...)")
checks never match from outside a shadow root.
Add tests for the surface attribute, font tokens, dark mode (incl.
nested components) and the editor theme.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Error banners used text-destructive-foreground on a 10% destructive tint. That token is meant for text on a solid destructive background and defaults to near-white in light mode, so error messages were practically invisible. Use text-destructive instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Document the typography tokens, dark mode behavior (including the Firefox limitation for ancestor-based detection), the opt-in surface attribute, how the code editor maps tokens, and the new dark destructive defaults. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A painted surface without inset put content flush against its edge. Components with the surface attribute now get padding from the new --ecc-surface-padding token (default 1rem) and rounded corners from --ecc-radius. Also extend tests (surface padding, light-mode control, .dark ancestor where :host-context() is supported, editor gutter token) and correct the docs on which tokens the code editor uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use a foreground tint for the active line instead of --accent, which produced a heavy colored band in themes with a saturated accent. Name the syntax color helper and its blend ratio, and explain the static isDark flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Hosts styled width: 100% overflowed their container by twice the surface padding. Use border-box sizing on surfaces. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
White text on the lighter dark-mode --destructive was about 2.9:1; use a dark --destructive-foreground (about 6:1). Document that only Chromium supports ancestor-based dark mode (Safari lacks :host-context() too) and that error text now uses --ecc-destructive. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ace switches its fold arrow images via the ace_dark class, which the token-driven theme cannot set, so arrows were dark on dark gutters. Draw them as masks filled with --muted-foreground instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Assert that fold arrows use the token-colored mask instead of Ace's images, explain why the width test uses the code editor, and tighten the ace-theme comments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The destructive button hard-coded text-white, which is about 2.9:1 on the lighter dark-mode --destructive (its dark: variant cannot match across the shadow boundary). Use --destructive-foreground like the badge; the light default stays near-white. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep a destructive tint for invalid fold widgets, which the token fold-arrow rule otherwise overrode. Wait for the gutter with waitUntil instead of a fixed timeout, and document that surface is meant for block-level components. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's GuideThis PR centralizes component theming around inherited ECC tokens, adding opt-in surface painting and configurable fonts, fixing cross-browser dark-mode propagation and destructive contrast, and replacing the code editor’s JavaScript-driven Ace themes with a CSS token-driven theme. It documents and tests the new behavior across Chromium and Firefox while repairing shared test-runner configuration and parallel package test execution. Sequence diagram for token-driven code editor initializationsequenceDiagram
participant Component as EccUtilsDesignCode
participant Ace as Ace editor
participant CSS as ECC token CSS
Component->>Ace: initializeAceEditor()
Component->>Ace: setTheme(ACE_THEME)
Ace->>CSS: Apply ace-ecc styles
CSS-->>Ace: Resolve background, syntax colors, selection, fold arrows and font from tokens
Flow diagram for cross-browser dark mode resolutionflowchart TD
Start["Component renders"] --> Explicit{dark attribute on component?}
Explicit -->|Yes| Defaults["Set inherited --_ecc-default-* dark properties"]
Explicit -->|No| Ancestor{dark ancestor selector supported?}
Ancestor -->|Yes| Defaults
Ancestor -->|No| Light["Keep light defaults"]
Defaults --> Tokens["Resolve --ecc-* tokens"]
Light --> Tokens
Tokens --> Override["Explicit --ecc-* tokens take precedence"]
Flow diagram for opt-in surface paintingflowchart LR
Component["Component host"] --> Default["No surface attribute: transparent and page-integrated"]
Component --> OptIn{surface attribute}
OptIn --> Painted["Paint --ecc-background and --ecc-foreground"]
Painted --> Layout["Apply --ecc-surface-padding, --ecc-radius and box-sizing"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Seven packages ran wtr with no test files, which exits with "Could not find any test files" and failed the turbo test run (and so the Test checks job of every PR) before packages with tests could finish. Without a test script, turbo skips them. Re-add the script together with the first test file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anuragxxd
reviewed
Oct 23, 2026
anuragxxd
left a comment
anuragxxd
left a comment
Member
There was a problem hiding this comment.
Looks good overall, thanks! The dark-mode inheritance fix and the CI test-script cleanup both look right.
One bug before merge: --ecc-font-sans has no effect on ecc-client-ga4gh-wes-run-create. run-create.ts:35-37 lists [GlobalStyles, TailwindStyles], so Tailwind's :host { font-family: inherit } (not in a layer, same specificity) overrides it. Every other component uses [TailwindStyles, GlobalStyles]. Swapping the order fixes it. A font test on one client-package component would catch this, since the current tests only cover badge.
run-create listed GlobalStyles before TailwindStyles, so Tailwind's
unlayered :host { font-family: inherit } overrode --ecc-font-sans.
Use the order every other component uses. Also fix its error message
color: hsl(var(--destructive)) wrapped an oklch() token and was
invalid, so errors were not colored at all.
Add WES theming tests (font token on all three components, run-create
error color) and a test script using the shared Playwright config.
Reported in review of #446.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test:watch still used the per-package wtr config (default Chrome launcher, development exports, no tsconfig), so watch runs differed from npm test. Mirror the design package scripts and remove the WES config. Clarify the theming test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
turbo runs package tests in parallel. With two packages using the shared wtr config, both could pick port 8000 between portfinder's check and the bind, failing one run with EADDRINUSE (reproduced in review). Pin design to 8000 and wes to 8001. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
Thanks, good catch! Fixed in b0b3e04: swapped the order, and also fixed run-create's error color ( |
This was referenced Oct 26, 2026
uniqueg
marked this pull request as ready for review
October 27, 2026 01:11
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/ecc-utils-design/src/components/code/ace-theme.ts" line_range="123" />
<code_context>
+// globalThis rather than window: this module is also evaluated during SSR
+const { ace } = globalThis as any;
+
+ace.define(
+ ACE_THEME,
+ ["require", "exports", "module", "ace/lib/dom"],
</code_context>
<issue_to_address>
**SSR imports fail without Ace**
When the design package is imported during server-side rendering where `ace-builds` does not set `globalThis.ace`, the module-level `ace.define(...)` call runs with `ace` undefined and throws, so importing the design package during SSR fails.
Guard the registration so it runs only when the Ace global is available.
Also at `packages/ecc-utils-design/src/components/code/ace-theme.ts:121-122`.
</issue_to_address>
### Comment 2
<location path="packages/ecc-utils-design/src/components/code/code.ts" line_range="666" />
<code_context>
- });
- }
-
- isDarkMode() {
- return (
- document.documentElement.classList.contains("dark") ||
</code_context>
<issue_to_address>
**Public editor methods are missing**
When an external consumer calls a previously public editor method, `EccUtilsDesignCode` no longer exposes `isDarkMode()`, `updateEditorTheme()`, or `setupThemeObserver()`, so TypeScript calls fail or runtime calls throw a `TypeError`, breaking downstream consumers.
Restore the removed public editor methods on `EccUtilsDesignCode`.
</issue_to_address>
### Comment 3
<location path="packages/ecc-utils-design/src/components/button/button.ts" line_range="22" />
<code_context>
default: "bg-primary text-primary-foreground shadow hover:bg-primary/90",
destructive:
- "bg-destructive text-white shadow-xs hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40 dark:bg-destructive/60",
+ "bg-destructive text-destructive-foreground shadow-xs hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40 dark:bg-destructive/60",
outline:
"border bg-background shadow-xs hover:bg-accent hover:text-accent-foreground dark:bg-input/30 dark:border-input dark:hover:bg-input/50",
</code_context>
<issue_to_address>
**Dark button variants stay inactive**
When a button using a `dark:*` variant is rendered in the new attribute-based dark mode, the new dark-token handling does not add a `.dark` ancestor inside the button’s shadow root, but Tailwind’s dark variant matches only `.dark *` (`packages/ecc-utils-design/src/tailwind.css:4`). Thus `dark:bg-destructive/60` and the other `dark:*` button utilities do not apply when dark mode is activated with the documented `dark` attribute or an ancestor theme, so the button keeps its non-dark variant styling.
Make the dark variant match the host’s `dark` attribute and supported dark ancestors across the shadow boundary, or replace these dark-specific utilities with token-driven styles.
</issue_to_address>
### Comment 4
<location path="packages/ecc-utils-design/src/global.test.ts" line_range="190-191" />
<code_context>
+ await waitUntil(() => inShadow(el, ".ace_fold-widget"));
+ const widget = inShadow(el, ".ace_fold-widget");
+ const style = getComputedStyle(widget);
+ expect(style.backgroundImage).to.equal("none");
+ expect(style.backgroundColor).to.equal("rgb(11, 22, 33)");
+ });
+
</code_context>
<issue_to_address>
**Fold arrows can render as solid blocks**
When the fold-arrow mask is removed or stops applying, the fold-widget test checks only that `backgroundImage` is `none` and `backgroundColor` matches the token; both assertions still pass if the mask that shapes the widget into an arrow is missing, so the test passes while users see a colored block instead of an arrow.
Assert that the computed mask is present, in addition to checking the token color and absence of Ace's background image.
</issue_to_address>Sourcery assessment
Approval pending. 4 findings to address first.
Blocking findings: packages/ecc-utils-design/src/components/code/ace-theme.ts:123, packages/ecc-utils-design/src/components/code/code.ts:666, packages/ecc-utils-design/src/components/button/button.ts:22, packages/ecc-utils-design/src/global.test.ts:191
| // globalThis rather than window: this module is also evaluated during SSR | ||
| const { ace } = globalThis as any; | ||
|
|
||
| ace.define( |
Contributor
There was a problem hiding this comment.
🟡 Medium · SSR imports fail without Ace
When the design package is imported during server-side rendering where ace-builds does not set globalThis.ace, the module-level ace.define(...) call runs with ace undefined and throws, so importing the design package during SSR fails.
Guard the registration so it runs only when the Ace global is available.
Also at packages/ecc-utils-design/src/components/code/ace-theme.ts:121-122.
Prompt for AI agents
In `packages/ecc-utils-design/src/components/code/ace-theme.ts` at line 123:
**SSR imports fail without Ace**
When the design package is imported during server-side rendering where `ace-builds` does not set `globalThis.ace`, the module-level `ace.define(...)` call runs with `ace` undefined and throws, so importing the design package during SSR fails.
Guard the registration so it runs only when the Ace global is available.
Also at `packages/ecc-utils-design/src/components/code/ace-theme.ts:121-122`.| }); | ||
| } | ||
|
|
||
| isDarkMode() { |
Contributor
There was a problem hiding this comment.
🟡 Medium · Public editor methods are missing
When an external consumer calls a previously public editor method, EccUtilsDesignCode no longer exposes isDarkMode(), updateEditorTheme(), or setupThemeObserver(), so TypeScript calls fail or runtime calls throw a TypeError, breaking downstream consumers.
Restore the removed public editor methods on EccUtilsDesignCode.
Prompt for AI agents
In `packages/ecc-utils-design/src/components/code/code.ts` at line 666:
**Public editor methods are missing**
When an external consumer calls a previously public editor method, `EccUtilsDesignCode` no longer exposes `isDarkMode()`, `updateEditorTheme()`, or `setupThemeObserver()`, so TypeScript calls fail or runtime calls throw a `TypeError`, breaking downstream consumers.
Restore the removed public editor methods on `EccUtilsDesignCode`.| default: "bg-primary text-primary-foreground shadow hover:bg-primary/90", | ||
| destructive: | ||
| "bg-destructive text-white shadow-xs hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40 dark:bg-destructive/60", | ||
| "bg-destructive text-destructive-foreground shadow-xs hover:bg-destructive/90 focus-visible:ring-destructive/20 dark:focus-visible:ring-destructive/40 dark:bg-destructive/60", |
Contributor
There was a problem hiding this comment.
🟡 Medium · Dark button variants stay inactive
When a button using a dark:* variant is rendered in the new attribute-based dark mode, the new dark-token handling does not add a .dark ancestor inside the button’s shadow root, but Tailwind’s dark variant matches only .dark * (packages/ecc-utils-design/src/tailwind.css:4). Thus dark:bg-destructive/60 and the other dark:* button utilities do not apply when dark mode is activated with the documented dark attribute or an ancestor theme, so the button keeps its non-dark variant styling.
Make the dark variant match the host’s dark attribute and supported dark ancestors across the shadow boundary, or replace these dark-specific utilities with token-driven styles.
Prompt for AI agents
In `packages/ecc-utils-design/src/components/button/button.ts` at line 22:
**Dark button variants stay inactive**
When a button using a `dark:*` variant is rendered in the new attribute-based dark mode, the new dark-token handling does not add a `.dark` ancestor inside the button’s shadow root, but Tailwind’s dark variant matches only `.dark *` (`packages/ecc-utils-design/src/tailwind.css:4`). Thus `dark:bg-destructive/60` and the other `dark:*` button utilities do not apply when dark mode is activated with the documented `dark` attribute or an ancestor theme, so the button keeps its non-dark variant styling.
Make the dark variant match the host’s `dark` attribute and supported dark ancestors across the shadow boundary, or replace these dark-specific utilities with token-driven styles.
Comment on lines
+190
to
+191
| expect(style.backgroundImage).to.equal("none"); | ||
| expect(style.backgroundColor).to.equal("rgb(11, 22, 33)"); |
Contributor
There was a problem hiding this comment.
🟡 Medium · Fold arrows can render as solid blocks
When the fold-arrow mask is removed or stops applying, the fold-widget test checks only that backgroundImage is none and backgroundColor matches the token; both assertions still pass if the mask that shapes the widget into an arrow is missing, so the test passes while users see a colored block instead of an arrow.
Assert that the computed mask is present, in addition to checking the token color and absence of Ace's background image.
Prompt for AI agents
In `packages/ecc-utils-design/src/global.test.ts` at lines 190-191:
**Fold arrows can render as solid blocks**
When the fold-arrow mask is removed or stops applying, the fold-widget test checks only that `backgroundImage` is `none` and `backgroundColor` matches the token; both assertions still pass if the mask that shapes the widget into an arrow is missing, so the test passes while users see a colored block instead of an arrow.
Assert that the computed mask is present, in addition to checking the token color and absence of Ace's background image.
This was referenced Oct 27, 2026
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Extends the native theming controls of all component packages, and fixes theming bugs found by trying to restyle the WES components using only those controls.
New controls
surfaceattribute (opt-in). Components stay transparent by default. Withsurface, a component paints its own background and text color from--ecc-backgroundand--ecc-foreground, inset by the new--ecc-surface-padding(default1rem) and rounded by--ecc-radius. A component can then use a dark (or any other) theme on any page.--ecc-font-sansand--ecc-font-monotokens. Without them, components still inherit the page font. Tailwind'sfont-mono(run IDs etc.) and the code editor follow--ecc-font-mono.cloud9_day/cloud9_nightAce themes. Background, gutter, syntax colors, selection, fold arrows and font all derive from the tokens, so the editor matches custom and dark themes.Fixes
darkattribute did nothing in Firefox.:host([dark])shared a selector list with:host-context(), which Firefox does not support, so Firefox dropped the whole rule. Dark defaults are now set as inherited private--_ecc-default-*properties in a separate:host([dark])rule. Components nested inside other components inherit them without needing:host-context(). Explicit--ecc-*tokens still win.text-destructive-foreground(near-white) on a 10% destructive tint; they now usetext-destructive. To keep error text readable in dark mode, the dark--ecc-destructivedefault is nowoklch(0.704 0.191 22.216)(shadcn's current value). The dark--ecc-destructive-foregroundis nowoklch(0.205 0 0), and the destructive button uses that token instead of hard-codedtext-white, so solid destructive badges and buttons stay readable.tsconfigto esbuild, so Lit's decorators crashed on import.Docs:
apps/documentation/pages/docs/customization.mdxcovers all of the above.Behavior changes to be aware of
ecc-utils-design-codeno longer has the public methodsisDarkMode(),updateEditorTheme()andsetupThemeObserver(); nothing in the repo calls them. The editor now goes dark through tokens like every other component, not by reading<html>/<body>classes in JavaScript.<html class="dark">in Firefox or Safari no longer darkens only the editor. It never darkened the rest of the components in those browsers, because:host-context()is Chromium-only. The editor is now consistent with the rest.--ecc-destructive. Apps that copied the old dark default (oklch(0.396 …)) into their own theme get dark red error text on dark backgrounds; the docs include a note on this.How it was verified
npm run build(all 9 turbo tasks, including the docs site): passes.npm run lint: passes, 0 errors. Warning counts are unchanged except design (11 → 13,anycasts around the globalaceobject, matching existing code).packages/ecc-utils-design/src/global.test.ts, 15 tests:.darktest is feature-detected because Firefox lacks:host-context().global.tsswapped back in, the Firefoxdarkattribute tests fail, as do the surface and font tests, confirming they cover the fixes.packages/ecc-client-ga4gh-wes/src/theming.test.ts:--ecc-font-sanson runs, run and run-create (catches the style-order bug from review), and run-create's error color. Chromium 4/4, Firefox 4/4. Package tests run on fixed ports (design 8000, WES 8001), because two packages now run in parallel and could otherwise both pick 8000 (EADDRINUSE, reproduced in review).libevent-2.1-7t64andlibavif16), but it runs in CI, see below.dpl_6DyaMFtU1Mi7mmyv1gUd5BD7qBq1), which is too fast for install ornext buildto have run. By comparison, chore(deps-dev): bump handlebars from 4.7.8 to 4.7.9 #441's successful deployment took about 6 minutes. This points to a Vercel project or account setting (for example a discontinued Node.js version selected in the dashboard, or a plan limit), not to the code. The only change underapps/iscustomization.mdx, and the docs site builds withnpm run buildlocally (Node 22) and in CI (Node 18 and 20). The other recent non-bot PRs (feat(ssr): make cloud components safe for SSR and hydration #431, feat: complete mock API layer implementation #432, docs(ecc-utils-design): fix missing exports in custom elements manifests #436) never got that far: they stopped at "Authorization required to deploy". See Open questions.Declined review findings
main, every package'stailwind.cssalready has an unlayered:host { font-family: inherit }, which beats Tailwind's layered preflight. The "inherits the page font" test also passes against the old styles.global.tscopies drift-proof (shared module or identity check). The 8 copies are byte-identical, which is the existing repo convention. Moving them into one shared module is a worthwhile follow-up refactor but outside this PR's scope.v*tag runspublish.yml, which sets all package versions, commitsbump: v…tomainand publishes to npm. The changesets flow (version.yml, "Version Packages" PRs) was last used in December 2024 (Version Packages #383). Adding a changeset would revive that parallel flow and open a competing version PR.display: blockonsurfacehosts. That would override the layout of inline design components such as badges and buttons. The docs instead state thatsurfaceis meant for top-level, block-level (service) components.oklch(0.396 …)shipped in published releases (e.g. 2.0.0-alpha.48).Unrelated fixes
ci: drop test scripts from packages without tests(85d8bdc). Seven packages ranwtrwithout any test files.wtrthen exits with "Could not find any test files", which failed the turbo test run, and with it the Test checks job of every PR since at least February, before the packages with tests could finish. Without atestscript, turbo skips these packages; add the script back together with a package's first test.test: honor package tsconfig in shared wtr config(769ebf9). Without it, the design package's tests could not even be imported.Open questions
elixir-cloud-aaiVercel team needs to checknpx vercel inspect dpl_6DyaMFtU1Mi7mmyv1gUd5BD7qBq1 --logsor the project's Node.js version setting.hsl(var(--success)), but no--successtoken exists, so the message is never colored. Should we add an--ecc-successtoken, or reuse an existing one (e.g.--ecc-chart-2)? Left unchanged here.Checklist
anycasts, see above.)🤖 Generated with Claude Code
Summary by Sourcery
Extend native component theming with surface, typography, and token-driven editor controls while fixing cross-browser dark mode and error contrast issues.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: