feat: System Theme Preference Support #1945#1953
feat: System Theme Preference Support #1945#1953desireddymohithreddy0925 wants to merge 3 commits into
Conversation
|
Someone is attempting to deploy a commit to the s3dfx-cyber's projects Team on Vercel. A member of the Team first needs to authorize it. |
💬 Faster Reviews & AssignmentsHi @desireddymohithreddy0925, for faster coordination and smoother communication, consider joining our Discord community: Useful Channels
|
✅ DCO Sign-off VerifiedHi @desireddymohithreddy0925 👋 All commits in this PR contain valid Thank you for following the DCO requirements 🚀 |
github-actions
Bot
added
dco-missing
size/s
gssoc26
👋 Thanks for opening a PR, @desireddymohithreddy0925!Your PR has entered the 🚦 PR Review Pipeline.
🔄 Review Flow
A pipeline status comment may appear automatically as your PR progresses. ✅ Contributor Checklist
|
|
|
Warning Review limit reached
More reviews will be available in 49 minutes and 23 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds system-preference-aware theme initialization in both ChangesThree-mode theme initialization with system preference support
CI workflow updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
github-actions
Bot
added
missing-program-classification
and removed
gssoc26
✅ Program Classification VerifiedDetected contribution program:
Program-aware automation and routing are now enabled for this PR. |
github-actions
Bot
added
gssoc26
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
index.html (1)
83-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
globalThisfor media-query access.Line 83 uses
window.matchMedia; switching toglobalThis.matchMediaaligns with the project’s static-analysis expectation and avoids recurring warnings.Suggested change
- } else if (window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches) { + } else if (globalThis.matchMedia?.('(prefers-color-scheme: dark)').matches) {🤖 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 `@index.html` at line 83, Replace the `window.matchMedia` reference with `globalThis.matchMedia` on line 83 where the color scheme preference is being checked. This change aligns with the project's static analysis expectations and eliminates linter warnings. Locate the line containing the condition `window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches` and update both occurrences of `window` to `globalThis` to ensure consistency with the project standards.Source: Linters/SAST tools
src/js/app.js (1)
117-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
windowwithglobalThisin theme media-query handling.Using
globalThis.matchMediahere keeps this file consistent with the repo convention and resolves the Sonar warnings at Lines 117, 124, and 125.Suggested change
- } else if (window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches) { + } else if (globalThis.matchMedia?.('(prefers-color-scheme: dark)').matches) { document.documentElement.classList.add('dark'); } else { document.documentElement.classList.remove('dark'); } updateThemeIcon(); - if (window.matchMedia) { - window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', e => { + if (globalThis.matchMedia) { + globalThis.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', e => { if (!localStorage.getItem('theme')) { document.documentElement.classList.toggle('dark', e.matches); updateThemeIcon(); } });Based on learnings, this repository intentionally prefers
globalThisoverwindowfor environment-agnostic JavaScript and SonarCloud compliance.Also applies to: 124-125
🤖 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 `@src/js/app.js` at line 117, Replace all instances of `window.matchMedia` with `globalThis.matchMedia` in the theme media-query handling section. This change should be applied to the three occurrences in the conditional checks for color scheme preference detection (the check for `(prefers-color-scheme: dark)` and related media queries at lines 117, 124, and 125). Update each `window.matchMedia` call to use `globalThis.matchMedia` instead to maintain consistency with repository conventions.Sources: Learnings, Linters/SAST tools
🤖 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.
Nitpick comments:
In `@index.html`:
- Line 83: Replace the `window.matchMedia` reference with
`globalThis.matchMedia` on line 83 where the color scheme preference is being
checked. This change aligns with the project's static analysis expectations and
eliminates linter warnings. Locate the line containing the condition
`window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches`
and update both occurrences of `window` to `globalThis` to ensure consistency
with the project standards.
In `@src/js/app.js`:
- Line 117: Replace all instances of `window.matchMedia` with
`globalThis.matchMedia` in the theme media-query handling section. This change
should be applied to the three occurrences in the conditional checks for color
scheme preference detection (the check for `(prefers-color-scheme: dark)` and
related media queries at lines 117, 124, and 125). Update each
`window.matchMedia` call to use `globalThis.matchMedia` instead to maintain
consistency with repository conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c987dad3-556b-4da2-ac07-47464f194655
📒 Files selected for processing (2)
index.htmlsrc/js/app.js
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-06-15T18:15:28.688Z
Learnt from: arghya29
Repo: S3DFX-CYBER/GSoC-Org-Finder- PR: 1882
File: src/js/footer.js:33-33
Timestamp: 2026-06-15T18:15:28.688Z
Learning: In this repo’s JavaScript (e.g., footer.js), `globalThis` is intentionally preferred over `window` to keep code environment-agnostic and to satisfy SonarCloud static analysis. The project targets modern browsers (ES2021) with no transpilation, so `globalThis` is fully supported—do not flag `globalThis` usage as a browser compatibility concern or suggest replacing it with `window` during review.
Applied to files:
src/js/app.js
🪛 GitHub Check: SonarCloud Code Analysis
src/js/app.js
[warning] 124-124: Prefer globalThis over window.
[warning] 125-125: Prefer globalThis over window.
[warning] 117-117: Prefer using an optional chain expression instead, as it's more concise and easier to read.
[warning] 117-117: Prefer globalThis over window.
[warning] 117-117: Prefer globalThis over window.
index.html
[warning] 83-83: Prefer globalThis over window.
[warning] 83-83: Prefer globalThis over window.
github-actions
Bot
added
type:devops
gssoc26
🤖 TENET Agent Review📋 SummaryThis pull request introduces support for automatically detecting and respecting the user's operating system theme preference (dark/light mode). It prioritizes user-saved preferences in 🔐 Security FindingsNo security issues found. 🧹 Code Quality
✅ What's Done Well
📝 Overall VerdictAPPROVE - The feature is well-implemented, addresses the problem, and introduces no security concerns. Review powered by TENET Agent 🛡️ | Triggered automatically on PR #1953 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Confidence score: 3/5
- In
index.html, the theme bootstrap putslocalStorageaccess andmatchMediafallback in the sametry, so if storage access throws, OS-theme detection is skipped and users can get the wrong initial theme flash/state—split the error handling somatchMediastill runs when storage is unavailable before merging. - Theme resolution is currently duplicated across
index.htmlandsrc/js/app.js, with an additional inline block inindex.htmlthat ignores OS preference, which increases drift risk and can cause inconsistent theme selection between first paint and app startup—consolidate to a single shared resolution path (or make all paths honor the same precedence) before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="index.html">
<violation number="1" location="index.html:78">
P1: OS-theme fallback is skipped when localStorage access throws because the matchMedia check is inside the same try block.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
✅ Merge Conflicts ResolvedThanks @desireddymohithreddy0925, this PR is mergeable again. |
1 participant
✨ Feature Description
Added support to automatically detect and respect the user's operating system theme preference (dark/light mode).
🤔 Problem It Solves
Currently, users must manually click the theme toggle button to switch to dark mode, and this preference is saved in localStorage. Users whose operating systems automatically switch themes based on the time of day (or manual OS toggles) have a disconnected experience where the website does not adapt automatically, forcing them to manually toggle it.
💡 Proposed Solution
window.matchMedia('(prefers-color-scheme: dark)')to check the system's preferred theme on initial load inindex.htmlandapp.js, using it as the default iflocalStoragehas no saved preference.matchMediato dynamically switch the theme if the system preference changes during the session.✅ Acceptance Criteria Verified
🌱 Contributor Checklist
program: gssoc