Frontend UI/UX improvements and usability enhancements#6
Conversation
Removed a bunch of duplicate data files and empty UI ghost files that were causing errors. Also added a working university search bar and updated the AI chat so it shows a nice demo message instead of crashing when the backend is offline.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR updates the public shell and accessibility chrome, consolidates RGPV content access, adds shared AI helpers and flows, and moves landing-page content and widgets onto shared data and reusable components. ChangesPublic app and content updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.13.0)app/globals.cssError: ENOENT: no such file or directory, open '/.stylelintrc.json' 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 |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/ai/workspace-chat.tsx (1)
79-149: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard
finallyagainst stale aborted requests.If a second
sendMessagestarts while another request is in flight, the first call aborts and still reachesfinally, which unconditionally setsloadingback tofalse. That leaves the replacement request running without a spinner and makes a third overlapping send reachable.Suggested direction
- try { - const abortController = new AbortController(); - abortControllerRef.current = abortController; + const abortController = new AbortController(); + abortControllerRef.current = abortController; + try { ... } finally { - setLoading(false); - inputRef.current?.focus(); + if (abortControllerRef.current === abortController) { + abortControllerRef.current = null; + setLoading(false); + inputRef.current?.focus(); + } }🤖 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 `@components/ai/workspace-chat.tsx` around lines 79 - 149, The sendMessage flow in workspace-chat.tsx unconditionally clears loading in the catch/finally path, so an aborted earlier request can turn off the spinner for a newer in-flight request. Track the active request with the AbortControllerRef.current (or a request id) inside the sendMessage async block, and only call setLoading(false) when the current request is still the active one. Update the abort handling around the existing abortControllerRef.current, fetch, and finally logic so stale aborted requests cannot overwrite state for the latest sendMessage call.
🧹 Nitpick comments (2)
components/ai/answer-viewer.tsx (1)
56-192: 📐 Maintainability & Code Quality | 🔵 TrivialType
markdownComponentsinstead of erasing it withany.Using
as anydisables TypeScript checks for the custom renderer map, potentially allowing prop mismatches to pass undetected. The project already depends onreact-markdown(v10.1.0), which exports aComponentstype that perfectly matches this object's shape.Applying this type ensures type safety for all custom element implementations (e.g.,
pre,code,table) and removes the unnecessary suppression.Fix
+import type { Components } from "react-markdown"; // ... -const markdownComponents = { +const markdownComponents: Components = { pre({ children }: any) { ... }, // ... other components ... -} as any; +};Note: You may still use
anyorunknownfor individual prop arguments if strict inference is too restrictive, but the container object itself should be typed asComponents.🤖 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 `@components/ai/answer-viewer.tsx` around lines 56 - 192, The markdownComponents renderer map is being erased with an unnecessary any cast, which removes TypeScript safety. Type the markdownComponents object as react-markdown’s Components instead of using as any, and keep individual handler props as any or unknown only if needed; this preserves checks for renderers like pre, code, and table while matching the existing custom component shape.Source: Linters/SAST tools
components/ai/chat-mockup.tsx (1)
1-4: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDrop the unnecessary client boundary here.
ChatMockupis static markup right now, so"use client"forces extra client-side JS for no functional gain. If you are not planning to add local motion/hooks here, make this a server component and remove theframer-motionimport.Proposed fix
-"use client"; - -import { motion } from "framer-motion"; import { BrainCircuit, ArrowRight } from "lucide-react";🤖 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 `@components/ai/chat-mockup.tsx` around lines 1 - 4, ChatMockup is currently marked as a client component without needing any client-only behavior, so remove the client boundary and any unused client-only imports. Update the ChatMockup component to be a server component by dropping "use client" and removing the framer-motion import, while keeping the static markup and the lucide-react icons intact.
🤖 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/error.tsx`:
- Line 24: The error message in the error page text contains an unescaped
apostrophe that violates the react/no-unescaped-entities lint rule. Update the
JSX content in the error component so the “We've encountered an unexpected
error. Please try again.” string uses an escaped apostrophe or equivalent
JSX-safe form, keeping the text the same while making the error page lint-clean.
In `@app/layout.tsx`:
- Around line 33-35: The skip link target in the main layout is not focusable,
so keyboard focus remains on the link instead of moving into the main content.
Update the <main> element in app layout to make the skip-link destination
focusable by adding a non-tabbable focus target, using the existing main-content
anchor used by the skip link. This change should be applied where the layout
renders <main id="main-content"> so the skip link can properly move focus past
the navbar.
In `@components/ai/generate-answer-button.tsx`:
- Around line 77-91: Restrict the demo fallback in the answer-generation flow so
it only triggers on real transport/config failures from the AI request, not on
every handled error. Update the logic around the error handling in the
generate-answer button component to stop using NEXT_PUBLIC_API_URL as a
demo-mode signal and avoid matching the local "generate answer" failure text.
Keep the fallback only for fetch/network-style failures from the API call, and
let genuine backend errors continue into the existing setError path.
In `@components/ai/workspace-chat.tsx`:
- Around line 128-147: In workspace-chat’s error handling catch block, stop
treating backend validation failures as demo mode by removing the "Failed to get
response" fallback from the demo-mode check. Keep the demo reply path only for
true connectivity/missing-API cases, and let concrete API errors from the
workspace route surface through setError instead of appending a fake assistant
message. Use the existing catch block in workspace-chat.tsx and the
isDemoMode/demoMessage logic to make the distinction based on actual error type
or status.
In `@components/navbar.tsx`:
- Around line 42-47: The navbar scroll state is only updated after the first
scroll event, so `scrolled` can be incorrect on initial render for restored or
deep-linked pages. In `components/navbar.tsx`, update the setup around
`handleScroll` and the `window.addEventListener("scroll", ...)` call so
`handleScroll()` is invoked once immediately during initialization, then keep
the listener attached as-is.
In `@components/syllabus/module-card.tsx`:
- Around line 50-56: The module header button in ModuleCard can still toggle
open state even when there are no questions to show, which creates an expandable
control with no content. Update the logic around the button and the conditional
rendering in ModuleCard so that modules with questions.length === 0 are either
rendered as disabled/non-expandable or show an explicit empty-state region; make
sure aria-expanded and aria-controls only apply when the content region is
actually rendered.
In `@features/landing/testimonials.tsx`:
- Around line 63-75: The pause/play toggle in testimonials.tsx still appears
interactive even when shouldReduceMotion forces motion off, so update the
control in Testimonials to be hidden or disabled in that mode and add
aria-pressed only when it is active. Use the existing isPaused,
shouldReduceMotion, and setIsPaused logic around the button and motion.div so
reduced-motion users don’t see a control that no longer affects the track.
In `@features/landing/universities.tsx`:
- Around line 42-52: The filtering logic in filteredUniversities should
normalize searchQuery by trimming whitespace before matching, so
leading/trailing spaces do not cause false negatives and an all-whitespace input
is treated as empty. Update the search handling in universities.tsx around
useState and filteredUniversities to compute a trimmed, lowercased query once,
then make the filter return all universities when that normalized query is
empty.
- Around line 105-110: The search field in the universities landing component
relies on placeholder text only, so add an explicit accessible name for the
input. Update the search input in the universities page to use a visible <label>
or an aria-label tied to the existing search control so assistive technologies
can identify it reliably. Use the search input element in the universities
landing component as the place to apply the fix.
In `@lib/content/index.ts`:
- Around line 39-49: The path built in the content lookup is vulnerable to
directory traversal because route params are joined directly into the filesystem
path. Update getPYQs() to resolve the target against a fixed base for
content/rgpv, validate the resolved path stays inside that base before calling
fs.readFile, and reject invalid inputs. If getSyllabus() uses the same pattern,
move the path resolution/validation into a shared helper so both functions
enforce the same guard.
In `@lib/data/landing.ts`:
- Around line 15-18: The landing page feature card in landing data conflicts
with the current product scope because it promises multi-university access while
the rest of the copy in the same data set focuses on RGPV. Update the relevant
entry in the landing content array so the title and description in the card are
scoped to the current RGPV-focused platform, and keep the wording consistent
with the rest of the copy in lib/data/landing.ts to avoid contradictory
messaging.
---
Outside diff comments:
In `@components/ai/workspace-chat.tsx`:
- Around line 79-149: The sendMessage flow in workspace-chat.tsx unconditionally
clears loading in the catch/finally path, so an aborted earlier request can turn
off the spinner for a newer in-flight request. Track the active request with the
AbortControllerRef.current (or a request id) inside the sendMessage async block,
and only call setLoading(false) when the current request is still the active
one. Update the abort handling around the existing abortControllerRef.current,
fetch, and finally logic so stale aborted requests cannot overwrite state for
the latest sendMessage call.
---
Nitpick comments:
In `@components/ai/answer-viewer.tsx`:
- Around line 56-192: The markdownComponents renderer map is being erased with
an unnecessary any cast, which removes TypeScript safety. Type the
markdownComponents object as react-markdown’s Components instead of using as
any, and keep individual handler props as any or unknown only if needed; this
preserves checks for renderers like pre, code, and table while matching the
existing custom component shape.
In `@components/ai/chat-mockup.tsx`:
- Around line 1-4: ChatMockup is currently marked as a client component without
needing any client-only behavior, so remove the client boundary and any unused
client-only imports. Update the ChatMockup component to be a server component by
dropping "use client" and removing the framer-motion import, while keeping the
static markup and the lucide-react icons intact.
🪄 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: b6b7202e-1326-4278-956e-82865e97a2d2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (45)
app/(public)/page.tsxapp/error.tsxapp/globals.cssapp/layout.tsxapp/rgpv/[branch]/[semester]/[subject]/page.tsxapp/rgpv/[branch]/[semester]/[subject]/pyqs/[paper]/page.tsxapp/rgpv/[branch]/[semester]/[subject]/syllabus/page.tsxapp/rgpv/[branch]/page.tsxcomponents/ai/answer-viewer.tsxcomponents/ai/chat-mockup.tsxcomponents/ai/generate-answer-button.tsxcomponents/ai/workspace-chat.tsxcomponents/ai/workspace-prompt-button.tsxcomponents/mode-toggle.tsxcomponents/navbar.tsxcomponents/syllabus/module-card.tsxcomponents/ui/button.tsxcomponents/ui/card.tsxcomponents/ui/dialog.tsxcomponents/ui/table.tsxcomponents/university/university-stats.tsxconstants/index.tsfeatures/landing/ai-demo.tsxfeatures/landing/faq.tsxfeatures/landing/features.tsxfeatures/landing/hero.tsxfeatures/landing/stats.tsxfeatures/landing/testimonials.tsxfeatures/landing/universities.tsxhooks/use-debounce.tslib/academic.tslib/api-endpoints.tslib/constants.tslib/content/index.tslib/content/pyqs.tslib/content/rgpv.tslib/data/index.tslib/data/landing.tslib/data/pyqs.tslib/data/semesters.tslib/data/syllabus.tslib/export-utils.tslib/utils.tspackage.jsontypes/ai.ts
💤 Files with no reviewable changes (8)
- lib/data/semesters.ts
- lib/academic.ts
- lib/data/syllabus.ts
- lib/data/pyqs.ts
- lib/content/rgpv.ts
- components/university/university-stats.tsx
- components/ai/workspace-prompt-button.tsx
- lib/content/pyqs.ts
| const handleScroll = () => { | ||
| setScrolled(window.scrollY > 20); | ||
| }; | ||
|
|
||
| window.addEventListener("scroll", handleScroll, { passive: true }); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Initialize scrolled from the current scroll position.
The listener only updates after the first scroll event, so a restored or deep-linked page can render the wrong header state initially. Call handleScroll() once during setup.
Suggested change
const handleScroll = () => {
setScrolled(window.scrollY > 20);
};
+ handleScroll();
window.addEventListener("scroll", handleScroll, { passive: true });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleScroll = () => { | |
| setScrolled(window.scrollY > 20); | |
| }; | |
| window.addEventListener("scroll", handleScroll, { passive: true }); | |
| const handleScroll = () => { | |
| setScrolled(window.scrollY > 20); | |
| }; | |
| handleScroll(); | |
| window.addEventListener("scroll", handleScroll, { passive: true }); |
🤖 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 `@components/navbar.tsx` around lines 42 - 47, The navbar scroll state is only
updated after the first scroll event, so `scrolled` can be incorrect on initial
render for restored or deep-linked pages. In `components/navbar.tsx`, update the
setup around `handleScroll` and the `window.addEventListener("scroll", ...)`
call so `handleScroll()` is invoked once immediately during initialization, then
keep the listener attached as-is.
| icon: Layers3, | ||
| title: "Multi-University Support", | ||
| description: | ||
| "Access curriculum structures, subjects, resources, and academic content across multiple universities from a single platform.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align this feature copy with the current product scope.
This card says users can access academic content across multiple universities today, but lib/data/landing.ts Lines 176-179 says the platform currently focuses on RGPV. Rendering both on the same landing page creates a contradictory promise.
✏️ Suggested copy update
{
icon: Layers3,
- title: "Multi-University Support",
+ title: "Built for Multi-University Expansion",
description:
- "Access curriculum structures, subjects, resources, and academic content across multiple universities from a single platform.",
+ "Hyper Learning currently focuses on RGPV content and is being structured to support additional universities over time.",
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| icon: Layers3, | |
| title: "Multi-University Support", | |
| description: | |
| "Access curriculum structures, subjects, resources, and academic content across multiple universities from a single platform.", | |
| icon: Layers3, | |
| title: "Built for Multi-University Expansion", | |
| description: | |
| "Hyper Learning currently focuses on RGPV content and is being structured to support additional universities over time.", |
🤖 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 `@lib/data/landing.ts` around lines 15 - 18, The landing page feature card in
landing data conflicts with the current product scope because it promises
multi-university access while the rest of the copy in the same data set focuses
on RGPV. Update the relevant entry in the landing content array so the title and
description in the card are scoped to the current RGPV-focused platform, and
keep the wording consistent with the rest of the copy in lib/data/landing.ts to
avoid contradictory messaging.
imuniqueshiv
left a comment
There was a problem hiding this comment.
Looks good. This commit contains formatting improvements only and does not change application behavior.
imuniqueshiv
left a comment
There was a problem hiding this comment.
Thanks @nitinmohan18 for the significant UI/UX improvements. I reviewed this PR and appreciate the work you've put into improving the frontend.
Before this PR can be merged, please address the following items.
Required Changes
-
Fix demo mode detection (
generate-answer-button.tsx)- Remove the dependency on
NEXT_PUBLIC_API_URLwhen determining demo mode. - The project now uses the internal
/api/ai/answerendpoint, so this check can incorrectly return the demo response instead of surfacing actual backend errors. - Restrict demo mode only to genuine transport or configuration failures.
- Remove the dependency on
-
Fix workspace demo fallback (
workspace-chat.tsx)- Please don't convert API validation or request errors into demo responses.
- Validation errors (such as missing topic, action, or subject) should be shown to the user instead of displaying a simulated AI response.
-
Accessibility
- Add an accessible label (
aria-labelor<label>) for the university search input. - Add
tabIndex={-1}to the<main>element so the skip link works correctly for keyboard users.
- Add an accessible label (
-
Search normalization
-
Update the search logic to:
const query = searchQuery.trim().toLowerCase();
-
This prevents whitespace-only searches from producing incorrect results.
-
-
Security
- Please address the filesystem path validation issue in
lib/content/index.tsto ensure user-controlled route parameters cannot resolve outside the intended content directory.
- Please address the filesystem path validation issue in
-
chat-mockup.tsx- Please verify that removing the
"use client"directive is intentional. - If this component uses client-side hooks, event handlers, or client-only libraries, the directive should remain.
- Please verify that removing the
Update your branch
Your branch is currently behind the latest main branch.
After completing the requested changes, please update your branch with the latest main before pushing:
git checkout improve-ui-ux
git fetch origin
git merge origin/mainIf Git reports any merge conflicts:
-
Resolve the conflicts.
-
Stage the resolved files:
git add . -
Complete the merge:
git commit
Then run the project checks:
npx prettier --write .
npm run format:check
npm run lint
npm run typecheck
npm run buildFinally, push the updated branch:
git push origin improve-ui-uxGitHub will automatically update this Pull Request and rerun all CI checks.
Thanks again for your contribution.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/content/index.ts (1)
10-21: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winThe traversal guard is still bypassable here.
path.join()+filePath.startsWith(baseDir)is not a safe confinement check. A sibling escape like../rgpv-evil/...can normalize to a path outsidecontent/rgpvwhile still matching the same string prefix, so both readers can reach unintended server-side files. Please switch both functions to a shared helper that usespath.resolve()and validates withpath.relative()beforefs.readFile().Proposed fix
+const CONTENT_ROOT = path.join(process.cwd(), "content", "rgpv"); + +function resolveContentFile(...segments: string[]) { + const filePath = path.resolve(CONTENT_ROOT, ...segments); + const relativePath = path.relative(CONTENT_ROOT, filePath); + + if (relativePath.startsWith("..") || path.isAbsolute(relativePath)) { + throw new Error("Invalid path parameter"); + } + + return filePath; +} + export async function getSyllabus( branch: string, semester: string, subjectCode: string ) { try { - const baseDir = path.join(process.cwd(), "content", "rgpv"); - const filePath = path.join( - baseDir, + const filePath = resolveContentFile( branch.toLowerCase(), semester.toLowerCase(), subjectCode.toLowerCase(), "syllabus.json" ); - - if (!filePath.startsWith(baseDir)) { - throw new Error("Invalid path parameter"); - } const file = await fs.readFile(filePath, "utf8"); @@ export async function getPYQs( branch: string, semester: string, subjectCode: string ) { try { - const baseDir = path.join(process.cwd(), "content", "rgpv"); - const filePath = path.join( - baseDir, + const filePath = resolveContentFile( branch.toLowerCase(), semester.toLowerCase(), subjectCode.toLowerCase(), "pyqs.json" ); - - if (!filePath.startsWith(baseDir)) { - throw new Error("Invalid path parameter"); - } const file = await fs.readFile(filePath, "utf8");Also applies to: 42-53
🤖 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 `@lib/content/index.ts` around lines 10 - 21, The path confinement check in the content readers is still bypassable because `path.join()` plus `startsWith()` is not a safe guard. Update the path-building logic in the relevant function(s) under `lib/content/index.ts` to use a shared helper that resolves the target with `path.resolve()` and verifies it stays under the `baseDir` with `path.relative()` before any `fs.readFile()` call. Keep the fix centralized so both readers use the same safe helper and reject any escaped branch/semester/subjectCode traversal inputs.Source: 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.
Duplicate comments:
In `@lib/content/index.ts`:
- Around line 10-21: The path confinement check in the content readers is still
bypassable because `path.join()` plus `startsWith()` is not a safe guard. Update
the path-building logic in the relevant function(s) under `lib/content/index.ts`
to use a shared helper that resolves the target with `path.resolve()` and
verifies it stays under the `baseDir` with `path.relative()` before any
`fs.readFile()` call. Keep the fix centralized so both readers use the same safe
helper and reject any escaped branch/semester/subjectCode traversal inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e04ffa4-a41a-436d-a404-8518d1f6ac1e
📒 Files selected for processing (16)
app/(public)/page.tsxapp/error.tsxapp/globals.cssapp/layout.tsxcomponents/ai/answer-viewer.tsxcomponents/ai/chat-mockup.tsxcomponents/ai/generate-answer-button.tsxcomponents/ai/workspace-chat.tsxcomponents/navbar.tsxcomponents/syllabus/module-card.tsxfeatures/landing/ai-demo.tsxfeatures/landing/stats.tsxfeatures/landing/testimonials.tsxfeatures/landing/universities.tsxlib/constants.tslib/content/index.ts
✅ Files skipped from review due to trivial changes (2)
- lib/constants.ts
- app/globals.css
🚧 Files skipped from review as they are similar to previous changes (12)
- app/error.tsx
- features/landing/ai-demo.tsx
- app/layout.tsx
- components/ai/answer-viewer.tsx
- features/landing/stats.tsx
- app/(public)/page.tsx
- features/landing/testimonials.tsx
- components/navbar.tsx
- features/landing/universities.tsx
- components/ai/generate-answer-button.tsx
- components/ai/workspace-chat.tsx
- components/syllabus/module-card.tsx
Summary by CodeRabbit
New Features
Bug Fixes
Accessibility / UI