Skip to content

fix(terminal): reduce resize flicker#1968

Merged
arnestrickmann merged 3 commits into
generalaction:mainfrom
janburzinski:emdash/terminal-flicker-ku5an
May 13, 2026
Merged

fix(terminal): reduce resize flicker#1968
arnestrickmann merged 3 commits into
generalaction:mainfrom
janburzinski:emdash/terminal-flicker-ku5an

Conversation

@janburzinski
Copy link
Copy Markdown
Collaborator

summary

fixes the terminal flickering when resizing (sidebar or terminal drawer)

Screen.Recording.2026-05-11.at.14.59.06.mov

@lassejlv
Copy link
Copy Markdown

Let's gooo

@jschwxrz
Copy link
Copy Markdown
Collaborator

@greptileai

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 11, 2026

Greptile Summary

This PR eliminates terminal flicker during panel/sidebar resizes through three coordinated mechanisms: removing CanvasAddon (which caused full-canvas repaints), making measureAndResize synchronous (so xterm's DOM updates before the browser paints), and adding a time-based suppression window (suppressFor) to the panel drag store so ResizeObserver events from programmatic panel toggles are coalesced into a single measurement after layout settles.

  • DraggableResizeHandle extracts and reuses the drag-tracking pointer-handler logic that was previously inlined twice, and correctly forwards caller-provided handlers.
  • panelDragStore.suppressFor(ms) extends the store so programmatic sidebar/drawer toggles (cmd+B, cmd+J, navigation) suppress terminal resize calls during the layout burst and fire exactly one resize afterwards.
  • measureAndResize drops the requestAnimationFrame wrapper and the ResizeObserver gains a leading-edge + trailing-edge debounce, ensuring single-shot layout changes are applied synchronously in the same paint frame while continuous window-corner resizes are still coalesced.

Confidence Score: 5/5

Safe to merge — changes are well-scoped to the resize/flicker path and preserve all existing drag-end measurement invariants.

The suppression logic correctly avoids spurious notifications via notifyIfChanged, the synchronous measureAndResize aligns with ResizeObserver's pre-paint timing guarantee, and the drag-end measurement effect is unchanged. No regressions were found in the terminal resize or PTY notification paths.

No files require special attention.

Important Files Changed

Filename Overview
src/renderer/lib/pty/use-pty.ts Removes the requestAnimationFrame wrapper from measureAndResize (now synchronous so xterm DOM catches up before paint) and adds leading-edge + trailing-edge debounce in the ResizeObserver callback.
src/renderer/lib/layout/panel-drag-store.ts Extends the store with a time-based suppressFor(ms) mechanism alongside the existing drag-bit, using notifyIfChanged to avoid spurious listener calls; logic is correct.
src/renderer/features/tasks/main-panel.tsx Extracts drag logic into reusable DraggableResizeHandle, adds suppressFor(140) before programmatic panel toggles, and now correctly forwards caller-provided pointer handlers.
src/renderer/lib/layout/layout-provider.tsx Adds suppressFor(140) before programmatic sidebar collapse/expand (cmd+B) so ResizeObserver events during the burst are suppressed and one resize fires after layout settles.
src/renderer/lib/pty/pty.ts Drops CanvasAddon and adds an inline comment explaining the tradeoff; renderer falls back to xterm's DOM renderer which avoids full-canvas repaints on resize.

Sequence Diagram

sequenceDiagram
    participant U as User (cmd+B / drag)
    participant LP as layout-provider / main-panel
    participant PDS as panelDragStore
    participant RO as ResizeObserver
    participant MP as measureAndResize

    note over U,MP: Programmatic toggle (cmd+B / cmd+J)
    U->>LP: trigger collapse/expand
    LP->>PDS: suppressFor(140ms)
    PDS-->>RO: "isPanelDragging = true (via useSyncExternalStore)"
    LP->>LP: panel.collapse() / panel.expand()
    LP->>RO: layout changes → ResizeObserver fires
    RO->>PDS: isPanelDraggingRef.current? → true → skip
    note over PDS: 140ms elapses
    PDS-->>RO: "isPanelDragging = false"
    PDS->>MP: drag-end useEffect → measureAndResize()

    note over U,MP: Manual drag
    U->>LP: pointerdown on DraggableResizeHandle
    LP->>PDS: setDragging(true)
    PDS-->>RO: "isPanelDragging = true"
    U->>LP: (dragging — many resize events)
    RO->>PDS: isPanelDraggingRef.current? → true → skip
    U->>LP: pointerup
    LP->>PDS: setDragging(false)
    PDS-->>RO: "isPanelDragging = false"
    PDS->>MP: drag-end useEffect → measureAndResize()

    note over U,MP: Continuous window resize (burst)
    U->>RO: ResizeObserver fires (first in quiet period)
    RO->>MP: leading-edge → fireResize()
    U->>RO: ResizeObserver fires (within 150ms)
    RO->>RO: schedule trailing timer (50ms)
    note over RO: 50ms elapses
    RO->>MP: trailing → fireResize()
Loading

Reviews (2): Last reviewed commit: "fix(terminal): address resize review com..." | Re-trigger Greptile

Comment on lines +121 to +132
<ResizableHandle
{...props}
onPointerDown={(e) => {
e.currentTarget.setPointerCapture(e.pointerId);
if (!draggingRef.current) {
draggingRef.current = true;
panelDragStore.setDragging(true);
}
}}
onPointerUp={stop}
onPointerCancel={stop}
/>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 The component spreads props first and then unconditionally overrides onPointerDown, onPointerUp, and onPointerCancel. Any caller that passes those handlers (or any future wrapper that enriches them) will have them silently dropped. Merging caller-provided handlers alongside the internal ones avoids the footgun.

Suggested change
<ResizableHandle
{...props}
onPointerDown={(e) => {
e.currentTarget.setPointerCapture(e.pointerId);
if (!draggingRef.current) {
draggingRef.current = true;
panelDragStore.setDragging(true);
}
}}
onPointerUp={stop}
onPointerCancel={stop}
/>
<ResizableHandle
{...props}
onPointerDown={(e) => {
props.onPointerDown?.(e);
e.currentTarget.setPointerCapture(e.pointerId);
if (!draggingRef.current) {
draggingRef.current = true;
panelDragStore.setDragging(true);
}
}}
onPointerUp={(e) => { props.onPointerUp?.(e); stop(); }}
onPointerCancel={(e) => { props.onPointerCancel?.(e); stop(); }}
/>
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/renderer/features/tasks/main-panel.tsx
Line: 121-132

Comment:
The component spreads `props` first and then unconditionally overrides `onPointerDown`, `onPointerUp`, and `onPointerCancel`. Any caller that passes those handlers (or any future wrapper that enriches them) will have them silently dropped. Merging caller-provided handlers alongside the internal ones avoids the footgun.

```suggestion
    <ResizableHandle
      {...props}
      onPointerDown={(e) => {
        props.onPointerDown?.(e);
        e.currentTarget.setPointerCapture(e.pointerId);
        if (!draggingRef.current) {
          draggingRef.current = true;
          panelDragStore.setDragging(true);
        }
      }}
      onPointerUp={(e) => { props.onPointerUp?.(e); stop(); }}
      onPointerCancel={(e) => { props.onPointerCancel?.(e); stop(); }}
    />
```

How can I resolve this? If you propose a fix, please make it concise.

@janburzinski
Copy link
Copy Markdown
Collaborator Author

@greptile

@jschwxrz
Copy link
Copy Markdown
Collaborator

nice! only thing would be performance tradeoffs when switching from canvas renderer to dom renderer for xterm. wdyt @Davidknp

@arnestrickmann arnestrickmann self-requested a review May 11, 2026 16:56
@janburzinski
Copy link
Copy Markdown
Collaborator Author

not very telling but interesting

Screenshot 2026-05-11 at 19 30 58

@arnestrickmann arnestrickmann merged commit 7549422 into generalaction:main May 13, 2026
1 check passed
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.

4 participants