feat(home): resizable chat/resource panel divider#3648
feat(home): resizable chat/resource panel divider#3648waleedlatif1 wants to merge 1 commit intostagingfrom
Conversation
|
You have used all Bugbot PR reviews included in your free trial for your GitHub account on this workspace. To continue using Bugbot reviews, enable Bugbot for your team in the Cursor dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR adds a draggable resize handle between the chat and resource (MothershipView) panels on the home page, using imperative DOM manipulation via a forwarded ref to achieve zero React re-renders during drag — consistent with the existing workflow panel resize pattern. Key changes:
Notable concerns:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ResizeHandle as Resize Handle (DOM)
participant Hook as useMothershipResize
participant Panel as MothershipView (DOM)
User->>ResizeHandle: mousedown
ResizeHandle->>Hook: handleResizeMouseDown(e)
Hook->>Panel: getBoundingClientRect() → pin current width
Hook->>Panel: style.transition = 'none'
Hook->>Hook: store cleanup in cleanupRef
Hook->>Hook: document.addEventListener('mousemove')
Hook->>Hook: document.addEventListener('mouseup')
loop During drag
User->>Hook: mousemove
Hook->>Hook: newWidth = window.innerWidth − clientX
Hook->>Hook: clamp(newWidth, MIN, MAX_PERCENTAGE × innerWidth)
Hook->>Panel: style.width = clampedWidth
end
User->>Hook: mouseup
Hook->>Panel: style.transition = prevTransition
Hook->>Hook: remove mousemove / mouseup listeners
Hook->>Hook: cleanupRef.current = null
alt User collapses panel
User->>Hook: collapseResource() → clearWidth()
Hook->>Panel: style.removeProperty('width')
Hook->>Panel: isCollapsed = true → CSS class w-0 takes control
end
alt Component unmounts mid-drag
Hook->>Hook: useEffect cleanup → cleanupRef.current?.()
Hook->>Panel: restore transition, remove listeners
end
Last reviewed commit: "feat(home): resizabl..." |
| <div className='relative z-20 w-0 flex-none' aria-hidden='true'> | ||
| <div | ||
| className='absolute inset-y-0 left-[-4px] w-[8px] cursor-ew-resize' | ||
| role='separator' | ||
| aria-orientation='vertical' | ||
| aria-label='Resize resource panel' | ||
| onMouseDown={handleResizeMouseDown} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
aria-hidden hides the ARIA separator from assistive technology
The outer <div> has aria-hidden='true', which propagates to all descendants and removes the entire subtree from the accessibility tree. This means the role='separator', aria-orientation='vertical', and aria-label='Resize resource panel' on the inner <div> are completely invisible to screen readers — they have no effect.
Either fully hide the handle from AT (consistent, since it's a pointer-only interaction) or expose the separator properly by removing aria-hidden from the outer wrapper:
| <div className='relative z-20 w-0 flex-none' aria-hidden='true'> | |
| <div | |
| className='absolute inset-y-0 left-[-4px] w-[8px] cursor-ew-resize' | |
| role='separator' | |
| aria-orientation='vertical' | |
| aria-label='Resize resource panel' | |
| onMouseDown={handleResizeMouseDown} | |
| /> | |
| </div> | |
| <div className='relative z-20 w-0 flex-none'> | |
| <div | |
| className='absolute inset-y-0 left-[-4px] w-[8px] cursor-ew-resize' | |
| role='separator' | |
| aria-orientation='vertical' | |
| aria-label='Resize resource panel' | |
| onMouseDown={handleResizeMouseDown} | |
| /> | |
| </div> |
| const handleOpenWorkflow = useCallback(() => { | ||
| router.push(`/workspace/${workspaceId}/w/${workflowId}`) | ||
| }, [router, workspaceId, workflowId]) | ||
| window.open(`/workspace/${workspaceId}/w/${workflowId}`, '_blank') |
There was a problem hiding this comment.
window.open may be blocked by popup blockers
Changing from router.push to window.open(..., '_blank') is a meaningful UX shift — the workflow now opens in a new tab rather than navigating the current page. While this makes sense in the context of keeping the resizable resource panel open, window.open calls that aren't directly within a synchronous click handler can be silently suppressed by popup blockers in some browsers. Since this callback is wrapped in useCallback and then bound to an onClick, it should be fine in practice, but it's worth confirming that this new behavior is intentional and documented for users who may expect same-tab navigation.
| const newWidth = window.innerWidth - moveEvent.clientX | ||
| const maxWidth = window.innerWidth * MOTHERSHIP_WIDTH.MAX_PERCENTAGE | ||
| el.style.width = `${Math.min(Math.max(newWidth, MOTHERSHIP_WIDTH.MIN), maxWidth)}px` |
There was a problem hiding this comment.
Panel width not clamped after viewport resize
MAX_PERCENTAGE is checked only during a drag operation. Once the user finishes dragging and the panel has a fixed inline width in pixels (e.g. 560px), resizing the browser window to a narrower viewport will not re-apply the 70% cap. The panel could then occupy more than the intended maximum share of screen space, potentially crushing the chat column below its min-w-[320px] floor.
Consider adding a resize event listener (or a ResizeObserver on the container) to re-clamp the stored width when the viewport shrinks, or store the width as a percentage rather than an absolute pixel value.
Summary
clearWidth()removes the inline style on collapse so the CSS class retakes control; auseEffectcleanup tears down listeners if the component unmounts mid-dragMOTHERSHIP_WIDTHconstants tostores/constants.ts(min: 280px, max: 70% viewport) consistent with other panel constantsmin-w-[320px]to the chat area so it can't be crushed below a usable widthuseEffectanti-patterns (adjusting state on prop change → guarded render-phase updates) inmothership-view.tsxandhome.tsxskipResourceTransitionsetter + watcher) into oneType of Change
Testing
Tested manually
Checklist