fix(ui): restore tea.ClearScreen on tab switch to eliminate ghost text#49
Closed
fullstackjam wants to merge 3 commits intomainfrom
Closed
fix(ui): restore tea.ClearScreen on tab switch to eliminate ghost text#49fullstackjam wants to merge 3 commits intomainfrom
fullstackjam wants to merge 3 commits intomainfrom
Conversation
…editor A corrupted or old-format snapshot file can contain a MacOSPref entry with an empty Key (e.g. Domain="cirruslabs/cli", Key=""), which would render as "cirruslabs/cli." in the macOS Prefs tab instead of in Taps. Filter out any MacOSPref entry where Domain or Key is empty in NewSnapshotEditor, since every valid macOS preference requires both. The tap itself is already correctly shown in the Taps tab.
bubbletea v1.3.0 + x/ansi v0.8.0 made tea.ClearScreen fully functional again (it was effectively a no-op in v1.1.0 because x/ansi v0.4.2 had silently changed EraseEntireDisplay to clear only the scrollback buffer rather than the visible frame). PR #47 removed the tea.ClearScreen workaround under the assumption that v1.3.0's incremental diff renderer would clear rows correctly on its own, but the ghost-text symptom (cirruslabs/cli appearing at the top of the macOS Prefs tab after switching from the Taps tab) persists in practice across tested terminals. Restoring the explicit clear is the safest and most portable fix: tea.ClearScreen forces bubbletea to erase the alt-screen buffer and do a full repaint before rendering the new tab, guaranteeing no stale rows survive the transition. Also adds two tests: - TestSnapshotEditorTabSwitchReturnsClearScreenCmd: asserts Tab and ShiftTab return a non-nil cmd (the ClearScreen cmd). - TestSnapshotEditorViewTabIsolation: asserts that View() for the macOS Prefs tab never contains tap-item text, and vice-versa, locking in cross-tab isolation at the logic level.
|
👋 Thanks for opening this pull request! Before merging:
@fullstackjam will review this soon. Thanks for contributing! 🚀 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
return m, tea.ClearScreenon Tab/ShiftTab in the snapshot editor to eliminate ghost text (e.g.cirruslabs/cliappearing on the macOS Prefs tab)TestSnapshotEditorTabSwitchReturnsClearScreenCmd— asserts Tab and ShiftTab return a non-nil cmdTestSnapshotEditorViewTabIsolation— asserts View() output for macOS Prefs never contains tap items, and Taps tab never contains pref itemsRoot cause analysis
PR #47 removed
tea.ClearScreenon tab switch, believing bubbletea v1.3.0's incremental diff renderer would correctly clear changed rows on its own. In theory this is correct: v1.3.0 callsansi.CursorPositionandansi.EraseEntireScreendirectly (matching x/ansi v0.4+ API), fixing the MoveCursor argument-swap bug that existed in v1.1.0.In practice, ghost text still appears across terminals when switching from a tab with fewer items (Taps, 8 items) to one with more (macOS Prefs, 45 items). Restoring the explicit
tea.ClearScreenforces bubbletea to erase the entire alt-screen buffer and do a full repaint before rendering the new tab — the safest and most portable guarantee against stale rows.Note: with bubbletea v1.3.0 + x/ansi v0.8.0,
tea.ClearScreenis now fully functional (in v1.1.0 + x/ansi v0.4.2 theEraseEntireDisplayconstant had been silently changed to\x1b[3Jwhich only clears scrollback, making it a no-op on the visible frame).Test plan
make test-unitpasses (all packages)go test ./internal/ui/...— all 50+ snapshot editor tests pass including the two new onesmake build && ./openboot snapshot→ navigate to macOS Prefs tab —cirruslabs/clishould no longer appear there