Fix: Blue color glitch on "Learn Logic Design" page (dark mode rendering issue)#778
Fix: Blue color glitch on "Learn Logic Design" page (dark mode rendering issue)#778Ram-sah19 wants to merge 10 commits intoCircuitVerse:masterfrom
Conversation
… GA config, and GitHub Actions version
… GA config, and GitHub Actions version
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 43 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughUpdated CI workflow to use actions/checkout@v4 and removed 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 2
🧹 Nitpick comments (6)
assets/js/module.js (2)
2350-2356: Inconsistent fallback handling between X and Y coordinates.Line 2350 uses
e.pageXdirectly without a fallback, while the new Y-coordinate logic (lines 2352-2355) includes fallbacks for browsers wheree.pageYmight not be a number. For consistency and robustness, consider applying the same fallback pattern to X:♻️ Suggested fix for consistent X/Y handling
- mx = e.pageX - offsetX; var scrollEl = document.getElementById("scrollcount"); var windowScrollTop = window.pageYOffset || document.documentElement.scrollTop || document.body.scrollTop || 0; + var windowScrollLeft = window.pageXOffset || document.documentElement.scrollLeft || document.body.scrollLeft || 0; + var pageX = (typeof e.pageX === "number") ? e.pageX : (e.clientX + windowScrollLeft); var pageY = (typeof e.pageY === "number") ? e.pageY : (e.clientY + windowScrollTop); + var containerScrollLeft = scrollEl ? scrollEl.scrollLeft : 0; var containerScrollTop = scrollEl ? scrollEl.scrollTop : 0; + mx = pageX - offsetX + containerScrollLeft; my = pageY - offsetY + containerScrollTop;
2352-2352: Consider using the non-deprecatedwindow.scrollYas the primary option.
window.pageYOffsetis deprecated (though still widely supported). For forward compatibility, consider usingwindow.scrollYas the primary withpageYOffsetas a fallback:♻️ Minor modernization suggestion
- var windowScrollTop = window.pageYOffset || document.documentElement.scrollTop || document.body.scrollTop || 0; + var windowScrollTop = window.scrollY || window.pageYOffset || document.documentElement.scrollTop || document.body.scrollTop || 0;_config.yml (1)
83-83: Thefooter_contentconfiguration with Liquid syntax is unused and should be removed or corrected.The
footer_contentvalue in_config.ymlcontains Liquid template syntax ({% assign current_year = 'now' | date: '%Y' %}{{ current_year }}), which just-the-docs does not process. While this doesn't currently cause a visual issue (the custom_includes/content_footer.htmloverrides the theme's default footer and doesn't referencesite.footer_content), the configuration is deprecated.Either remove the
footer_contentsetting or use the recommended_includes/footer_custom.htmlapproach for dynamic content like the current year.assets/js/global_scripts.js (1)
6-11: Type inconsistency between localStorage (string) and numeric comparisons.
localStorage.getItem()returns a string (or null), but the code compares with numbers using loose equality (==). While this works due to type coercion, it's fragile. Consider using strict equality with explicit string values for clarity:♻️ Suggested improvement
-var isDarkMode = localStorage.getItem(storageItem); +var isDarkMode = localStorage.getItem(storageItem) === "1"; -if (isDarkMode == null) { - isDarkMode = 0; - localStorage.setItem(storageItem, isDarkMode); +if (isDarkMode === null) { + isDarkMode = false; + localStorage.setItem(storageItem, "0"); } // Apply saved theme only after jtd is ready to avoid race condition document.addEventListener('DOMContentLoaded', function () { - if (isDarkMode == 1) { + if (isDarkMode) { jtd.setTheme('circuitversedark'); } });Also applies to: 15-15, 25-25, 30-30
_sass/custom/custom.scss (1)
307-317: Gate animation widget may appear jarring in dark mode.The
.gate-anim-wrapperusesbackground: $whitewhich won't adapt to dark mode. This could create visual inconsistency when users toggle themes. Consider using a theme-aware background variable or adding a dark mode override.♻️ Suggested improvement for dark mode support
.gate-anim-wrapper { - background: $white; + background: $body-background-color; border: 1px solid $border-color; border-radius: 6px; display: inline-block; margin: 1rem 0; max-width: 420px; padding: 0.75rem 1rem; width: 100%; }_includes/gate_animation.html (1)
29-39: Missingdefaultcase in switch statement.If an unsupported gate type is passed,
computeOutputreturnsundefined, which could cause unexpected behavior in the animation. Consider adding a default case for defensive programming.♻️ Suggested improvement
function computeOutput(gate, a, b) { switch(gate) { case "AND": return a && b; case "OR": return a || b; case "NOT": return !a; case "NAND": return !(a && b); case "NOR": return !(a || b); case "XOR": return a !== b; case "XNOR": return a === b; + default: return false; } }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6045076-7060-4a98-84b4-88c51d3e91fd
📒 Files selected for processing (9)
.github/scripts/assign-or-comment.js.github/workflows/deploy.yml_config.yml_includes/gate_animation.html_sass/custom/custom.scssassets/js/global_scripts.jsassets/js/module.jsassets/js/quiz.jsdocs/comb-ssi/logic-gates.md
💤 Files with no reviewable changes (1)
- .github/scripts/assign-or-comment.js
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
_includes/gate_animation.html (1)
29-39: Add an explicit fallback for unsupported gate values.
computeOutputcurrently falls through silently for unknown gates. An explicit default path makes template misuse fail predictably.♻️ Proposed refactor
function computeOutput(gate, a, b) { switch(gate) { case "AND": return a && b; case "OR": return a || b; case "NOT": return !a; case "NAND": return !(a && b); case "NOR": return !(a || b); case "XOR": return a !== b; case "XNOR": return a === b; + default: + throw new Error("Unsupported gate: " + gate); } }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 111d7c76-c808-4ea2-b153-d38739e06038
📒 Files selected for processing (4)
_includes/gate_animation.html_sass/custom/custom.scssassets/js/global_scripts.jsassets/js/module.js
🚧 Files skipped from review as they are similar to previous changes (1)
- assets/js/global_scripts.js
1 participant
🔧 Summary
fix #659
This PR fixes an intermittent blue color rendering issue on the "Learn Logic Design" page.
🐛 Problem
✅ Fix Implemented
🧪 Testing
💡 Notes
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores