fix(floating): repaint immediately when hiding popup
- commit
- 5e8bad47972e57cf1d286e9e4b584d09a9d760a2
- author
- brice · · 2026-07-13 12:36 UTC
- parent
- bb3b15d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +71 -0
M internal/usecase/daemon/floating.go +6 -0
| @@ -200,10 +200,12 @@ func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | |||
| 200 | 200 | } | |
| 201 | 201 | cfg := d.currentFloatingConfig() | |
| 202 | 202 | tb.mu.Lock() | |
| 203 | + | wasVisible := tb.floating.state == floatingVisible | |
| 203 | 204 | start, generation := tb.toggleFloatingLocked() | |
| 204 | 205 | // desiredVisible is meaningful only while a launch is warming. Installed | |
| 205 | 206 | // slots derive their resize and paint work from the resulting slot state. | |
| 206 | 207 | visible := tb.floating.state == floatingVisible | |
| 208 | + | hidden := wasVisible && tb.floating.state == floatingHidden | |
| 207 | 209 | tb.mu.Unlock() | |
| 208 | 210 | if start { | |
| 209 | 211 | d.launchFloating(sess, tb, cfg, generation, true) | |
| @@ -212,6 +214,10 @@ func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | |||
| 212 | 214 | } | |
| 213 | 215 | return nil | |
| 214 | 216 | } | |
| 217 | + | if hidden { | |
| 218 | + | d.invalidateRender(sess, ac, true, "floating.go") | |
| 219 | + | return nil | |
| 220 | + | } | |
| 215 | 221 | if visible { | |
| 216 | 222 | tb.mu.Lock() | |
| 217 | 223 | size := domain.Size{Cols: tb.size.Cols, Rows: tb.size.Rows + 2} | |
| @@ -200,10 +200,12 @@ func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | |||||
| 200 | } | 200 | } | ||
| 201 | cfg := d.currentFloatingConfig() | 201 | cfg := d.currentFloatingConfig() | ||
| 202 | tb.mu.Lock() | 202 | tb.mu.Lock() | ||
| 203 | + | wasVisible := tb.floating.state == floatingVisible | |||
| 203 | start, generation := tb.toggleFloatingLocked() | 204 | start, generation := tb.toggleFloatingLocked() | ||
| 204 | // desiredVisible is meaningful only while a launch is warming. Installed | 205 | // desiredVisible is meaningful only while a launch is warming. Installed | ||
| 205 | // slots derive their resize and paint work from the resulting slot state. | 206 | // slots derive their resize and paint work from the resulting slot state. | ||
| 206 | visible := tb.floating.state == floatingVisible | 207 | visible := tb.floating.state == floatingVisible | ||
| 208 | + | hidden := wasVisible && tb.floating.state == floatingHidden | |||
| 207 | tb.mu.Unlock() | 209 | tb.mu.Unlock() | ||
| 208 | if start { | 210 | if start { | ||
| 209 | d.launchFloating(sess, tb, cfg, generation, true) | 211 | d.launchFloating(sess, tb, cfg, generation, true) | ||
| @@ -212,6 +214,10 @@ func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | |||||
| 212 | } | 214 | } | ||
| 213 | return nil | 215 | return nil | ||
| 214 | } | 216 | } | ||
| 217 | + | if hidden { | |||
| 218 | + | d.invalidateRender(sess, ac, true, "floating.go") | |||
| 219 | + | return nil | |||
| 220 | + | } | |||
| 215 | if visible { | 221 | if visible { | ||
| 216 | tb.mu.Lock() | 222 | tb.mu.Lock() | ||
| 217 | size := domain.Size{Cols: tb.size.Cols, Rows: tb.size.Rows + 2} | 223 | size := domain.Size{Cols: tb.size.Cols, Rows: tb.size.Rows + 2} | ||
M internal/usecase/daemon/input_test.go +65 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "encoding/base64" | |
| 5 | 6 | "fmt" | |
| 6 | 7 | "strings" | |
| @@ -169,6 +170,70 @@ func TestAltCForwardsToPTY(t *testing.T) { | |||
| 169 | 170 | releasePTY() | |
| 170 | 171 | } | |
| 171 | 172 | ||
| 173 | + | func TestAltFToggleRetainedFloatingPaneRepaintsImmediately(t *testing.T) { | |
| 174 | + | normal, releaseNormal := newBlockingPTY(t) | |
| 175 | + | floatingPTY, releaseFloating := newBlockingPTY(t) | |
| 176 | + | defer releaseNormal() | |
| 177 | + | defer releaseFloating() | |
| 178 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, normal) | |
| 179 | + | client := vt.NewScreen(80, 25) | |
| 180 | + | floatingCtx, cancelFloating := context.WithCancel(sess.ctx) | |
| 181 | + | defer cancelFloating() | |
| 182 | + | floating := newPane("floating", floatingPTY, domain.Size{Cols: 20, Rows: 5}) | |
| 183 | + | floating.ctx = floatingCtx | |
| 184 | + | floating.screen.Write([]byte("popup-content")) | |
| 185 | + | installTestFloating(sess.activeTab(), floating, false) | |
| 186 | + | ||
| 187 | + | // Establish the client shadow while the retained popup is hidden. | |
| 188 | + | d.paint(sess, ac, true) | |
| 189 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |
| 190 | + | ||
| 191 | + | // Route the real Alt+F binding. Showing a retained pane must paint once. | |
| 192 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |
| 193 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |
| 194 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |
| 195 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "┌") | |
| 196 | + | select { | |
| 197 | + | case extra := <-sends: | |
| 198 | + | t.Fatalf("show emitted duplicate output: %#v", extra) | |
| 199 | + | default: | |
| 200 | + | } | |
| 201 | + | ||
| 202 | + | // The second real key must repaint immediately, rather than waiting for | |
| 203 | + | // unrelated output, while retaining the existing PTY and context. | |
| 204 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |
| 205 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |
| 206 | + | require.NotContains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |
| 207 | + | tb := sess.activeTab() | |
| 208 | + | tb.mu.Lock() | |
| 209 | + | require.Equal(t, floatingHidden, tb.floating.state) | |
| 210 | + | require.Same(t, floating, tb.floating.pane) | |
| 211 | + | generation := tb.floating.generation | |
| 212 | + | tb.mu.Unlock() | |
| 213 | + | require.Same(t, floatingPTY, floating.pty) | |
| 214 | + | select { | |
| 215 | + | case <-floating.ctx.Done(): | |
| 216 | + | t.Fatal("hiding retained popup cancelled its context") | |
| 217 | + | default: | |
| 218 | + | } | |
| 219 | + | ||
| 220 | + | // A third show uses the installed pane; it must not launch or emit a second | |
| 221 | + | // resize/start frame. | |
| 222 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |
| 223 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |
| 224 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |
| 225 | + | tb.mu.Lock() | |
| 226 | + | require.Equal(t, floatingVisible, tb.floating.state) | |
| 227 | + | require.Same(t, floating, tb.floating.pane) | |
| 228 | + | require.Equal(t, generation, tb.floating.generation) | |
| 229 | + | tb.mu.Unlock() | |
| 230 | + | select { | |
| 231 | + | case extra := <-sends: | |
| 232 | + | t.Fatalf("retained show emitted duplicate output: %#v", extra) | |
| 233 | + | default: | |
| 234 | + | } | |
| 235 | + | } | |
| 236 | + | ||
| 172 | 237 | func requireFloatingInitialized(t *testing.T, tb *tab) { | |
| 173 | 238 | t.Helper() | |
| 174 | 239 | tb.mu.Lock() | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "encoding/base64" | 5 | "encoding/base64" | ||
| 5 | "fmt" | 6 | "fmt" | ||
| 6 | "strings" | 7 | "strings" | ||
| @@ -169,6 +170,70 @@ func TestAltCForwardsToPTY(t *testing.T) { | |||||
| 169 | releasePTY() | 170 | releasePTY() | ||
| 170 | } | 171 | } | ||
| 171 | 172 | ||||
| 173 | + | func TestAltFToggleRetainedFloatingPaneRepaintsImmediately(t *testing.T) { | |||
| 174 | + | normal, releaseNormal := newBlockingPTY(t) | |||
| 175 | + | floatingPTY, releaseFloating := newBlockingPTY(t) | |||
| 176 | + | defer releaseNormal() | |||
| 177 | + | defer releaseFloating() | |||
| 178 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, normal) | |||
| 179 | + | client := vt.NewScreen(80, 25) | |||
| 180 | + | floatingCtx, cancelFloating := context.WithCancel(sess.ctx) | |||
| 181 | + | defer cancelFloating() | |||
| 182 | + | floating := newPane("floating", floatingPTY, domain.Size{Cols: 20, Rows: 5}) | |||
| 183 | + | floating.ctx = floatingCtx | |||
| 184 | + | floating.screen.Write([]byte("popup-content")) | |||
| 185 | + | installTestFloating(sess.activeTab(), floating, false) | |||
| 186 | + | ||||
| 187 | + | // Establish the client shadow while the retained popup is hidden. | |||
| 188 | + | d.paint(sess, ac, true) | |||
| 189 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |||
| 190 | + | ||||
| 191 | + | // Route the real Alt+F binding. Showing a retained pane must paint once. | |||
| 192 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |||
| 193 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |||
| 194 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |||
| 195 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "┌") | |||
| 196 | + | select { | |||
| 197 | + | case extra := <-sends: | |||
| 198 | + | t.Fatalf("show emitted duplicate output: %#v", extra) | |||
| 199 | + | default: | |||
| 200 | + | } | |||
| 201 | + | ||||
| 202 | + | // The second real key must repaint immediately, rather than waiting for | |||
| 203 | + | // unrelated output, while retaining the existing PTY and context. | |||
| 204 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |||
| 205 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |||
| 206 | + | require.NotContains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |||
| 207 | + | tb := sess.activeTab() | |||
| 208 | + | tb.mu.Lock() | |||
| 209 | + | require.Equal(t, floatingHidden, tb.floating.state) | |||
| 210 | + | require.Same(t, floating, tb.floating.pane) | |||
| 211 | + | generation := tb.floating.generation | |||
| 212 | + | tb.mu.Unlock() | |||
| 213 | + | require.Same(t, floatingPTY, floating.pty) | |||
| 214 | + | select { | |||
| 215 | + | case <-floating.ctx.Done(): | |||
| 216 | + | t.Fatal("hiding retained popup cancelled its context") | |||
| 217 | + | default: | |||
| 218 | + | } | |||
| 219 | + | ||||
| 220 | + | // A third show uses the installed pane; it must not launch or emit a second | |||
| 221 | + | // resize/start frame. | |||
| 222 | + | d.handleInput(sess, ac, []byte("\x1bf")) | |||
| 223 | + | mustApplyOutput(t, client, awaitFrame(t, sends, ports.MsgOutput)) | |||
| 224 | + | require.Contains(t, strings.Join(frameRows(client.Frame), "\n"), "popup-content") | |||
| 225 | + | tb.mu.Lock() | |||
| 226 | + | require.Equal(t, floatingVisible, tb.floating.state) | |||
| 227 | + | require.Same(t, floating, tb.floating.pane) | |||
| 228 | + | require.Equal(t, generation, tb.floating.generation) | |||
| 229 | + | tb.mu.Unlock() | |||
| 230 | + | select { | |||
| 231 | + | case extra := <-sends: | |||
| 232 | + | t.Fatalf("retained show emitted duplicate output: %#v", extra) | |||
| 233 | + | default: | |||
| 234 | + | } | |||
| 235 | + | } | |||
| 236 | + | ||||
| 172 | func requireFloatingInitialized(t *testing.T, tb *tab) { | 237 | func requireFloatingInitialized(t *testing.T, tb *tab) { | ||
| 173 | t.Helper() | 238 | t.Helper() | ||
| 174 | tb.mu.Lock() | 239 | tb.mu.Lock() | ||