Conversation
Contributor
📝 WalkthroughWalkthroughRefactors the app layout to integrate a new responsive AiLeftPanelLayout and AiLeftPanel components (desktop resizable panel and mobile drawer), adds a useIsMobile hook, introduces a Drawer component wrapping vaul primitives, and updates exports and minor footer styling. Changes
Sequence DiagramsequenceDiagram
participant User
participant Layout as AiLeftPanelLayout
participant Panel as AiLeftPanel
participant Desktop
participant Mobile
User->>Layout: Render / Interact (open/close/drag)
Layout->>Layout: useIsMobile() check
alt Desktop (isMobile = false)
Layout->>Desktop: Render left panel with resizer
User->>Desktop: Drag resizer
Desktop->>Layout: Update panelWidth
Layout->>Desktop: Re-render with new width
else Mobile (isMobile = true)
Layout->>Mobile: Render Drawer (panelOpen state)
User->>Mobile: Tap trigger / close
Mobile->>Panel: Open / Close
Panel->>Layout: onClose -> update panelOpen
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
DSanich
force-pushed
the
feat/1957-ai-left-panel
branch
from
24d0c31 to
d60f45c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/epics/src/common/ai-left-panel-layout.tsx (1)
69-73: Resizable separator should support keyboard interaction.This control is currently pointer-only. Consider adding
tabIndex,aria-valuemin,aria-valuemax, and Arrow key resizing to keep it operable for keyboard users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/ai-left-panel-layout.tsx` around lines 69 - 73, The separator is pointer-only; add keyboard support by making it focusable (add tabIndex=0), expose range limits (aria-valuemin and aria-valuemax) alongside the existing aria-valuenow={panelWidth}, and implement an onKeyDown handler (e.g., onResizeKeyDown) that listens for ArrowLeft/ArrowRight (and optionally Home/End/PageUp/PageDown) to increment/decrement panelWidth within the min/max bounds; wire that handler into the same element and reuse the existing resize logic or call the same state-updating function used by onResizeMouseDown so keyboard changes mirror pointer behavior.apps/web/src/app/layout.tsx (1)
138-141: Consider removing the unnecessary Fragment wrapper.The Fragment (
<>...</>) around the content and Footer is not required sinceAiLeftPanelLayoutacceptsReactNodechildren directly. You can simplify by removing it.♻️ Suggested simplification
<AiLeftPanelLayout> - <> - <div className="w-full shrink-0 pb-8">{children}</div> - <Footer /> - </> + <div className="w-full shrink-0 pb-8">{children}</div> + <Footer /> </AiLeftPanelLayout>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/layout.tsx` around lines 138 - 141, Remove the unnecessary fragment around the children and Footer by using a single parent element instead: eliminate the <>...</> wrapper and either move Footer inside the existing div with className "w-full shrink-0 pb-8" or replace the fragment with one parent container so AiLeftPanelLayout receives one ReactNode (keep references to children and Footer intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/epics/src/common/ai-left-panel-layout.tsx`:
- Around line 34-50: The drag listeners can remain active if a mouseup is
missed; update the drag teardown in the drag-start logic so mouseup is
registered with { once: true } and add a window 'blur' handler to perform the
same cleanup as onMouseUp; specifically, keep your existing onMouseMove and
onMouseUp functions (referenced by dragStartX, dragStartWidth, setPanelWidth,
setIsDragging) but register window.addEventListener('mouseup', onMouseUp, {
once: true }) and also add a window.addEventListener('blur', onMouseUp) so the
blur event will call the same cleanup removing the mousemove listener and
resetting isDragging; ensure the same onMouseMove and onMouseUp references are
used when removing listeners.
In `@packages/epics/src/common/ai-left-panel.tsx`:
- Line 3: Add "lucide-react" as a direct dependency of the packages/epics
package.json because ai-left-panel.tsx imports PanelLeftClose from
'lucide-react'; update the dependencies section in packages/epics/package.json
to include the same lucide-react version used by the workspace (or a compatible
semver), then run install/build to verify the import resolves without relying on
transitive root workspace deps.
In `@packages/ui/package.json`:
- Around line 19-21: package.json was updated to add the "vaul" dependency but
the lockfile was not updated causing CI mismatch; run pnpm install in the
repository root to regenerate/synchronize pnpm-lock.yaml (or the project's
lockfile), verify the lockfile changes include the "vaul" entry, and commit the
updated lockfile alongside the package.json update so CI can pass.
---
Nitpick comments:
In `@apps/web/src/app/layout.tsx`:
- Around line 138-141: Remove the unnecessary fragment around the children and
Footer by using a single parent element instead: eliminate the <>...</> wrapper
and either move Footer inside the existing div with className "w-full shrink-0
pb-8" or replace the fragment with one parent container so AiLeftPanelLayout
receives one ReactNode (keep references to children and Footer intact).
In `@packages/epics/src/common/ai-left-panel-layout.tsx`:
- Around line 69-73: The separator is pointer-only; add keyboard support by
making it focusable (add tabIndex=0), expose range limits (aria-valuemin and
aria-valuemax) alongside the existing aria-valuenow={panelWidth}, and implement
an onKeyDown handler (e.g., onResizeKeyDown) that listens for
ArrowLeft/ArrowRight (and optionally Home/End/PageUp/PageDown) to
increment/decrement panelWidth within the min/max bounds; wire that handler into
the same element and reuse the existing resize logic or call the same
state-updating function used by onResizeMouseDown so keyboard changes mirror
pointer behavior.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web/src/app/layout.tsxpackages/epics/src/common/ai-left-panel-layout.tsxpackages/epics/src/common/ai-left-panel.tsxpackages/epics/src/common/index.tspackages/epics/src/hooks/index.tspackages/epics/src/hooks/use-is-mobile.tsxpackages/ui/package.jsonpackages/ui/src/drawer.tsxpackages/ui/src/index.tspackages/ui/src/organisms/footer.tsx
Comment on lines
+34
to
+50
| const onMouseMove = (e: MouseEvent) => { | ||
| const delta = e.clientX - dragStartX.current; | ||
| const newWidth = Math.min( | ||
| MAX_WIDTH, | ||
| Math.max(MIN_WIDTH, dragStartWidth.current + delta), | ||
| ); | ||
| setPanelWidth(newWidth); | ||
| }; | ||
|
|
||
| const onMouseUp = () => { | ||
| setIsDragging(false); | ||
| window.removeEventListener('mousemove', onMouseMove); | ||
| window.removeEventListener('mouseup', onMouseUp); | ||
| }; | ||
|
|
||
| window.addEventListener('mousemove', onMouseMove); | ||
| window.addEventListener('mouseup', onMouseUp); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
FILE="packages/epics/src/common/ai-left-panel-layout.tsx"
wc -l "$FILE"Repository: hypha-dao/hypha-web
Length of output: 117
🏁 Script executed:
FILE="packages/epics/src/common/ai-left-panel-layout.tsx"
sed -n '1,80p' "$FILE"Repository: hypha-dao/hypha-web
Length of output: 2739
🏁 Script executed:
FILE="packages/epics/src/common/ai-left-panel-layout.tsx"
rg -n "addEventListener|removeEventListener|useEffect|useRef|isDragging" "$FILE" -A 2 -B 1Repository: hypha-dao/hypha-web
Length of output: 1298
Drag listener cleanup lacks robustness for missed mouseup events.
If the user releases the mouse outside the window or the tab loses focus mid-drag, mouseup may not fire on that window. The listeners and isDragging state would persist, and subsequent interactions could start in an inconsistent state. Protect against this by using { once: true } on one-shot listeners and adding a blur event handler as a secondary cleanup trigger.
💡 Hardening listener teardown
const onMouseUp = () => {
setIsDragging(false);
window.removeEventListener('mousemove', onMouseMove);
window.removeEventListener('mouseup', onMouseUp);
+ window.removeEventListener('blur', onMouseUp);
};
window.addEventListener('mousemove', onMouseMove);
- window.addEventListener('mouseup', onMouseUp);
+ window.addEventListener('mouseup', onMouseUp, { once: true });
+ window.addEventListener('blur', onMouseUp, { once: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/epics/src/common/ai-left-panel-layout.tsx` around lines 34 - 50, The
drag listeners can remain active if a mouseup is missed; update the drag
teardown in the drag-start logic so mouseup is registered with { once: true }
and add a window 'blur' handler to perform the same cleanup as onMouseUp;
specifically, keep your existing onMouseMove and onMouseUp functions (referenced
by dragStartX, dragStartWidth, setPanelWidth, setIsDragging) but register
window.addEventListener('mouseup', onMouseUp, { once: true }) and also add a
window.addEventListener('blur', onMouseUp) so the blur event will call the same
cleanup removing the mousemove listener and resetting isDragging; ensure the
same onMouseMove and onMouseUp references are used when removing listeners.
| @@ -0,0 +1,34 @@ | |||
| 'use client'; | |||
|
|
|||
| import { PanelLeftClose } from 'lucide-react'; | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd package.json | while read -r manifest; do
echo "== $manifest =="
jq -r '"name=\(.name // "unknown") | dep=\(.dependencies["lucide-react"] // "null") | devDep=\(.devDependencies["lucide-react"] // "null") | peerDep=\(.peerDependencies["lucide-react"] // "null")"' "$manifest"
doneRepository: hypha-dao/hypha-web
Length of output: 2299
Add lucide-react to packages/epics dependencies.
This file directly imports from lucide-react, but the package does not explicitly declare it as a dependency. Currently relying on the transitive dependency from the root workspace. Each package should declare its own dependencies to avoid strict-install/build breaks and maintain proper package boundaries.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/epics/src/common/ai-left-panel.tsx` at line 3, Add "lucide-react" as
a direct dependency of the packages/epics package.json because ai-left-panel.tsx
imports PanelLeftClose from 'lucide-react'; update the dependencies section in
packages/epics/package.json to include the same lucide-react version used by the
workspace (or a compatible semver), then run install/build to verify the import
resolves without relying on transitive root workspace deps.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ui/src/drawer.tsx (1)
45-49: Default bottom-drawer styles may need override for alternative directions.The default classes (
inset-x-0 bottom-0 mt-24 rounded-t-[10px]) assume a bottom-positioned drawer. When usingdirection="left"(as inAiLeftPanelLayout), consumers must override these viaclassNameandstyleprops. This works but requires careful attention.Consider documenting this behavior or providing variant props for different directions if left/right drawers become common.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/drawer.tsx` around lines 45 - 49, The Drawer component currently hardcodes bottom-oriented Tailwind classes in the className concatenation (e.g., 'inset-x-0 bottom-0 mt-24 rounded-t-[10px]') which forces consumers to override styles for left/right drawers; update Drawer to accept a direction prop (or variants) and map directions to appropriate class sets (e.g., bottom vs left/right positioning and rounded corners) instead of always using bottom classes, and update the className merge logic in the Drawer component so consumer-provided className still overrides defaults when needed; reference the Drawer component and its className concatenation logic to implement this change and add a brief note in the component docs about the direction prop behavior.apps/web/src/app/layout.tsx (1)
137-142: Consider removing the unnecessary fragment wrapper.The
AiLeftPanelLayoutacceptsReactNodeas children, so you can pass the content directly without the extra fragment.♻️ Suggested simplification
<AiLeftPanelLayout> - <> - <div className="w-full shrink-0 pb-8">{children}</div> - <Footer /> - </> + <div className="w-full shrink-0 pb-8">{children}</div> + <Footer /> </AiLeftPanelLayout>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/layout.tsx` around lines 137 - 142, Remove the unnecessary fragment around the layout content: inside AiLeftPanelLayout replace the <>...</> wrapper by a single container element that holds the page content and Footer (e.g., wrap the children and <Footer /> in one div), so update the JSX around AiLeftPanelLayout to render a single parent node containing {children} and Footer instead of using the fragment shorthand.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/app/layout.tsx`:
- Around line 137-142: Remove the unnecessary fragment around the layout
content: inside AiLeftPanelLayout replace the <>...</> wrapper by a single
container element that holds the page content and Footer (e.g., wrap the
children and <Footer /> in one div), so update the JSX around AiLeftPanelLayout
to render a single parent node containing {children} and Footer instead of using
the fragment shorthand.
In `@packages/ui/src/drawer.tsx`:
- Around line 45-49: The Drawer component currently hardcodes bottom-oriented
Tailwind classes in the className concatenation (e.g., 'inset-x-0 bottom-0 mt-24
rounded-t-[10px]') which forces consumers to override styles for left/right
drawers; update Drawer to accept a direction prop (or variants) and map
directions to appropriate class sets (e.g., bottom vs left/right positioning and
rounded corners) instead of always using bottom classes, and update the
className merge logic in the Drawer component so consumer-provided className
still overrides defaults when needed; reference the Drawer component and its
className concatenation logic to implement this change and add a brief note in
the component docs about the direction prop behavior.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/web/src/app/layout.tsxpackages/epics/src/common/ai-left-panel-layout.tsxpackages/epics/src/common/ai-left-panel.tsxpackages/epics/src/common/index.tspackages/epics/src/hooks/index.tspackages/epics/src/hooks/use-is-mobile.tsxpackages/ui/package.jsonpackages/ui/src/drawer.tsxpackages/ui/src/index.tspackages/ui/src/organisms/footer.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/ui/src/index.ts
- packages/epics/src/common/ai-left-panel-layout.tsx
- packages/ui/package.json
- packages/epics/src/common/index.ts
- packages/epics/src/hooks/use-is-mobile.tsx
- packages/epics/src/common/ai-left-panel.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Style