fix(dashboard): unfreeze animations broken by transition tokens used as durations#1386
Conversation
…as durations Spinners, status-dot pulses, and entrance animations across the dashboard rendered frozen: transition tokens (--transition-slow: 0.3s ease) bundle a duration AND an easing, and 15 animation declarations reused them as bare durations. Substituting "0.3s ease" next to an explicit easing (linear, ease-in-out, ease-out) — or inside calc() — makes the declaration invalid at computed-value time, which per spec resolves the entire property to animation: none with no console error. - add duration-only tokens (--duration-instant/fast/normal/slow) and derive the --transition-* tokens from them so the two cannot drift - switch all 15 broken animation declarations across 14 CSS files to the duration tokens, preserving effective durations - add animation-duration-tokens.css.test.ts: sweeps every app CSS file and fails on transition-token-as-duration, calc() over a transition token, and transition token in animation-duration Verified in a real browser against the production build: previously frozen .status-dot--connecting and calc-based NodesView spinners now report running animations; transition shorthands still resolve to "0.15s / ease". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 11 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR fixes a CSS animation regression where transition tokens (duration + easing) were incorrectly used as animation durations. The change introduces duration-only tokens, redefines transition tokens from those, updates all component animations, and adds validation to prevent recurrence. ChangesCSS Animation Token Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 1
🤖 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 `@packages/dashboard/app/__tests__/animation-duration-tokens.css.test.ts`:
- Around line 42-67: The current per-line checks in the lines.forEach loop miss
multiline declarations; update the test's findViolations logic to accumulate CSS
declarations across lines (e.g., join subsequent lines until a terminating ';'
or '}' or run the regexes against the full file/content chunk) before applying
the three detection rules (calc\(\s*var\(--transition-..., animation shorthand
check that uses EASING_KEYWORD, and animation-duration pattern). Modify the loop
that uses lines.forEach (or replace it) so animation/animation-duration/calc
patterns are evaluated on the assembled declaration string (referencing the
existing animationMatch handling and EASING_KEYWORD) to ensure multiline
declarations are detected.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e3447d76-ee67-4b18-ac4e-6e8ff7f2efdb
📒 Files selected for processing (17)
.changeset/fix-frozen-dashboard-animations.mdpackages/dashboard/app/__tests__/animation-duration-tokens.css.test.tspackages/dashboard/app/components/CustomProvidersSection.csspackages/dashboard/app/components/DbCorruptionBanner.csspackages/dashboard/app/components/DockerProvisioningStatus.csspackages/dashboard/app/components/FileMentionPopup.csspackages/dashboard/app/components/IssueMentionPopup.csspackages/dashboard/app/components/ListView.csspackages/dashboard/app/components/NodesView.csspackages/dashboard/app/components/OnboardingDisclosure.csspackages/dashboard/app/components/OnboardingResumeCard.csspackages/dashboard/app/components/SetupWizardModal.csspackages/dashboard/app/components/SystemStatsModal.csspackages/dashboard/app/components/TaskIdIntegrityBanner.csspackages/dashboard/app/components/TodoView.csspackages/dashboard/app/styles.csspackages/dashboard/vitest.config.ts
| lines.forEach((line, index) => { | ||
| const lineNo = index + 1; | ||
|
|
||
| // calc() over a duration+easing pair is always invalid. | ||
| if (/calc\(\s*var\(--transition-/.test(line)) { | ||
| violations.push(`line ${lineNo}: calc() over a transition token — ${line.trim()}`); | ||
| return; | ||
| } | ||
|
|
||
| // animation shorthand: a transition token plus an explicit easing keyword | ||
| // substitutes to two easing functions and invalidates the declaration. | ||
| const animationMatch = line.match(/animation\s*:\s*([^;}]*)/); | ||
| if (animationMatch && /var\(--transition-/.test(animationMatch[1])) { | ||
| const valueWithoutToken = animationMatch[1].replace(/var\(--transition-[a-z]+\)/g, ""); | ||
| if (EASING_KEYWORD.test(valueWithoutToken)) { | ||
| violations.push( | ||
| `line ${lineNo}: transition token + explicit easing in animation shorthand — ${line.trim()}`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // animation-duration longhand cannot hold "0.3s ease" either. | ||
| if (/animation-duration\s*:\s*[^;}]*var\(--transition-/.test(line)) { | ||
| violations.push(`line ${lineNo}: transition token as animation-duration — ${line.trim()}`); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Harden violation detection for multiline CSS declarations.
findViolations only inspects one line at a time, so multiline animation, animation-duration, or calc(var(--transition-...)) declarations can slip through undetected and reintroduce frozen animations without failing this test.
Suggested patch
function findViolations(css: string): string[] {
const violations: string[] = [];
- const lines = css.split("\n");
-
- lines.forEach((line, index) => {
- const lineNo = index + 1;
-
- // calc() over a duration+easing pair is always invalid.
- if (/calc\(\s*var\(--transition-/.test(line)) {
- violations.push(`line ${lineNo}: calc() over a transition token — ${line.trim()}`);
- return;
- }
-
- // animation shorthand: a transition token plus an explicit easing keyword
- // substitutes to two easing functions and invalidates the declaration.
- const animationMatch = line.match(/animation\s*:\s*([^;}]*)/);
- if (animationMatch && /var\(--transition-/.test(animationMatch[1])) {
- const valueWithoutToken = animationMatch[1].replace(/var\(--transition-[a-z]+\)/g, "");
- if (EASING_KEYWORD.test(valueWithoutToken)) {
- violations.push(
- `line ${lineNo}: transition token + explicit easing in animation shorthand — ${line.trim()}`,
- );
- }
- }
-
- // animation-duration longhand cannot hold "0.3s ease" either.
- if (/animation-duration\s*:\s*[^;}]*var\(--transition-/.test(line)) {
- violations.push(`line ${lineNo}: transition token as animation-duration — ${line.trim()}`);
- }
- });
+ const declarationRegex = /([a-z-]+)\s*:\s*([\s\S]*?);/gi;
+ let match: RegExpExecArray | null;
+ while ((match = declarationRegex.exec(css)) !== null) {
+ const [fullDecl, prop, value] = match;
+ const lineNo = css.slice(0, match.index).split("\n").length;
+ const decl = fullDecl.replace(/\s+/g, " ").trim();
+
+ if (/calc\(\s*var\(--transition-/.test(value)) {
+ violations.push(`line ${lineNo}: calc() over a transition token — ${decl}`);
+ continue;
+ }
+
+ if (prop === "animation" && /var\(--transition-/.test(value)) {
+ const valueWithoutToken = value.replace(/var\(--transition-[a-z]+\)/g, "");
+ if (EASING_KEYWORD.test(valueWithoutToken)) {
+ violations.push(
+ `line ${lineNo}: transition token + explicit easing in animation shorthand — ${decl}`,
+ );
+ }
+ }
+
+ if (prop === "animation-duration" && /var\(--transition-/.test(value)) {
+ violations.push(`line ${lineNo}: transition token as animation-duration — ${decl}`);
+ }
+ }
return violations;
}🤖 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 `@packages/dashboard/app/__tests__/animation-duration-tokens.css.test.ts`
around lines 42 - 67, The current per-line checks in the lines.forEach loop miss
multiline declarations; update the test's findViolations logic to accumulate CSS
declarations across lines (e.g., join subsequent lines until a terminating ';'
or '}' or run the regexes against the full file/content chunk) before applying
the three detection rules (calc\(\s*var\(--transition-..., animation shorthand
check that uses EASING_KEYWORD, and animation-duration pattern). Modify the loop
that uses lines.forEach (or replace it) so animation/animation-duration/calc
patterns are evaluated on the assembled declaration string (referencing the
existing animationMatch handling and EASING_KEYWORD) to ensure multiline
declarations are detected.
Greptile SummaryThis PR fixes all frozen dashboard animations caused by CSS IACVT (invalid-at-computed-value-time):
Confidence Score: 5/5Safe to merge — all 15 broken animation declarations are corrected, the token redesign is backward-compatible, and a regression test enforces the new constraint going forward. The CSS changes are mechanical and verified: each --transition-* reference in an animation context is replaced with its --duration-* counterpart, durations are numerically identical, and every existing transition: consumer is untouched. The regression test covers all three known violation shapes and is correctly registered in the test suite. The only gap found is a narrow false-positive scenario in the multi-animation comma-list check, which has no current manifestation in the codebase. No files require special attention. The test logic at animation-duration-tokens.css.test.ts lines 53-60 has a theoretical false-positive for comma-separated multi-animation values, but no such pattern currently exists in the codebase. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["--transition-slow: 0.3s ease\n(before: single token)"] -->|"used as bare duration in animation:"| B["animation: spin 0.3s ease linear infinite\n(after var() substitution)"]
B --> C["Two easing-functions: ease + linear\n→ IACVT: whole declaration dropped"]
C --> D["animation: none — spinner frozen\n(no error, no DevTools warning)"]
E["--duration-slow: 0.3s\n(new bare-duration token)"] -->|"used in animation:"| F["animation: spin 0.3s linear infinite\n(after var() substitution)"]
F --> G["Valid — one easing, one duration\n→ animation runs correctly"]
E -->|"derived into"| H["--transition-slow: var(--duration-slow) ease\n(transition token stays derived)"]
H -->|"used in transition:"| I["transition: color 0.3s ease\n→ unchanged, still valid"]
style D fill:#f66,color:#fff
style G fill:#2a2,color:#fff
Reviews (2): Last reviewed commit: "docs: capture IACVT frozen-animation lea..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| lines.forEach((line, index) => { | ||
| const lineNo = index + 1; | ||
|
|
||
| // calc() over a duration+easing pair is always invalid. | ||
| if (/calc\(\s*var\(--transition-/.test(line)) { | ||
| violations.push(`line ${lineNo}: calc() over a transition token — ${line.trim()}`); | ||
| return; | ||
| } | ||
|
|
||
| // animation shorthand: a transition token plus an explicit easing keyword | ||
| // substitutes to two easing functions and invalidates the declaration. | ||
| const animationMatch = line.match(/animation\s*:\s*([^;}]*)/); | ||
| if (animationMatch && /var\(--transition-/.test(animationMatch[1])) { | ||
| const valueWithoutToken = animationMatch[1].replace(/var\(--transition-[a-z]+\)/g, ""); | ||
| if (EASING_KEYWORD.test(valueWithoutToken)) { | ||
| violations.push( | ||
| `line ${lineNo}: transition token + explicit easing in animation shorthand — ${line.trim()}`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // animation-duration longhand cannot hold "0.3s ease" either. | ||
| if (/animation-duration\s*:\s*[^;}]*var\(--transition-/.test(line)) { | ||
| violations.push(`line ${lineNo}: transition token as animation-duration — ${line.trim()}`); | ||
| } | ||
| }); |
There was a problem hiding this comment.
findViolations doesn't strip inline CSS comments before pattern-matching
The findViolations function checks every raw line including the content of CSS block comments. A single-line comment like /* animation: spin var(--transition-slow) linear infinite; */ matches the animation\s*:\s* regex, and because valueWithoutToken would still contain linear, it would be counted as a violation — triggering a false-positive test failure. Any future developer who comments out an old animation declaration to document the before-state (e.g., as a migration guide or debug trace) would get a spurious test failure with no obvious cause. Stripping inline block comments (line.replace(/\/\*.*?\*\//g, "")) before the three pattern checks would prevent this.
| const animationMatch = line.match(/animation\s*:\s*([^;}]*)/); | ||
| if (animationMatch && /var\(--transition-/.test(animationMatch[1])) { | ||
| const valueWithoutToken = animationMatch[1].replace(/var\(--transition-[a-z]+\)/g, ""); | ||
| if (EASING_KEYWORD.test(valueWithoutToken)) { | ||
| violations.push( | ||
| `line ${lineNo}: transition token + explicit easing in animation shorthand — ${line.trim()}`, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
animation declarations are silently skipped
The regex ([^;}]*) on a single line means that if an animation value is written across multiple lines (e.g., value starts on the next line after the colon), the capture group on the property line will be an empty string and the violation check is bypassed. While multi-line animation declarations are rare in this codebase, the guard test in "flags the frozen-spinner pattern" only exercises single-line inputs, so a future multi-line violation would go undetected without any indication the guard is incomplete. A brief comment in findViolations noting this limitation would at least make the gap intentional and visible.
Add docs/solutions/ui-bugs/ learning for the silent animation freeze caused by transition tokens (duration+easing pairs) used as bare durations, and update dashboard-guide.md design-token and pitfalls sections to cover the new --duration-* tokens and the IACVT gotcha. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
Dashboard spinners and pulse animations rendered completely frozen — the in-progress status dots stopped pulsing, the Nodes/Todo view loaders and mention-popup spinners sat motionless, and onboarding/setup entrance animations never played. They all animate again.
The cause was invisible in devtools: the transition tokens bundle a duration and an easing (
--transition-slow: 0.3s ease), and 15 animation declarations reused them as bare durations. Substituting0.3s easeinto ananimationshorthand that names its own easing produces two<easing-function>values — and per the custom-properties spec, that makes the declaration invalid at computed-value time, resolving the whole property toanimation: nonewith no parse error and no console warning.calc(var(--transition-slow) * 4)fails the same way (you can't multiply0.3s ease). This is also why the two prior spinner fixes (FN-5855, FN-5913) didn't catch it: they covered the.animate-spinutility, which uses a literal1sand was never affected.What changed
--duration-instant/fast/normal/slow; the--transition-*tokens are now derived from them (var(--duration-slow) ease) so the two sets can't drift apart. Existingtransition:usages are unaffected.animation-duration-tokens.css.test.ts) sweeps every CSS file underapp/and fails on the three invalid shapes: transition token + explicit easing in ananimationshorthand,calc()over a transition token, and a transition token inanimation-duration. It flagged all 15 sites red before the fix.Test plan
.status-dot--connectingwent fromanimationName: none/ 0 running animations →status-dot-pulse, 0.3s, ease-in-out, running; the calc-based NodesView pattern resolves to1.2sand runs;transition: color var(--transition-fast)still computes to0.15s / easeSummary by CodeRabbit