-
Notifications
You must be signed in to change notification settings - Fork 105
chore: replace my-site links with migrated pages #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying welcome-rds with
|
| Latest commit: |
52eb1fa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4ffbe330.welcome-rds.pages.dev |
| Branch Preview URL: | https://replace-my-sit.welcome-rds.pages.dev |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update URL references across the codebase, migrating navigation from the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
4580225 to
9b2c1ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
js/login.js (1)
39-56: Use theMAIN_SITEconstant for the base URL instead of hard-coding it (Line 47-49).The current implementation hard-codes
https://www.realdevsquad.com/new-signup, butMAIN_SITEis already defined inconstants.js. Import and use it to avoid drift:import { MAIN_SITE } from './constants.js'; // Then: return window.location.replace(`${MAIN_SITE}/new-signup`);This aligns with the existing pattern used in other files and centralizes URL management.
faq.html (1)
56-61: Addjs/constants.jsbeforejs/navbar.jsin faq.html; verify greeting link behavior.The page currently loads
js/navbar.js(line 261) but omitsjs/constants.js, even though navbar.js referencesAPI_BASE_URL(line 21),MAIN_SITE(lines 35, 40, 50, 55), andSTATUS_BASE_URL(line 45). This will cause ReferenceErrors at runtime, breaking the dropdown and profile/status/tasks buttons. All other pages (index.html, discord.html, code-of-conduct.html) load both scripts in the correct order.Additionally, the greeting element at line 57 is an
<a href="https://www.realdevsquad.com/">that triggers a dropdown toggle on click (navbar.js lines 58–62) but lackspreventDefault(), risking unintended navigation.Proposed fixes
+ <script type="text/javascript" src="./js/constants.js"></script> <script type="text/javascript" src="./js/navbar.js"></script>For the greeting link, either use
javascript:void(0)to prevent navigation, or replace the<a>with a styled<button>:- <a href="https://www.realdevsquad.com/"> + <a href="javascript:void(0)"> <div class="user-greet-msg">Hello, User!</div> <img class="user-profile-pic" /> </a>
🤖 Fix all issues with AI agents
In @js/constants.js:
- Around line 1-3: API_BASE_URL is hardcoded to http://localhost:3000 which
causes mixed-content failures in production; change it to be environment-aware
by computing its value at runtime (similar to the detection in navbar.js) —
e.g., if window.location.hostname indicates production use the secure production
API base (https://<production-api-host>) and otherwise use localhost (or a
configurable env var); update the constant definition for API_BASE_URL so code
that calls `${API_BASE_URL}/auth/signout` uses the correct scheme/host in both
dev and prod.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
faq.htmljs/constants.jsjs/login.jsjs/navbar.js
🧰 Additional context used
🧬 Code graph analysis (1)
js/navbar.js (1)
js/constants.js (2)
MAIN_SITE(2-2)STATUS_BASE_URL(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
Date: 11-01-26
Developer Name: @MayankBansal12
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
demo
Screencast.from.2026-01-11.03-53-15.mp4