Skip to content

feat(home): resizable chat/resource panel divider#3648

Open
waleedlatif1 wants to merge 1 commit intostagingfrom
feat/resize
Open

feat(home): resizable chat/resource panel divider#3648
waleedlatif1 wants to merge 1 commit intostagingfrom
feat/resize

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Added a draggable resize handle between the chat and resource (MothershipView) panels on the home page
  • Uses imperative DOM manipulation (zero React re-renders during drag) via a forwarded ref on the MothershipView root div
  • Handle straddles the border as a zero-width flex child with an absolutely-positioned 8px hit target — same pattern as the workflow panel resize handle
  • clearWidth() removes the inline style on collapse so the CSS class retakes control; a useEffect cleanup tears down listeners if the component unmounts mid-drag
  • Added MOTHERSHIP_WIDTH constants to stores/constants.ts (min: 280px, max: 70% viewport) consistent with other panel constants
  • Added min-w-[320px] to the chat area so it can't be crushed below a usable width
  • Fixed two useEffect anti-patterns (adjusting state on prop change → guarded render-phase updates) in mothership-view.tsx and home.tsx
  • Merged two chained Effects (skipResourceTransition setter + watcher) into one

Type of Change

  • New feature

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@cursor
Copy link

cursor bot commented Mar 18, 2026

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.

@vercel
Copy link

vercel bot commented Mar 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 18, 2026 4:05am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 18, 2026

Greptile Summary

This 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:

  • New useMothershipResize hook — attaches mousemove/mouseup listeners imperatively, clamps width between MOTHERSHIP_WIDTH.MIN (280 px) and 70% of the viewport, and exposes clearWidth() so the CSS class can retake control on collapse.
  • MothershipView now uses memo(forwardRef(...)) to expose its root <div> ref to the parent; useEffect for preview-mode reset replaced with a React-recommended guarded render-phase update.
  • Two merged effects in home.tsx — the skipResourceTransition setter and its watcher are consolidated into a single useEffect; chatId-driven editingInputValue reset is similarly moved to a render-phase guard.
  • window.open(..., '_blank') replaces router.push in EmbeddedWorkflowActions, opening workflows in a new tab to preserve the chat/resource context.
  • MOTHERSHIP_WIDTH constants added to stores/constants.ts alongside existing panel constants.

Notable concerns:

  • The resize handle container uses aria-hidden='true' while the inner element declares role='separator' and aria-label — the aria-hidden on the parent suppresses all descendant ARIA from the accessibility tree, rendering those attributes a no-op.
  • The fixed pixel width set during drag is never re-clamped when the browser window is resized after the drag ends, which could cause the panel to exceed 70% of a narrowed viewport.

Confidence Score: 4/5

  • PR is safe to merge; issues found are minor accessibility inconsistency and a non-critical edge case with post-drag viewport resize.
  • The core resize mechanism is well-implemented with proper cleanup and a familiar pattern. The memo(forwardRef(...)) refactor is correct, and the Effect anti-pattern fixes are valid. The two flagged issues — the aria-hidden/role conflict and the pixel-width clamping gap on window resize — do not affect functionality for the majority of users and do not introduce regressions.
  • home.tsx (aria-hidden conflict on resize handle) and use-mothership-resize.ts (no viewport-resize reclamping).

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/home/hooks/use-mothership-resize.ts New hook implementing imperative DOM-based panel resizing with proper event listener cleanup; minor issue with the fixed pixel width not being re-clamped on viewport resize.
apps/sim/app/workspace/[workspaceId]/home/home.tsx Integrates resize hook, adds the drag handle UI, and refactors two useEffect anti-patterns; the resize handle div has a conflicting aria-hidden/role combination that renders the ARIA attributes ineffective.
apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/mothership-view.tsx Correctly wraps component in memo(forwardRef(...)) to expose the root div ref; replaces a one-line useEffect with a guarded render-phase update following React's derived-state pattern.
apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx Changes handleOpenWorkflow from router.push to window.open, opening workflows in a new tab; this is an intentional UX shift but is a notable behavioral change not explicitly called out in the PR description.
apps/sim/stores/constants.ts Adds MOTHERSHIP_WIDTH constants (MIN: 280, MAX_PERCENTAGE: 0.7) consistent with existing panel width constants.
apps/sim/app/workspace/[workspaceId]/home/hooks/index.ts Barrel export update to expose the new useMothershipResize hook; no issues.

Sequence Diagram

sequenceDiagram
    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
Loading

Last reviewed commit: "feat(home): resizabl..."

Comment on lines +466 to +474
<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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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:

Suggested change
<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>

Comment on lines 162 to +163
const handleOpenWorkflow = useCallback(() => {
router.push(`/workspace/${workspaceId}/w/${workflowId}`)
}, [router, workspaceId, workflowId])
window.open(`/workspace/${workspaceId}/w/${workflowId}`, '_blank')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Comment on lines +33 to +35
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`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant