Fix browser compatibility: guard requestIdleCallback and startViewTransition#1464
Conversation
…nsition Co-Authored-By: Konstantin Wohlwend <n2d4xc@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds runtime compatibility checks for two browser APIs in the dashboard. ThemeToggle now guards ChangesBrowser API Compatibility Guards
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds graceful fallbacks for newer browser APIs (Idle Callback + View Transitions) to improve compatibility across environments.
Changes:
- Add
requestIdleCallbackfeature detection with asetTimeoutfallback inuseWaitForIdle - Add
document.startViewTransitionfeature detection with a directsetThemefallback inThemeToggle
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/dashboard/src/hooks/use-wait-for-idle.tsx | Adds fallback scheduling when requestIdleCallback isn’t available |
| apps/dashboard/src/components/theme-toggle.tsx | Avoids calling View Transitions API when unsupported |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| let cancelled = false; | ||
| setTimeout(() => { | ||
| requestIdleCallback(() => { | ||
| const cb = () => { | ||
| if (cancelled) return; | ||
| setHasWaited(true); | ||
| }, { timeout: max - min }); | ||
| }; | ||
| if (typeof requestIdleCallback === "function") { | ||
| requestIdleCallback(cb, { timeout: max - min }); | ||
| } else { | ||
| setTimeout(cb, max - min); | ||
| } | ||
| }, min); | ||
| return () => { | ||
| cancelled = true; |
| if (typeof requestIdleCallback === "function") { | ||
| requestIdleCallback(cb, { timeout: max - min }); | ||
| } else { | ||
| setTimeout(cb, max - min); | ||
| } |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b5232a1. Configure here.
| if (typeof document.startViewTransition !== "function") { | ||
| setTheme(nextTheme); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Redundant startViewTransition check duplicates existing guard
Low Severity
The new typeof document.startViewTransition !== "function" check on lines 25-28 is redundant dead code. The existing supportsViewTransitions() function (line 8-10) already checks "startViewTransition" in document, and the early return on line 20 ensures execution only reaches line 25 when that property exists. In all real browser implementations, when "startViewTransition" in document is true, the property is always a function, making this second guard unreachable.
Reviewed by Cursor Bugbot for commit b5232a1. Configure here.
Greptile SummaryThis PR adds browser compatibility guards for two APIs that are not universally supported:
Confidence Score: 4/5Safe to merge — both changes are defensive guards that improve cross-browser compatibility without altering the happy path. The use-wait-for-idle fix is clean and correct. The theme-toggle fix adds a guard that is redundant with the already-present supportsViewTransitions() check, making the new block unreachable dead code rather than a functional improvement. The redundant guard in theme-toggle.tsx (lines 25-28) is worth cleaning up, but poses no runtime risk. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[handleToggle called] --> B{prefersReducedMotion or !supportsViewTransitions?}
B -- Yes --> C[setTheme directly]
B -- No --> D{typeof startViewTransition === function?}
D -- No --> C
D -- Yes --> E[startViewTransition callback]
E --> F[setTheme inside transition]
E --> G[Animate old/new views]
H[useWaitForIdle hook] --> I[outer setTimeout min ms]
I --> J{typeof requestIdleCallback === function?}
J -- Yes --> K[requestIdleCallback cb timeout: max-min ms]
J -- No --> L[setTimeout cb max-min ms]
K --> M[setHasWaited true]
L --> M
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/dashboard/src/components/theme-toggle.tsx:25-28
**Redundant guard — already covered by `supportsViewTransitions()`**
The new `typeof document.startViewTransition !== "function"` check is unreachable in practice. The existing `supportsViewTransitions()` call on line 20 uses `"startViewTransition" in document`, which already returns `false` whenever the API is absent, causing an early return before this block is ever evaluated. The only case where this new guard would fire is if `startViewTransition` is present on `document` but isn't a function — a scenario that doesn't exist in any real browser. Consider either updating `supportsViewTransitions()` to use `typeof document.startViewTransition === "function"` and removing this guard, or simply removing this redundant block.
Reviews (1): Last reviewed commit: "Fix browser compatibility: guard request..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (typeof document.startViewTransition !== "function") { | ||
| setTheme(nextTheme); | ||
| return; | ||
| } |
There was a problem hiding this comment.
supportsViewTransitions()
The new typeof document.startViewTransition !== "function" check is unreachable in practice. The existing supportsViewTransitions() call on line 20 uses "startViewTransition" in document, which already returns false whenever the API is absent, causing an early return before this block is ever evaluated. The only case where this new guard would fire is if startViewTransition is present on document but isn't a function — a scenario that doesn't exist in any real browser. Consider either updating supportsViewTransitions() to use typeof document.startViewTransition === "function" and removing this guard, or simply removing this redundant block.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/dashboard/src/components/theme-toggle.tsx
Line: 25-28
Comment:
**Redundant guard — already covered by `supportsViewTransitions()`**
The new `typeof document.startViewTransition !== "function"` check is unreachable in practice. The existing `supportsViewTransitions()` call on line 20 uses `"startViewTransition" in document`, which already returns `false` whenever the API is absent, causing an early return before this block is ever evaluated. The only case where this new guard would fire is if `startViewTransition` is present on `document` but isn't a function — a scenario that doesn't exist in any real browser. Consider either updating `supportsViewTransitions()` to use `typeof document.startViewTransition === "function"` and removing this guard, or simply removing this redundant block.
How can I resolve this? If you propose a fix, please make it concise.
Add feature checks for two browser APIs that aren't universally supported:
requestIdleCallback: Falls back tosetTimeouton browsers that don't support it (e.g. Safari)document.startViewTransition: Applies the theme change directly when the View Transitions API is unavailableLink to Devin session: https://app.devin.ai/sessions/c77b612db7834e9aa71c6b5a0fde316b
Requested by: @N2D4
Note
Low Risk
Low risk: adds simple runtime feature-detection with safe fallbacks for unsupported browser APIs, affecting only theme toggling animation and an idle-wait hook.
Overview
Improves dashboard browser compatibility by adding runtime guards and fallbacks for unsupported web APIs.
ThemeTogglenow skips the View Transitions path whendocument.startViewTransitionisn’t a function, applying the theme change directly instead.useWaitForIdlenow falls back tosetTimeoutwhenrequestIdleCallbackis unavailable, while preserving the existing min/max delay behavior.Reviewed by Cursor Bugbot for commit b5232a1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes