refactor(ai): improve workspace navigation layout#75
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a focused, distraction-free workspace layout for the RGPV AI chat route. The AI subject page is simplified to render only the chat loader, workspace-chat component state and UI are reworked (sidebar, collapsible follow-ups, breadcrumbs), and Breadcrumbs/Navbar/layout components gain route-detection logic to render minimal headers on that route. ChangesWorkspace focus AI chat redesign
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
components/ai/workspace-chat.tsx (2)
45-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
explanationCachedprop is declared but never consumed.
explanationCachedremains inWorkspaceChatProps(line 47) but is no longer destructured in the component (lines 127–141). The AI page still computes and passes it (explanationCached={explanationCached}at line 84 ofai/page.tsx), making the variable assignment, prop, and interface field all dead code.♻️ Proposed cleanup
Remove
explanationCachedfrom the interface:export interface WorkspaceChatProps { subjectCode: string; branch?: string; semester?: string; topicId?: string; topicTitle?: string; moduleTitle?: string; contextLabel?: string; cachedExplanation?: string; - explanationCached?: boolean; initialPrompts?: Array<{ prompt: string; topicId?: string; action?: string; }>;And remove the dead variable + prop in
app/rgpv/[branch]/[semester]/[subject]/ai/page.tsx:let cachedExplanation: string | undefined; -let explanationCached: boolean | undefined; if (isTopicMode && topicId) { try { const result = await generateTopicAnswer({ branch, semester, topicId, subjectCode: subject.toUpperCase(), action: "EXPLAIN", }); cachedExplanation = result.answer; - explanationCached = result.cached; } catch (error) {cachedExplanation={cachedExplanation} - explanationCached={explanationCached} initialPrompts={isTopicMode ? followupPrompts : generalPrompts}🤖 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 45 - 47, `explanationCached` is dead state in the chat flow: it is still declared in `WorkspaceChatProps` and passed from the AI page, but `WorkspaceChat` no longer reads it. Remove the unused `explanationCached` field from the `WorkspaceChatProps` interface and delete the corresponding variable/prop wiring in `ai/page.tsx`, keeping only the values that are actually consumed by `WorkspaceChat`.
778-835: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
SidebarDetailsdefined insideWorkspaceChatcauses unnecessary remounts.
SidebarDetailsis declared as a nested function component and rendered as<SidebarDetails />(lines 509, 522). On everyWorkspaceChatrender — including every keystroke — React sees a new component type and unmounts/remounts the entire subtree. This breaks the progress bar'smotion.divwidth animation (it jumps instead of animating) and can cause badge flicker.♻️ Proposed fix: extract SidebarDetails outside WorkspaceChat
Move
SidebarDetailsaboveWorkspaceChatand pass the needed values as props:function SidebarDetails({ sessionSaved, isTopicContextReady, topicTitle, moduleTitle, contextLabel, followupStatusLabel, progressPercent, lastStudied, }: { sessionSaved: boolean; isTopicContextReady: boolean; topicTitle?: string; moduleTitle?: string; contextLabel?: string; followupStatusLabel: string; progressPercent: number; lastStudied: { prefix: string; detail?: string } | null; }) { return ( <div className="space-y-4"> {/* ... existing JSX unchanged ... */} </div> ); }Then update the two call sites:
- <SidebarDetails /> + <SidebarDetails + sessionSaved={sessionSaved} + isTopicContextReady={isTopicContextReady} + topicTitle={topicTitle} + moduleTitle={moduleTitle} + contextLabel={contextLabel} + followupStatusLabel={followupStatusLabel} + progressPercent={progressPercent} + lastStudied={lastStudied} + />🤖 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 778 - 835, `SidebarDetails` is defined as an inner component inside `WorkspaceChat`, which makes React treat it as a new component type on each render and remount the subtree. Extract `SidebarDetails` to module scope and pass the values it needs (`sessionSaved`, `isTopicContextReady`, `topicTitle`, `moduleTitle`, `contextLabel`, `followupStatusLabel`, `progressPercent`, `lastStudied`) as props, then update the existing `<SidebarDetails />` call sites in `WorkspaceChat` to use the new prop-based component so the progress animation and badges stay mounted.components/breadcrumbs.tsx (1)
22-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnreachable
isWorkspaceFocusRouteternaries after early return.After
if (isWorkspaceFocusRoute) return null;(lines 14–16),isWorkspaceFocusRouteis alwaysfalsein the remaining JSX. The ternaries on lines 24, 28, 31, 40, 46, and 75 will always evaluate to the non-workspace branch, making the workspace-focus styling dead code that could mislead future developers.♻️ Proposed simplification
<div - className={`mx-auto w-full ${isWorkspaceFocusRoute ? "max-w-[1500px] px-4 sm:px-4 lg:px-6" : "max-w-7xl px-6 lg:px-8"}`} + className="mx-auto w-full max-w-7xl px-6 lg:px-8" > <nav aria-label="breadcrumb" - className={isWorkspaceFocusRoute ? "py-2" : "py-4"} + className="py-4" > <ol - className={`flex flex-wrap items-center gap-1.5 text-muted-foreground ${isWorkspaceFocusRoute ? "text-xs" : "text-sm"}`} + className="flex flex-wrap items-center gap-1.5 text-muted-foreground text-sm" > <li> <Link href="/" className="flex items-center transition-colors hover:text-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 rounded" aria-label="Home" > - <Home - className={isWorkspaceFocusRoute ? "h-3.5 w-3.5" : "h-4 w-4"} - /> + <Home className="h-4 w-4" /> </Link> </li> {pathNames.length > 0 && ( - <ChevronRight - className={isWorkspaceFocusRoute ? "h-3.5 w-3.5" : "h-4 w-4"} - /> + <ChevronRight className="h-4 w-4" /> )}And similarly for the per-segment
ChevronRightat line 73–77:- <ChevronRight - className={ - isWorkspaceFocusRoute ? "h-3.5 w-3.5" : "h-4 w-4" - } - /> + <ChevronRight className="h-4 w-4" />🤖 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/breadcrumbs.tsx` around lines 22 - 86, The `Breadcrumbs` component has dead `isWorkspaceFocusRoute` ternaries after the early return, so the remaining JSX in `Breadcrumbs` only ever uses the non-workspace branch. Simplify the conditional styling in `Breadcrumbs` by removing those unreachable workspace-focus checks and keeping only the active branch for the rendered breadcrumb markup, including the wrapper, `nav`, `ol`, `Home`, and `ChevronRight` elements.
🤖 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 `@components/ai/workspace-chat.tsx`:
- Around line 593-667: The chat view in workspace-chat.tsx is missing the
auto-scroll behavior because the removed messagesEndRef/scrollToBottom logic was
not replaced, so new conversation updates can stay out of view. Restore
scrolling by adding a ref and effect in workspace-chat.tsx that tracks the
latest message/loading state and scrolls the message list to the bottom when
conversationMessages, explanationMessage, or loading changes. Attach that ref to
the overflow-y-auto container that wraps the message list so the latest
WorkspaceMessage entries are always visible.
---
Nitpick comments:
In `@components/ai/workspace-chat.tsx`:
- Around line 45-47: `explanationCached` is dead state in the chat flow: it is
still declared in `WorkspaceChatProps` and passed from the AI page, but
`WorkspaceChat` no longer reads it. Remove the unused `explanationCached` field
from the `WorkspaceChatProps` interface and delete the corresponding
variable/prop wiring in `ai/page.tsx`, keeping only the values that are actually
consumed by `WorkspaceChat`.
- Around line 778-835: `SidebarDetails` is defined as an inner component inside
`WorkspaceChat`, which makes React treat it as a new component type on each
render and remount the subtree. Extract `SidebarDetails` to module scope and
pass the values it needs (`sessionSaved`, `isTopicContextReady`, `topicTitle`,
`moduleTitle`, `contextLabel`, `followupStatusLabel`, `progressPercent`,
`lastStudied`) as props, then update the existing `<SidebarDetails />` call
sites in `WorkspaceChat` to use the new prop-based component so the progress
animation and badges stay mounted.
In `@components/breadcrumbs.tsx`:
- Around line 22-86: The `Breadcrumbs` component has dead
`isWorkspaceFocusRoute` ternaries after the early return, so the remaining JSX
in `Breadcrumbs` only ever uses the non-workspace branch. Simplify the
conditional styling in `Breadcrumbs` by removing those unreachable
workspace-focus checks and keeping only the active branch for the rendered
breadcrumb markup, including the wrapper, `nav`, `ol`, `Home`, and
`ChevronRight` elements.
🪄 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: d5bbc28a-7439-4668-a4bb-7f09c062340a
📒 Files selected for processing (7)
app/layout.tsxapp/rgpv/[branch]/[semester]/[subject]/ai/page.tsxapp/rgpv/layout.tsxcomponents/ai/workspace-chat.tsxcomponents/ai/workspace-message.tsxcomponents/breadcrumbs.tsxcomponents/navbar.tsx
| <div className="min-h-0 flex-1 overflow-y-auto px-2.5 py-2 sm:px-4"> | ||
| {!isTopicContextReady && messages.length === 0 ? ( | ||
| <div className="flex min-h-full flex-col items-center justify-center py-8 text-center"> | ||
| <div className="mb-4 rounded-full bg-blue-500/10 p-4"> | ||
| <Sparkles className="h-8 w-8 text-blue-600 dark:text-blue-400" /> | ||
| </div> | ||
| )} | ||
| <h3 className="text-lg font-semibold text-foreground"> | ||
| Welcome to Hyper AI Workspace | ||
| </h3> | ||
| <p className="mt-2 max-w-md text-sm text-muted-foreground"> | ||
| {welcomeMessage} | ||
| </p> | ||
| </div> | ||
| ) : ( | ||
| <div className="space-y-3 pb-2"> | ||
| {explanationLoading && !explanationMessage && ( | ||
| <div className="flex items-center justify-center py-8"> | ||
| <Loader2 className="h-6 w-6 animate-spin text-blue-600 dark:text-blue-400" /> | ||
| </div> | ||
| )} | ||
|
|
||
| <AnimatePresence initial={false}> | ||
| {messages.map((message, index) => ( | ||
| <motion.div | ||
| key={message.id} | ||
| initial={false} | ||
| animate={{ opacity: 1, y: 0 }} | ||
| transition={{ | ||
| duration: 0.3, | ||
| delay: index === messages.length - 1 ? 0 : 0.05, | ||
| }} | ||
| > | ||
| <WorkspaceMessage | ||
| answer={message.content} | ||
| role={message.role} | ||
| timestamp={message.timestamp} | ||
| /> | ||
| </motion.div> | ||
| ))} | ||
| {loading && ( | ||
| <motion.div | ||
| initial={{ opacity: 0, y: 20 }} | ||
| animate={{ opacity: 1, y: 0 }} | ||
| className="flex items-start gap-3" | ||
| > | ||
| <div className="flex h-8 w-8 flex-shrink-0 items-center justify-center rounded-full bg-blue-500/10"> | ||
| <Sparkles className="h-4 w-4 text-blue-600 dark:text-blue-400" /> | ||
| </div> | ||
| <div className="rounded-2xl rounded-tl-md border border-border bg-muted/30 px-4 py-3"> | ||
| <Loader2 className="h-5 w-5 animate-spin text-blue-600 dark:text-blue-400" /> | ||
| </div> | ||
| </motion.div> | ||
| {explanationMessage && ( | ||
| <WorkspaceMessage | ||
| answer={explanationMessage.content} | ||
| role={explanationMessage.role} | ||
| timestamp={explanationMessage.timestamp} | ||
| /> | ||
| )} | ||
| </AnimatePresence> | ||
|
|
||
| {isFollowupMode && | ||
| !hasConversation && | ||
| !explanationLoading && | ||
| !limitReached && ( | ||
| <div className="rounded-2xl border border-dashed border-border bg-muted/10 p-4 text-center"> | ||
| <p className="text-sm text-foreground"> | ||
| Start asking questions about this topic. | ||
| </p> | ||
| <p className="mt-1 text-xs text-muted-foreground"> | ||
| Your progress will be remembered on this device. | ||
|
|
||
| <AnimatePresence initial={false}> | ||
| {conversationMessages.map((message, index) => ( | ||
| <motion.div | ||
| key={message.id} | ||
| initial={false} | ||
| animate={{ opacity: 1, y: 0 }} | ||
| transition={{ | ||
| duration: 0.3, | ||
| delay: | ||
| index === conversationMessages.length - 1 ? 0 : 0.05, | ||
| }} | ||
| > | ||
| <WorkspaceMessage | ||
| answer={message.content} | ||
| role={message.role} | ||
| timestamp={message.timestamp} | ||
| /> | ||
| </motion.div> | ||
| ))} | ||
| {loading && ( | ||
| <motion.div | ||
| initial={{ opacity: 0, y: 20 }} | ||
| animate={{ opacity: 1, y: 0 }} | ||
| className="flex items-start gap-3" | ||
| > | ||
| <div className="flex h-8 w-8 flex-shrink-0 items-center justify-center rounded-full bg-blue-500/10"> | ||
| <Sparkles className="h-4 w-4 text-blue-600 dark:text-blue-400" /> | ||
| </div> | ||
| <div className="rounded-2xl rounded-tl-md border border-border bg-muted/30 px-4 py-3"> | ||
| <Loader2 className="h-5 w-5 animate-spin text-blue-600 dark:text-blue-400" /> | ||
| </div> | ||
| </motion.div> | ||
| )} | ||
| </AnimatePresence> | ||
|
|
||
| {error && ( | ||
| <div className="flex items-start gap-2 rounded-xl border border-red-500/20 bg-red-500/10 p-3"> | ||
| <AlertCircle className="h-4 w-4 flex-shrink-0 text-red-500" /> | ||
| <p className="text-sm text-red-600 dark:text-red-400"> | ||
| {error} | ||
| </p> | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing auto-scroll to latest message — chat UX regression.
The previous messagesEndRef/scrollToBottom wiring was removed (per diff at lines 181–200) but no replacement was added. The message container at line 593 has overflow-y-auto but no ref or scroll effect, so new messages (both user and AI) won't be visible without manual scrolling — a core chat UX expectation.
🐛 Proposed fix: restore auto-scroll
Add a ref and effect to scroll to bottom when messages or loading state change:
const inputRef = useRef<HTMLTextAreaElement>(null);
const abortControllerRef = useRef<AbortController | null>(null);
const explanationRequestedRef = useRef(initialWorkspace.explanationRequested);
const skipPersistRef = useRef(initialWorkspace.skipPersist);
+const scrollRef = useRef<HTMLDivElement>(null); useEffect(() => {
inputRef.current?.focus();
}, []);
+useEffect(() => {
+ if (scrollRef.current) {
+ scrollRef.current.scrollTop = scrollRef.current.scrollHeight;
+ }
+}, [messages, loading]);Then attach the ref to the scroll container:
- <div className="min-h-0 flex-1 overflow-y-auto px-2.5 py-2 sm:px-4">
+ <div ref={scrollRef} className="min-h-0 flex-1 overflow-y-auto px-2.5 py-2 sm:px-4">📝 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.
| <div className="min-h-0 flex-1 overflow-y-auto px-2.5 py-2 sm:px-4"> | |
| {!isTopicContextReady && messages.length === 0 ? ( | |
| <div className="flex min-h-full flex-col items-center justify-center py-8 text-center"> | |
| <div className="mb-4 rounded-full bg-blue-500/10 p-4"> | |
| <Sparkles className="h-8 w-8 text-blue-600 dark:text-blue-400" /> | |
| </div> | |
| )} | |
| <h3 className="text-lg font-semibold text-foreground"> | |
| Welcome to Hyper AI Workspace | |
| </h3> | |
| <p className="mt-2 max-w-md text-sm text-muted-foreground"> | |
| {welcomeMessage} | |
| </p> | |
| </div> | |
| ) : ( | |
| <div className="space-y-3 pb-2"> | |
| {explanationLoading && !explanationMessage && ( | |
| <div className="flex items-center justify-center py-8"> | |
| <Loader2 className="h-6 w-6 animate-spin text-blue-600 dark:text-blue-400" /> | |
| </div> | |
| )} | |
| <AnimatePresence initial={false}> | |
| {messages.map((message, index) => ( | |
| <motion.div | |
| key={message.id} | |
| initial={false} | |
| animate={{ opacity: 1, y: 0 }} | |
| transition={{ | |
| duration: 0.3, | |
| delay: index === messages.length - 1 ? 0 : 0.05, | |
| }} | |
| > | |
| <WorkspaceMessage | |
| answer={message.content} | |
| role={message.role} | |
| timestamp={message.timestamp} | |
| /> | |
| </motion.div> | |
| ))} | |
| {loading && ( | |
| <motion.div | |
| initial={{ opacity: 0, y: 20 }} | |
| animate={{ opacity: 1, y: 0 }} | |
| className="flex items-start gap-3" | |
| > | |
| <div className="flex h-8 w-8 flex-shrink-0 items-center justify-center rounded-full bg-blue-500/10"> | |
| <Sparkles className="h-4 w-4 text-blue-600 dark:text-blue-400" /> | |
| </div> | |
| <div className="rounded-2xl rounded-tl-md border border-border bg-muted/30 px-4 py-3"> | |
| <Loader2 className="h-5 w-5 animate-spin text-blue-600 dark:text-blue-400" /> | |
| </div> | |
| </motion.div> | |
| {explanationMessage && ( | |
| <WorkspaceMessage | |
| answer={explanationMessage.content} | |
| role={explanationMessage.role} | |
| timestamp={explanationMessage.timestamp} | |
| /> | |
| )} | |
| </AnimatePresence> | |
| {isFollowupMode && | |
| !hasConversation && | |
| !explanationLoading && | |
| !limitReached && ( | |
| <div className="rounded-2xl border border-dashed border-border bg-muted/10 p-4 text-center"> | |
| <p className="text-sm text-foreground"> | |
| Start asking questions about this topic. | |
| </p> | |
| <p className="mt-1 text-xs text-muted-foreground"> | |
| Your progress will be remembered on this device. | |
| <AnimatePresence initial={false}> | |
| {conversationMessages.map((message, index) => ( | |
| <motion.div | |
| key={message.id} | |
| initial={false} | |
| animate={{ opacity: 1, y: 0 }} | |
| transition={{ | |
| duration: 0.3, | |
| delay: | |
| index === conversationMessages.length - 1 ? 0 : 0.05, | |
| }} | |
| > | |
| <WorkspaceMessage | |
| answer={message.content} | |
| role={message.role} | |
| timestamp={message.timestamp} | |
| /> | |
| </motion.div> | |
| ))} | |
| {loading && ( | |
| <motion.div | |
| initial={{ opacity: 0, y: 20 }} | |
| animate={{ opacity: 1, y: 0 }} | |
| className="flex items-start gap-3" | |
| > | |
| <div className="flex h-8 w-8 flex-shrink-0 items-center justify-center rounded-full bg-blue-500/10"> | |
| <Sparkles className="h-4 w-4 text-blue-600 dark:text-blue-400" /> | |
| </div> | |
| <div className="rounded-2xl rounded-tl-md border border-border bg-muted/30 px-4 py-3"> | |
| <Loader2 className="h-5 w-5 animate-spin text-blue-600 dark:text-blue-400" /> | |
| </div> | |
| </motion.div> | |
| )} | |
| </AnimatePresence> | |
| {error && ( | |
| <div className="flex items-start gap-2 rounded-xl border border-red-500/20 bg-red-500/10 p-3"> | |
| <AlertCircle className="h-4 w-4 flex-shrink-0 text-red-500" /> | |
| <p className="text-sm text-red-600 dark:text-red-400"> | |
| {error} | |
| </p> | |
| </div> | |
| )} | |
| </div> | |
| )} | |
| </div> | |
| const inputRef = useRef<HTMLTextAreaElement>(null); | |
| const abortControllerRef = useRef<AbortController | null>(null); | |
| const explanationRequestedRef = useRef(initialWorkspace.explanationRequested); | |
| const skipPersistRef = useRef(initialWorkspace.skipPersist); | |
| const scrollRef = useRef<HTMLDivElement>(null); | |
| useEffect(() => { | |
| inputRef.current?.focus(); | |
| }, []); | |
| useEffect(() => { | |
| if (scrollRef.current) { | |
| scrollRef.current.scrollTop = scrollRef.current.scrollHeight; | |
| } | |
| }, [messages, loading]); | |
| <div ref={scrollRef} className="min-h-0 flex-1 overflow-y-auto px-2.5 py-2 sm:px-4"> |
🤖 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 593 - 667, The chat view in
workspace-chat.tsx is missing the auto-scroll behavior because the removed
messagesEndRef/scrollToBottom logic was not replaced, so new conversation
updates can stay out of view. Restore scrolling by adding a ref and effect in
workspace-chat.tsx that tracks the latest message/loading state and scrolls the
message list to the bottom when conversationMessages, explanationMessage, or
loading changes. Attach that ref to the overflow-y-auto container that wraps the
message list so the latest WorkspaceMessage entries are always visible.
🎉 Congratulations @imuniqueshiv!Thank you for contributing to HyperLearningTech. Your pull request has been successfully merged into main. 📦 Merge Summary
🚀 Keep Contributing
Thank you for helping make HyperLearningTech better. Happy Coding! 🚀 |
Pull Request
Summary
Refactor the Hyper AI Workspace layout to improve navigation structure and prepare the workspace for a cleaner, more maintainable UI architecture.
Related Issue
N/A
Type of Change
What Changed?
Screenshots (UI Changes Only)
Updated Hyper AI Workspace navigation and layout.
NA
Testing
Verified the changes locally by running:
npm run format:checknpm run lintnpm run typechecknpm run buildThe application was also tested locally to ensure the workspace renders correctly without affecting existing functionality.
Checklist
main.npx prettier --write <file>).npm run format:checkpasses.npm run lintpasses.npm run typecheckpasses.npm run buildpasses.Additional Notes
This PR focuses solely on restructuring the Hyper AI Workspace UI. It does not modify AI generation, API routes, caching, markdown rendering, or any backend functionality.
Summary by CodeRabbit
New Features
Bug Fixes