fix: attach window.matchMedia listener for OS-level dark mode changes#438
Conversation
|
@ZEMAD0N26 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Warning Review limit reached
More reviews will be available in 57 minutes and 4 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. 📝 WalkthroughWalkthroughThe PR introduces dark mode theme support and third-party widget style isolation. CSS rules establish an isolated ChangesTheme and Widget Isolation Infrastructure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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 `@app/layout.tsx`:
- Around line 101-106: The media query handling currently registers only
mql.addEventListener('change', updateTheme) and never applies the initial theme,
which breaks in Safari and causes a flash; update the logic around mql and
updateTheme so you: (1) immediately call updateTheme with the current matches
(e.g., invoke updateTheme({matches: mql.matches}) or call updateTheme directly
using mql.matches) to sync theme right away, and (2) add a backwards-compatible
listener by falling back to mql.addListener(updateTheme) when addEventListener
is not available (keep using mql.addEventListener when present), referencing the
existing mql and updateTheme identifiers and ensuring
document.documentElement.classList.toggle('dark', ...) is used by updateTheme.
🪄 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
Run ID: a9207ed8-046b-4c8c-8da1-0b4e3b826e7b
📒 Files selected for processing (2)
app/globals.cssapp/layout.tsx
| const mql = window.matchMedia('(prefers-color-scheme: dark)'); | ||
| function updateTheme(e) { | ||
| document.documentElement.classList.toggle('dark', e.matches); | ||
| } | ||
| mql.addEventListener('change', updateTheme); | ||
| } catch (err) {} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the browser support status for MediaQueryList.addEventListener('change') vs MediaQueryList.addListener, especially on Safari/iOS versions still commonly in use?
💡 Result:
The MediaQueryList.addEventListener('change',...) method is the modern, standards-compliant approach for monitoring media query changes [1][2], while MediaQueryList.addListener(...) is deprecated [1][3]. Browser Support Summary: 1. MediaQueryList.addEventListener('change'): Supported in Safari since version 14 (on both macOS and iOS) [2][4]. If your project needs to support Safari 13 or earlier, this method will be undefined, and attempting to call it will throw an error [4]. 2. MediaQueryList.addListener: A legacy alias that was used before MediaQueryList inherited from EventTarget [1][5]. It is supported in older Safari versions where addEventListener is missing [2][5]. Recommendation for Compatibility: To support both modern browsers and older versions of Safari (specifically Safari 13 and below), use a feature detection pattern. Check for the existence of addEventListener before falling back to the deprecated addListener [4][6]: const mql = window.matchMedia('(prefers-color-scheme: dark)'); if (mql.addEventListener) { // Modern browsers (Safari 14+, Chrome, Firefox, etc.) mql.addEventListener('change', callback); } else if (mql.addListener) { // Legacy support (Safari 13 and older) mql.addListener(callback); } This approach ensures your code utilizes the modern EventTarget interface where available while maintaining functionality on older, legacy browsers that are still encountered in the wild [4][6].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/MediaQueryList/addListener
- 2: https://docs.w3cub.com/dom/mediaquerylist.html
- 3: https://github.com/mdn/content/blob/main/files/en-us/web/api/mediaquerylist/index.md
- 4: addEventListener on window.matchMedia("(prefers-color-scheme: dark)") undefined on Safari 13 and earlier vercel/next.js#29193
- 5: https://udn.realityripple.com/docs/Web/API/MediaQueryList/addListener
- 6: https://stackoverflow.com/questions/56466261/matchmedia-addlistener-marked-as-deprecated-addeventlistener-equivalent
Sync theme immediately and add MediaQueryList fallback for Safari
- The block only uses
mql.addEventListener('change', ...); Safari 13 and earlier may not supportaddEventListeneronMediaQueryList, and the surroundingtry/catchwill silently prevent listener hookup. - The script never applies the initial
mql.matches, so it can mismatch/flash until the rest of the theme logic runs.
Suggested patch
(function() {
try {
const mql = window.matchMedia('(prefers-color-scheme: dark)');
- function updateTheme(e) {
- document.documentElement.classList.toggle('dark', e.matches);
+ function updateTheme(target) {
+ document.documentElement.classList.toggle('dark', target.matches);
}
- mql.addEventListener('change', updateTheme);
+ updateTheme(mql);
+ if (typeof mql.addEventListener === 'function') {
+ mql.addEventListener('change', updateTheme);
+ } else if (typeof mql.addListener === 'function') {
+ mql.addListener(updateTheme);
+ }
} catch (err) {}
})();🤖 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 `@app/layout.tsx` around lines 101 - 106, The media query handling currently
registers only mql.addEventListener('change', updateTheme) and never applies the
initial theme, which breaks in Safari and causes a flash; update the logic
around mql and updateTheme so you: (1) immediately call updateTheme with the
current matches (e.g., invoke updateTheme({matches: mql.matches}) or call
updateTheme directly using mql.matches) to sync theme right away, and (2) add a
backwards-compatible listener by falling back to mql.addListener(updateTheme)
when addEventListener is not available (keep using mql.addEventListener when
present), referencing the existing mql and updateTheme identifiers and ensuring
document.documentElement.classList.toggle('dark', ...) is used by updateTheme.
2 participants
closes #348
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes