fix(ui): keep the last legend click across intermediate config reloads
- commit
- 320f6a594fd0f03b7ffe83dd60ab136914e3cbf3
- author
- brice · · 2026-10-14 08:21 UTC
- parent
- 5df8873
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +57 -10
M internal/ui/app_mode_frame.go +9 -10
| @@ -86,8 +86,8 @@ func (a *App) modeFrameAction(bw *browserWindow, action input.Action) { | |||
| 86 | 86 | // modeLegendPreference is the app-wide "legend collapsed" state, shared by | |
| 87 | 87 | // every mode and window. A click is applied at once and saved in the | |
| 88 | 88 | // background. It keeps priority over the configured value, which may still be | |
| 89 | - | // the previous one while the save is in flight, until a configuration reload | |
| 90 | - | // settles it (see configReloaded). | |
| 89 | + | // the previous one while saves are in flight, until a configuration reload | |
| 90 | + | // received after the writer drained settles it (see configReloaded). | |
| 91 | 91 | type modeLegendPreference struct { | |
| 92 | 92 | mu sync.Mutex | |
| 93 | 93 | collapsed bool // last click; meaningful while clicked is set | |
| @@ -97,7 +97,6 @@ type modeLegendPreference struct { | |||
| 97 | 97 | // and the file ends on the last click. | |
| 98 | 98 | pending *bool | |
| 99 | 99 | saving bool | |
| 100 | - | idle sync.WaitGroup | |
| 101 | 100 | } | |
| 102 | 101 | ||
| 103 | 102 | // current returns the state to show, given the configured one. | |
| @@ -120,7 +119,6 @@ func (p *modeLegendPreference) click(collapsed bool) (startWriter bool) { | |||
| 120 | 119 | return false | |
| 121 | 120 | } | |
| 122 | 121 | p.saving = true | |
| 123 | - | p.idle.Add(1) | |
| 124 | 122 | return true | |
| 125 | 123 | } | |
| 126 | 124 | ||
| @@ -130,20 +128,21 @@ func (p *modeLegendPreference) next() (collapsed, ok bool) { | |||
| 130 | 128 | defer p.mu.Unlock() | |
| 131 | 129 | if p.pending == nil { | |
| 132 | 130 | p.saving = false | |
| 133 | - | p.idle.Done() | |
| 134 | 131 | return false, false | |
| 135 | 132 | } | |
| 136 | 133 | collapsed, p.pending = *p.pending, nil | |
| 137 | 134 | return collapsed, true | |
| 138 | 135 | } | |
| 139 | 136 | ||
| 140 | - | // configReloaded hands the state back to the configuration when a reload | |
| 141 | - | // agrees with the last click (it is that click's own save), or when no save is | |
| 142 | - | // in flight (an external edit, or a save that failed). | |
| 143 | - | func (p *modeLegendPreference) configReloaded(configured bool) { | |
| 137 | + | // configReloaded hands the state back to the configuration on a reload | |
| 138 | + | // received once the writer has drained: that reload reflects the saved clicks | |
| 139 | + | // (or an external edit, or a failed save). Reloads received while the writer | |
| 140 | + | // runs are ignored: they may come from an earlier save and would undo the | |
| 141 | + | // latest click. | |
| 142 | + | func (p *modeLegendPreference) configReloaded(_ bool) { | |
| 144 | 143 | p.mu.Lock() | |
| 145 | 144 | defer p.mu.Unlock() | |
| 146 | - | if !p.saving || configured == p.collapsed { | |
| 145 | + | if !p.saving { | |
| 147 | 146 | p.clicked = false | |
| 148 | 147 | } | |
| 149 | 148 | } | |
| @@ -86,8 +86,8 @@ func (a *App) modeFrameAction(bw *browserWindow, action input.Action) { | |||||
| 86 | // modeLegendPreference is the app-wide "legend collapsed" state, shared by | 86 | // modeLegendPreference is the app-wide "legend collapsed" state, shared by | ||
| 87 | // every mode and window. A click is applied at once and saved in the | 87 | // every mode and window. A click is applied at once and saved in the | ||
| 88 | // background. It keeps priority over the configured value, which may still be | 88 | // background. It keeps priority over the configured value, which may still be | ||
| 89 | - | // the previous one while the save is in flight, until a configuration reload | 89 | + | // the previous one while saves are in flight, until a configuration reload |
| 90 | - | // settles it (see configReloaded). | 90 | + | // received after the writer drained settles it (see configReloaded). |
| 91 | type modeLegendPreference struct { | 91 | type modeLegendPreference struct { | ||
| 92 | mu sync.Mutex | 92 | mu sync.Mutex | ||
| 93 | collapsed bool // last click; meaningful while clicked is set | 93 | collapsed bool // last click; meaningful while clicked is set | ||
| @@ -97,7 +97,6 @@ type modeLegendPreference struct { | |||||
| 97 | // and the file ends on the last click. | 97 | // and the file ends on the last click. | ||
| 98 | pending *bool | 98 | pending *bool | ||
| 99 | saving bool | 99 | saving bool | ||
| 100 | - | idle sync.WaitGroup | |||
| 101 | } | 100 | } | ||
| 102 | 101 | ||||
| 103 | // current returns the state to show, given the configured one. | 102 | // current returns the state to show, given the configured one. | ||
| @@ -120,7 +119,6 @@ func (p *modeLegendPreference) click(collapsed bool) (startWriter bool) { | |||||
| 120 | return false | 119 | return false | ||
| 121 | } | 120 | } | ||
| 122 | p.saving = true | 121 | p.saving = true | ||
| 123 | - | p.idle.Add(1) | |||
| 124 | return true | 122 | return true | ||
| 125 | } | 123 | } | ||
| 126 | 124 | ||||
| @@ -130,20 +128,21 @@ func (p *modeLegendPreference) next() (collapsed, ok bool) { | |||||
| 130 | defer p.mu.Unlock() | 128 | defer p.mu.Unlock() | ||
| 131 | if p.pending == nil { | 129 | if p.pending == nil { | ||
| 132 | p.saving = false | 130 | p.saving = false | ||
| 133 | - | p.idle.Done() | |||
| 134 | return false, false | 131 | return false, false | ||
| 135 | } | 132 | } | ||
| 136 | collapsed, p.pending = *p.pending, nil | 133 | collapsed, p.pending = *p.pending, nil | ||
| 137 | return collapsed, true | 134 | return collapsed, true | ||
| 138 | } | 135 | } | ||
| 139 | 136 | ||||
| 140 | - | // configReloaded hands the state back to the configuration when a reload | 137 | + | // configReloaded hands the state back to the configuration on a reload |
| 141 | - | // agrees with the last click (it is that click's own save), or when no save is | 138 | + | // received once the writer has drained: that reload reflects the saved clicks |
| 142 | - | // in flight (an external edit, or a save that failed). | 139 | + | // (or an external edit, or a failed save). Reloads received while the writer |
| 143 | - | func (p *modeLegendPreference) configReloaded(configured bool) { | 140 | + | // runs are ignored: they may come from an earlier save and would undo the |
| 141 | + | // latest click. | |||
| 142 | + | func (p *modeLegendPreference) configReloaded(_ bool) { | |||
| 144 | p.mu.Lock() | 143 | p.mu.Lock() | ||
| 145 | defer p.mu.Unlock() | 144 | defer p.mu.Unlock() | ||
| 146 | - | if !p.saving || configured == p.collapsed { | 145 | + | if !p.saving { |
| 147 | p.clicked = false | 146 | p.clicked = false | ||
| 148 | } | 147 | } | ||
| 149 | } | 148 | } | ||
M internal/ui/mode_toaster_test.go +48 -0
| @@ -473,6 +473,54 @@ func TestModeLegendPreferenceClickWinsUntilSettled(t *testing.T) { | |||
| 473 | 473 | require.True(t, p.current(true)) | |
| 474 | 474 | } | |
| 475 | 475 | ||
| 476 | + | func TestModeLegendPreferenceReloadsDuringSavesDoNotUndoLastClick(t *testing.T) { | |
| 477 | + | var p modeLegendPreference | |
| 478 | + | require.True(t, p.click(true)) | |
| 479 | + | first, ok := p.next() // the writer takes the first value | |
| 480 | + | require.True(t, ok) | |
| 481 | + | require.True(t, first) | |
| 482 | + | require.False(t, p.click(false), "the writer is still active") | |
| 483 | + | ||
| 484 | + | p.configReloaded(false) // matches the latest click, but the save is still pending | |
| 485 | + | p.configReloaded(true) // the first save's own reload, arriving late | |
| 486 | + | require.False(t, p.current(true), "the latest click survives intermediate reloads") | |
| 487 | + | ||
| 488 | + | second, ok := p.next() | |
| 489 | + | require.True(t, ok) | |
| 490 | + | require.False(t, second) | |
| 491 | + | require.False(t, p.current(true), "still the latest click until the writer drains") | |
| 492 | + | _, ok = p.next() | |
| 493 | + | require.False(t, ok) | |
| 494 | + | require.False(t, p.current(true), "drained: reloads have not been received yet") | |
| 495 | + | ||
| 496 | + | p.configReloaded(false) // first reload after the drain | |
| 497 | + | require.False(t, p.current(false)) | |
| 498 | + | require.True(t, p.current(true), "the configuration decides again") | |
| 499 | + | } | |
| 500 | + | ||
| 501 | + | func TestModeLegendPreferenceReloadAfterDrain(t *testing.T) { | |
| 502 | + | tests := []struct { | |
| 503 | + | name string | |
| 504 | + | reloaded bool | |
| 505 | + | configured bool | |
| 506 | + | }{ | |
| 507 | + | {"own save reloads", true, true}, | |
| 508 | + | {"external edit", false, false}, | |
| 509 | + | {"failed save leaves the file unchanged", false, false}, | |
| 510 | + | } | |
| 511 | + | for _, tt := range tests { | |
| 512 | + | t.Run(tt.name, func(t *testing.T) { | |
| 513 | + | var p modeLegendPreference | |
| 514 | + | require.True(t, p.click(true)) | |
| 515 | + | _, _ = p.next() | |
| 516 | + | _, _ = p.next() | |
| 517 | + | p.configReloaded(tt.reloaded) | |
| 518 | + | require.Equal(t, tt.configured, p.current(tt.configured)) | |
| 519 | + | require.Equal(t, !tt.configured, p.current(!tt.configured)) | |
| 520 | + | }) | |
| 521 | + | } | |
| 522 | + | } | |
| 523 | + | ||
| 476 | 524 | func TestModeLegendPreferenceCoalescesClicksAndKeepsLast(t *testing.T) { | |
| 477 | 525 | var p modeLegendPreference | |
| 478 | 526 | require.True(t, p.click(true)) | |
| @@ -473,6 +473,54 @@ func TestModeLegendPreferenceClickWinsUntilSettled(t *testing.T) { | |||||
| 473 | require.True(t, p.current(true)) | 473 | require.True(t, p.current(true)) | ||
| 474 | } | 474 | } | ||
| 475 | 475 | ||||
| 476 | + | func TestModeLegendPreferenceReloadsDuringSavesDoNotUndoLastClick(t *testing.T) { | |||
| 477 | + | var p modeLegendPreference | |||
| 478 | + | require.True(t, p.click(true)) | |||
| 479 | + | first, ok := p.next() // the writer takes the first value | |||
| 480 | + | require.True(t, ok) | |||
| 481 | + | require.True(t, first) | |||
| 482 | + | require.False(t, p.click(false), "the writer is still active") | |||
| 483 | + | ||||
| 484 | + | p.configReloaded(false) // matches the latest click, but the save is still pending | |||
| 485 | + | p.configReloaded(true) // the first save's own reload, arriving late | |||
| 486 | + | require.False(t, p.current(true), "the latest click survives intermediate reloads") | |||
| 487 | + | ||||
| 488 | + | second, ok := p.next() | |||
| 489 | + | require.True(t, ok) | |||
| 490 | + | require.False(t, second) | |||
| 491 | + | require.False(t, p.current(true), "still the latest click until the writer drains") | |||
| 492 | + | _, ok = p.next() | |||
| 493 | + | require.False(t, ok) | |||
| 494 | + | require.False(t, p.current(true), "drained: reloads have not been received yet") | |||
| 495 | + | ||||
| 496 | + | p.configReloaded(false) // first reload after the drain | |||
| 497 | + | require.False(t, p.current(false)) | |||
| 498 | + | require.True(t, p.current(true), "the configuration decides again") | |||
| 499 | + | } | |||
| 500 | + | ||||
| 501 | + | func TestModeLegendPreferenceReloadAfterDrain(t *testing.T) { | |||
| 502 | + | tests := []struct { | |||
| 503 | + | name string | |||
| 504 | + | reloaded bool | |||
| 505 | + | configured bool | |||
| 506 | + | }{ | |||
| 507 | + | {"own save reloads", true, true}, | |||
| 508 | + | {"external edit", false, false}, | |||
| 509 | + | {"failed save leaves the file unchanged", false, false}, | |||
| 510 | + | } | |||
| 511 | + | for _, tt := range tests { | |||
| 512 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 513 | + | var p modeLegendPreference | |||
| 514 | + | require.True(t, p.click(true)) | |||
| 515 | + | _, _ = p.next() | |||
| 516 | + | _, _ = p.next() | |||
| 517 | + | p.configReloaded(tt.reloaded) | |||
| 518 | + | require.Equal(t, tt.configured, p.current(tt.configured)) | |||
| 519 | + | require.Equal(t, !tt.configured, p.current(!tt.configured)) | |||
| 520 | + | }) | |||
| 521 | + | } | |||
| 522 | + | } | |||
| 523 | + | ||||
| 476 | func TestModeLegendPreferenceCoalescesClicksAndKeepsLast(t *testing.T) { | 524 | func TestModeLegendPreferenceCoalescesClicksAndKeepsLast(t *testing.T) { | ||
| 477 | var p modeLegendPreference | 525 | var p modeLegendPreference | ||
| 478 | require.True(t, p.click(true)) | 526 | require.True(t, p.click(true)) | ||