[UI Enhancement] Faq Enhancement #325
Conversation
|
@ishanraj953 is attempting to deploy a commit to the santanu-atta03's projects Team on Vercel. A member of the Team first needs to authorize it. |
Thanks for creating a PR for your Issue!
|
There was a problem hiding this comment.
Pull request overview
This PR modernizes the FAQ hover styling (glassmorphism/elevation) and refactors multiple pages to use a shared Navbar component instead of duplicating navbar markup across pages.
Changes:
- Added a reusable
Frontend/src/components/Navbar.jsxand replaced inline navbars across several pages. - Updated the Landing page FAQ section styling to include a glassmorphism hover effect.
- Updated React/lockfiles (React 19 → 18 in
Frontend/, plus root lockfile changes).
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
Frontend/src/components/Navbar.jsx |
New shared navbar component used across multiple pages. |
Frontend/src/pages/Landing.jsx |
Uses shared navbar; updates FAQ section hover/visual styling. |
Frontend/src/pages/PricingPage.jsx |
Replaces inline navbar with shared navbar. |
Frontend/src/pages/AboutUs.jsx |
Replaces inline navbar with shared navbar (skin tone + theme toggle). |
Frontend/src/pages/ContactUs.jsx |
Replaces inline navbar with shared navbar (skin tone + theme toggle). |
Frontend/src/pages/Career.jsx |
Replaces inline navbar with shared navbar (auth buttons disabled). |
Frontend/src/pages/FAQ.jsx |
Replaces inline navbar with shared “simple” navbar variant. |
Frontend/src/pages/Terms.jsx |
Replaces inline navbar with shared navbar. |
Frontend/src/pages/Privacy.jsx |
Replaces inline navbar with shared navbar. |
Frontend/src/pages/CookiePolicy.jsx |
Replaces inline navbar with shared navbar. |
Frontend/package.json |
Changes React/ReactDOM versions. |
Frontend/package-lock.json |
Lockfile updates reflecting dependency/version changes. |
package-lock.json |
Root lockfile updated (adds React-related peer entries). |
Files not reviewed (1)
- Frontend/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
Frontend/package-lock.json:1716
- After downgrading React to 18, the lockfile still pins
@types/react/@types/react-domto v19.x (and@types/reactis markeddevOptional). This mismatch can cause type-checking/editor issues. Align the@types/*versions with the chosen React major version (or remove them if they're not needed).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div className="relative"> | ||
| <div className="w-10 h-10 sm:w-12 sm:h-12 rounded-lg sm:rounded-xl flex items-center justify-center "> | ||
| <img src={logo} alt="logo" /> | ||
| </div> |
There was a problem hiding this comment.
logo is referenced in the variant === "simple" navbar markup but isn't imported/defined in this file, which will throw a ReferenceError at runtime (e.g., when rendering the FAQ page). Import the logo asset (or accept it via props) and use that variable here, or remove the <img> if not needed.
| {openFaq === 1 && ( | ||
| <p className="mt-6 text-xl text-white/90 leading-relaxed"> | ||
| Yes! We have role-specific question banks for 50+ tech positions | ||
| including Frontend, Backend, Full Stack, DevOps, Data Science, ML | ||
| Engineer, Product Manager, and more. | ||
| </p> |
There was a problem hiding this comment.
The FAQ content text was shortened (e.g., removing sentences about curated questions / scenarios) even though the PR description says the layout/structure/functionality is unchanged and the linked issue only requests a hover-state enhancement. Please restore the original FAQ copy (or move copy edits into a separate PR and update the description accordingly).
| import { useSelector } from "react-redux"; | ||
| import Navbar from "../components/Navbar"; | ||
|
|
||
| export default function PrivacyPolicy() { |
There was a problem hiding this comment.
CookiePolicy.jsx exports a component named PrivacyPolicy, which is confusing in stack traces/React DevTools and makes the file/component naming inconsistent. Rename the function to CookiePolicy (or similar) to match the page it renders.
| export default function PrivacyPolicy() { | |
| export default function CookiePolicy() { |
| </div> | ||
| )} | ||
| </nav> | ||
| <Navbar tone="light" activeKey="about" /> |
There was a problem hiding this comment.
activeKey is set to "about" on the Terms page, which will highlight the About nav item as active. If this page isn't meant to mark About as active, pass activeKey={undefined} (or add a dedicated key/link if you want Terms highlighted).
| <Navbar tone="light" activeKey="about" /> | |
| <Navbar tone="light" activeKey={undefined} /> |
| </div> | ||
| )} | ||
| </nav> | ||
| <Navbar tone="light" activeKey="about" /> |
There was a problem hiding this comment.
activeKey is set to "about" on the Privacy page, which will highlight the About nav item as active. If this page isn't meant to mark About as active, pass activeKey={undefined} (or add a dedicated key/link if you want Privacy highlighted).
| <Navbar tone="light" activeKey="about" /> | |
| <Navbar tone="light" activeKey={undefined} /> |
| </Link> | ||
| </div> | ||
| </nav> | ||
| <Navbar variant="simple" /> |
There was a problem hiding this comment.
Navbar variant="simple" currently renders only branding (no Home/back link). Previously this page had a Home link in the navbar, so this is a user-facing navigation regression. Consider adding a Home link/action to the simple variant or switching this page back to a navbar variant that includes navigation.
| <div className="border-b-4 border-white pb-6 rounded-2xl transition-all duration-300 hover:bg-white/10 hover:backdrop-blur-md hover:border-white/70 hover:shadow-[0_8px_32px_rgba(255,255,255,0.15)] px-4"> | ||
| <button | ||
| onClick={() => toggleFaq(0)} | ||
| className="w-full flex items-center justify-between text-left group" |
There was a problem hiding this comment.
The FAQ accordion button still doesn't set a pointer cursor, so the row may not feel clickable (an explicit acceptance criterion in issue #248). Consider adding cursor-pointer (and ideally a visible focus style) to the clickable button/row.
| className="w-full flex items-center justify-between text-left group" | |
| className="w-full flex items-center justify-between text-left group cursor-pointer focus:outline-none focus-visible:ring-2 focus-visible:ring-white/80 focus-visible:ring-offset-2 focus-visible:ring-offset-transparent" |
| </div> | ||
| )} | ||
| </nav> | ||
| <Navbar tone="light" activeKey="about" /> |
There was a problem hiding this comment.
activeKey is set to "about" on the Cookie Policy page, which will highlight the About nav item as active. If this page isn't meant to mark About as active, pass activeKey={undefined} (or add a dedicated key/link if you want Cookie Policy highlighted).
| <Navbar tone="light" activeKey="about" /> | |
| <Navbar tone="light" activeKey={undefined} /> |
| "react": "^18.3.1", | ||
| "react-dom": "^18.3.1", |
There was a problem hiding this comment.
This PR changes React/ReactDOM from ^19.1.1 to ^18.3.1, which is a significant dependency change and not mentioned in the PR description/linked issue. If this downgrade is required, please explain why in the PR description and ensure related deps are aligned (e.g., @types/react/@types/react-dom versions). Otherwise, revert this to keep the PR scoped to the FAQ hover UI change.
|
HI, Please check PR under OSCG'26. If you find any other possible improvement then please let me know. |
Pull Request — Intervyo
📌 Related Issue
Fixes #248
Description of Changes
Type of Change
Testing
Describe the tests you ran to verify your changes.
📸 Screenshots / Video