fix: add global spinner animation to prevent stuck loaders#40
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a global ChangesGlobal Spinner Animation
Test Hooks / Mocks
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Greptile SummaryThis PR fixes frozen spinner animations by promoting
Confidence Score: 5/5Safe to merge — the CSS change is correctly placed at the top level and the test change adds a missing mock stub. The No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["styles.css loads on every page"] --> B["@keyframes spin defined at top level"]
B --> C[".animate-spin class available"]
B --> D["inline animation: spin ... resolves"]
C --> E["25+ components using className=animate-spin"]
D --> F["BackgroundTasksIndicator inline style"]
E --> G["Spinners animate correctly on all pages"]
F --> G
Reviews (7): Last reviewed commit: "Merge branch 'main' into timothyjlaurent..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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/styles.css`:
- Around line 167-176: Move the `@keyframes` spin and .animate-spin rules out of
any :root block to the stylesheet top level (so the keyframes and utility class
are not nested); update the .animate-spin animation duration to use a design
token like var(--animation-duration-spin) instead of the hardcoded 1s, and
add/ensure the token definition exists inside :root (e.g.,
--animation-duration-spin: 1s) so the duration is configurable and follows the
coding guidelines.
🪄 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: 93400971-62e9-4a18-b94f-6611e148245e
📒 Files selected for processing (1)
packages/dashboard/app/styles.css
The @Keyframes spin and .animate-spin rules were incorrectly placed inside the first :root {} block, which is invalid CSS — @Keyframes cannot be nested inside selector blocks. Browsers silently ignore them in that position, so the spinners would still get stuck on pages without component CSS loaded. Moves the rules to the stylesheet top level between the two :root blocks. Also fixes the mobile-css regression test that parses the first :root block with a non-greedy regex. Addresses review feedback from Greptile (P1) and CodeRabbit on PR Runfusion#40. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addressing review feedbackGreptile P1 — Greptile P2 — Duplicate keyframes in component CSS: Acknowledged. The ~10 copies in CodeRabbit — Design token for duration: The existing component copies hardcode various values ( CI — Test shard 3/3 failure: Caused by the same |
The `.animate-spin` CSS utility class and `@keyframes spin` were only defined in TaskCard.css but used by 25+ components (~60 instances). On pages without a TaskCard rendered, the class didn't exist in the stylesheet, causing all spinners to appear frozen. Moves both the keyframes and utility class to the global styles.css so they're available from the first paint regardless of which components are loaded. Closes Runfusion#39 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The @Keyframes spin and .animate-spin rules were incorrectly placed inside the first :root {} block, which is invalid CSS — @Keyframes cannot be nested inside selector blocks. Browsers silently ignore them in that position, so the spinners would still get stuck on pages without component CSS loaded. Moves the rules to the stylesheet top level between the two :root blocks. Also fixes the mobile-css regression test that parses the first :root block with a non-greedy regex. Addresses review feedback from Greptile (P1) and CodeRabbit on PR Runfusion#40. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
timothyjlaurent
force-pushed
the
timothyjlaurent/stuck-spinners
branch
from
fe82bb0 to
c68bfff
Compare
The options parameter in __setCreateFnAgent's inline arrow already uses any to match the module-level let (which has its own suppress comment). Add the missing eslint-disable-next-line so CI passes. Unrelated to the spinner fix — pre-existing on main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@copilot resolve the merge conflicts in this pull request |
…SS change) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Flaky test in Test shard 2/3The Pushed an empty commit to re-trigger CI. All other checks were green:
|
The MissionLoop calls this.missionStore.listMissions() during startup recovery, but the mock TaskStore's getMissionStore() return value didn't include this method. When the runtime startup sequence raced ahead, it would hit "listMissions is not a function" — making the test flaky. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The @Keyframes spin and .animate-spin rules were incorrectly placed inside the first :root {} block, which is invalid CSS — @Keyframes cannot be nested inside selector blocks. Browsers silently ignore them in that position, so the spinners would still get stuck on pages without component CSS loaded. Moves the rules to the stylesheet top level between the two :root blocks. Also fixes the mobile-css regression test that parses the first :root block with a non-greedy regex. Addresses review feedback from Greptile (P1) and CodeRabbit on PR Runfusion#40. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Summary
@keyframes spinand.animate-spinfromTaskCard.cssto the globalstyles.css, making the spinner animation available on every page from the first paintProblem
The
.animate-spinCSS class (used by 25+ components, ~60 instances) was defined only inTaskCard.css. On pages that don't render aTaskCard(loading screen, settings, agent detail, etc.), the class didn't exist in the stylesheet, so allLoader2spinners appeared frozen.Similarly,
BackgroundTasksIndicator.tsxused inlinestyle={{ animation: "spin 1s linear infinite" }}without defining@keyframes spinin its own CSS — relying on other components' CSS files being loaded.Test Plan
vite buildpasses.animate-spinclass and@keyframes spinappear in the main CSS bundle (index-*.css).animate-spinis defined beforeTaskCardrendersCloses #39
🤖 Generated with Claude Code
Summary by CodeRabbit