Skip to content

[codex] fix v1 terminal resize repaint#3756

Merged
Kitenite merged 1 commit intomainfrom
fix-glyph-resize-bug-v1
Apr 26, 2026
Merged

[codex] fix v1 terminal resize repaint#3756
Kitenite merged 1 commit intomainfrom
fix-glyph-resize-bug-v1

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Apr 26, 2026

Summary

Fixes the v1 terminal resize/fit path so cached terminals repaint correctly after attach, resize, and font appearance changes.

Root cause

The v1 cached terminal fit logic was duplicated across attach, resize, and appearance updates. It fit only when the container was visible, but did not consistently preserve viewport state around fit() and refresh the visible rows after fitting. That can leave stale or blank glyph rendering after resize/reattach flows.

Changes

  • Track the live container while a cached v1 terminal is attached.
  • Centralize visible-host fit logic in fitAndRefresh().
  • Preserve pinned-bottom versus scrolled viewport state across fit.
  • Refresh visible rows after fit.
  • Notify the backend resize path only when terminal cols/rows actually change.

Validation

  • bunx @biomejs/biome@2.4.2 check --write --unsafe apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts
  • bun test apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.test.ts
  • bun run --cwd apps/desktop typecheck

Summary by cubic

Fixes v1 terminal repaint so cached terminals render correctly after attach, resize, and font changes. Centralizes fit logic to preserve scroll state and only trigger backend resize when size actually changes.

  • Bug Fixes
    • Track the live container and guard on host visibility.
    • Add fitAndRefresh() to fit, restore pinned-bottom or scrolled viewport, and refresh visible rows.
    • Deduplicate fit across attach/resize/appearance; manage ResizeObserver in the cache.
    • Update lastCols/lastRows and invoke resize callback only when cols/rows change.

Written for commit d61cefe. Summary will update on new commits.

Summary by CodeRabbit

  • Refactor
    • Improved terminal resizing and appearance update behavior by centralizing handling and better tracking of terminal dimensions. This enhances the responsiveness and reliability of terminal window resizing.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2bb76032-526e-4efb-a188-96f6edd6929f

📥 Commits

Reviewing files that changed from the base of the PR and between d3753d0 and d61cefe.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts

📝 Walkthrough

Walkthrough

The terminal cache refactoring centralizes resize behavior into a fitAndRefresh() method that tracks the attached DOM container, manages terminal fitting and refreshing, and detects dimension changes. Attachment, resize observation, and appearance updates now delegate to this unified method instead of independently invoking fit and refresh operations.

Changes

Cohort / File(s) Summary
Terminal Cache Refactoring
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts
Added container field to track attached DOM element; introduced fitAndRefresh() method to centralize terminal fitting and refreshing logic; updated attachment, detachment, resize observation, and appearance update flows to use the new unified method; ResizeObserver now only triggers onResize callback when dimensions actually change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related issues

Poem

🐰 A rabbit hops through cached flows,
Where fitAndRefresh now glows—
Container tracked, resize contained,
Terminal harmony attained!
Order from chaos, neat and tight,

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing v1 terminal resize/repaint behavior in cached terminals, which is the core objective.
Description check ✅ Passed The description includes all essential sections: Summary, Root cause, Changes, and Validation. It provides clear context, implementation details, and testing evidence.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-glyph-resize-bug-v1

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 26, 2026

Greptile Summary

This PR refactors v1 terminal cache fit/refresh logic into a single fitAndRefresh() helper, adds a container field to track the live host element, and preserves viewport scroll state (pinned-to-bottom vs. scrolled offset) across resize operations. The consolidation eliminates three duplicated inline fit blocks in attachToContainer, the ResizeObserver callback, and updateAppearance, and adds a guard so the backend is only notified of a resize when the terminal's column/row count actually changes.

Confidence Score: 5/5

Safe to merge — clean refactor with no logic regressions and improved correctness around viewport state and visibility guards.

All findings are P2 or lower. The centralized fitAndRefresh() function is logically equivalent to the three inline blocks it replaces, with added improvements (viewport preservation, refresh-only-when-visible). No data loss, race conditions, or broken contracts identified.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/v1-terminal-cache.ts Centralizes fit/refresh logic into fitAndRefresh(), adds container tracking, and preserves viewport state around resize — clean refactor with no logic regressions found.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[attachToContainer] --> B[entry.container = container]
    B --> C[appendChild wrapper]
    C --> D[fitAndRefresh]
    D --> E{hostIsVisible?}
    E -- No --> F[return false]
    E -- Yes --> G[save viewport state\nwasPinnedToBottom / savedViewportY]
    G --> H[fitAddon.fit]
    H --> I[update lastCols / lastRows]
    I --> J{wasPinnedToBottom?}
    J -- Yes --> K[scrollToBottom]
    J -- No --> L[scrollToLine clamped to baseY]
    K --> M[xterm.refresh 0..rows-1]
    L --> M
    M --> N[return cols/rows changed?]
    N --> O{ResizeObserver callback}
    O --> D
    N --> P{updateAppearance}
    P --> Q[update font options]
    Q --> D
    D2[detachFromContainer] --> R[disconnect ResizeObserver]
    R --> S[entry.container = null]
    S --> T[wrapper.remove]
Loading

Reviews (1): Last reviewed commit: "fix v1 terminal resize repaint" | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

@Kitenite Kitenite merged commit 62737db into main Apr 26, 2026
7 checks passed
@Kitenite Kitenite deleted the fix-glyph-resize-bug-v1 branch April 26, 2026 07:07
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch

Thank you for your contribution! 🎉

MocA-Love added a commit to MocA-Love/superset that referenced this pull request Apr 26, 2026
3 PR (#442, #443, #444) で取り込み済みの 9 commits を git 履歴上もマージ済みに記録する。
内容差分は無し (merge -s ours)。

取り込み内容:
- 6b96acd Improve sidebar group management UX (superset-sh#3745)
- a4079e7 Update dashboard sidebar workspace icons (superset-sh#3755)
- d3753d0 [codex] Use dynamic footer copyright years (superset-sh#3754)
- ce606be Handle browser passthrough during v2 resize (superset-sh#3744)
- b1e1eb7 Refactor v2 workspace page (superset-sh#3747)
- 8693869 [codex] move v2 toggle to experimental settings (superset-sh#3748)
- ef3f381 Revert "fix(desktop): refit v2 terminal after font settle (superset-sh#3742)" (superset-sh#3750) - 手動移植 (vibrancy patch 維持)
- 25b2d52 Show terminal sessions from all workspaces in dropdown (superset-sh#3751)
- 62737db fix v1 terminal resize repaint (superset-sh#3756)
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