detached c64260b

#18 fix(focus): flash the focused window on click focus

state
merged · c64260b · fast-forward
branch
fix/click-focus-flash → main
opened
bnema ·
updated

Clicking a window that changes focus now triggers the focus flash, as keyboard focus actions do.

  • clickFocus and beginDrag (modifier-click move/resize) share focusClick.
  • The flash targets the resulting focus (modal dialog, not its parent).
  • No flash for an already-focused window, hover focus, or Focus.Flash=false.
  • Tested: core regression tests, headless e2e.
CommitSubjectAuthorDateSignatureStatus
c64260btest(app): accept cancellation during session lock teardownbriceGPG Verified
716a579fix(focus): flash the resulting focus on modal and drag clicksbriceGPG Verified
0c04aeefix(focus): flash the focused window on click focusbriceGPG Verified

Checks 2

4 files · +131 -9
Unified
UnifiedSplit
2/2
⋯

Merge settings

modes
fast-forward, squash, merge commit
checks
failing or unfinished checks block the merge
M internal/app/session_lock_test.go +4 -1
@@ -3,6 +3,7 @@ package app
33 import (
44 "bytes"
55 "context"
6+ "errors"
67 "io"
78 "os"
89 "path/filepath"
@@ -52,7 +53,9 @@ func TestHeadlessSessionLockEndToEnd(t *testing.T) {
5253 }
5354 select {
5455 case err := <-done:
55- if err != nil {
56+ // We canceled Run above. Workers may report that cancellation
57+ // during shutdown; it is not a failed session-lock assertion.
58+ if err != nil && !errors.Is(err, context.Canceled) {
5659 t.Errorf("Run: %v", err)
5760 }
5861 case <-time.After(3 * time.Second):
@@ -3,6 +3,7 @@ package app
3 import (3 import (
4 "bytes"4 "bytes"
5 "context"5 "context"
6+ "errors"
6 "io"7 "io"
7 "os"8 "os"
8 "path/filepath"9 "path/filepath"
@@ -52,7 +53,9 @@ func TestHeadlessSessionLockEndToEnd(t *testing.T) {
52 }53 }
53 select {54 select {
54 case err := <-done:55 case err := <-done:
55- if err != nil {56+ // We canceled Run above. Workers may report that cancellation
57+ // during shutdown; it is not a failed session-lock assertion.
58+ if err != nil && !errors.Is(err, context.Canceled) {
56 t.Errorf("Run: %v", err)59 t.Errorf("Run: %v", err)
57 }60 }
58 case <-time.After(3 * time.Second):61 case <-time.After(3 * time.Second):
M internal/core/drag.go +1 -2
@@ -154,14 +154,13 @@ func (c *Core) beginDrag(ctx context.Context, id WindowID, button uint32, resize
154154 return nil
155155 }
156156 // Click raises a float above the columns: Escape keeps that.
157- w.Click(id)
157+ c.focusClick(s, w, id)
158158 d := &dragState{id: id, button: button, resize: resize, edges: edges, start: [2]float64{c.cursorX, c.cursorY}}
159159 if i := w.floatIndex(id); i >= 0 {
160160 d.float, d.from, d.rect = true, w.Floats[i], w.floatRect(w.Floats[i])
161161 }
162162 c.drag = d
163163 c.buttons[button] = true
164- c.focusScreen = c.screenIndex(s.name())
165164 c.grab = 0
166165 if c.pointer != 0 {
167166 c.pointer = 0
@@ -154,14 +154,13 @@ func (c *Core) beginDrag(ctx context.Context, id WindowID, button uint32, resize
154 return nil154 return nil
155 }155 }
156 // Click raises a float above the columns: Escape keeps that.156 // Click raises a float above the columns: Escape keeps that.
157- w.Click(id)157+ c.focusClick(s, w, id)
158 d := &dragState{id: id, button: button, resize: resize, edges: edges, start: [2]float64{c.cursorX, c.cursorY}}158 d := &dragState{id: id, button: button, resize: resize, edges: edges, start: [2]float64{c.cursorX, c.cursorY}}
159 if i := w.floatIndex(id); i >= 0 {159 if i := w.floatIndex(id); i >= 0 {
160 d.float, d.from, d.rect = true, w.Floats[i], w.floatRect(w.Floats[i])160 d.float, d.from, d.rect = true, w.Floats[i], w.floatRect(w.Floats[i])
161 }161 }
162 c.drag = d162 c.drag = d
163 c.buttons[button] = true163 c.buttons[button] = true
164- c.focusScreen = c.screenIndex(s.name())
165 c.grab = 0164 c.grab = 0
166 if c.pointer != 0 {165 if c.pointer != 0 {
167 c.pointer = 0166 c.pointer = 0
M internal/core/flash_test.go +113 -4
@@ -389,11 +389,8 @@ func TestFlashNoImplicitFocusTriggers(t *testing.T) {
389389 if err := c.pointerMotion(ctx, ports.PointerMotion{X: 50, Y: 50}); err != nil {
390390 t.Fatal(err)
391391 }
392- if err := c.pointerButton(ctx, ports.PointerButton{Button: btnLeft, Pressed: true}); err != nil {
393- t.Fatal(err)
394- }
395392 if c.flash.pending != nil {
396- t.Fatal("pointer focus flashed")
393+ t.Fatal("hover focus flashed")
397394 }
398395 }
399396
@@ -774,3 +771,115 @@ func TestFlashFollowsInnerBorderAndRadius(t *testing.T) {
774771 })
775772 }
776773 }
774+
775+func TestFlashClickFocus(t *testing.T) {
776+ for _, test := range []struct {
777+ name string
778+ target WindowID
779+ enabled bool
780+ want bool
781+ }{
782+ {"different window", 1, true, true},
783+ {"already focused", 3, true, false},
784+ {"disabled", 1, false, false},
785+ } {
786+ t.Run(test.name, func(t *testing.T) {
787+ c, clock := flashCore(t)
788+ c.cfg.Focus.Flash = test.enabled
789+ indicatorScene(t, c)
790+ if c.keyboardFocus() != 3 {
791+ t.Fatal("setup: expected window 3 focused")
792+ }
793+ c.pointer = test.target
794+ if err := c.pointerButton(context.Background(), ports.PointerButton{Button: btnLeft, Pressed: true}); err != nil {
795+ t.Fatal(err)
796+ }
797+ if c.keyboardFocus() != test.target {
798+ t.Fatalf("focus = %d, want %d", c.keyboardFocus(), test.target)
799+ }
800+ if (c.flash.pending != nil) != test.want {
801+ t.Fatalf("flash request = %+v, want flash %v", c.flash.pending, test.want)
802+ }
803+ // Let the click's viewport transition settle before the flash.
804+ clock.now = clock.now.Add(time.Second)
805+ stepScenes(t, c, c.cur())
806+ if c.flashTick() != test.want {
807+ t.Fatalf("flash start: want %v", test.want)
808+ }
809+ if test.want && (c.flash.running.window != test.target || c.flash.running.level != flashWindow || c.flashFor(c.cur()) == nil) {
810+ t.Fatalf("wrong running flash: %+v", c.flash.running)
811+ }
812+ })
813+ }
814+}
815+
816+func TestFlashClickModalFocus(t *testing.T) {
817+ for _, unchanged := range []bool{false, true} {
818+ t.Run(map[bool]string{false: "changed", true: "unchanged"}[unchanged], func(t *testing.T) {
819+ c, _ := flashCore(t)
820+ w := c.cur().mon.Current()
821+ c.windows.mapped(ports.WindowMapped{ID: 4}, c.now())
822+ w.AddDialog(4, 1, 100, 80)
823+ w.SetModal(4, true)
824+ if !unchanged {
825+ w.Click(2)
826+ }
827+ indicatorScene(t, c)
828+ c.pointer = 1
829+ if err := c.pointerButton(context.Background(), ports.PointerButton{Button: btnLeft, Pressed: true}); err != nil {
830+ t.Fatal(err)
831+ }
832+ if c.keyboardFocus() != 4 {
833+ t.Fatalf("focus = %d, want modal 4", c.keyboardFocus())
834+ }
835+ if unchanged {
836+ if c.flash.pending != nil {
837+ t.Fatalf("unchanged modal flashed: %+v", c.flash.pending)
838+ }
839+ } else if c.flash.pending == nil || c.flash.pending.window != 4 {
840+ t.Fatalf("want modal flash, got %+v", c.flash.pending)
841+ }
842+ })
843+ }
844+}
845+
846+func TestFlashModifierClickDrag(t *testing.T) {
847+ for _, resize := range []bool{false, true} {
848+ for _, mode := range []string{"changed", "unchanged", "disabled"} {
849+ t.Run(map[bool]string{false: "move", true: "resize"}[resize]+"/"+mode, func(t *testing.T) {
850+ c, clock := flashCore(t)
851+ w := c.cur().mon.Current()
852+ w.Click(1)
853+ if resize {
854+ w.Apply(ActionToggleFloating)
855+ }
856+ if mode != "unchanged" {
857+ w.Click(3)
858+ }
859+ c.cfg.Focus.Flash = mode != "disabled"
860+ indicatorScene(t, c)
861+ c.pointer = 1
862+ c.mods = c.cmdMod
863+ button := uint32(btnLeft)
864+ if resize {
865+ button = btnRight
866+ }
867+ if err := c.pointerButton(context.Background(), ports.PointerButton{Button: button, Pressed: true}); err != nil {
868+ t.Fatal(err)
869+ }
870+ if c.drag == nil || c.drag.id != 1 || c.drag.resize != resize {
871+ t.Fatalf("drag not started: %+v", c.drag)
872+ }
873+ want := mode == "changed"
874+ if (c.flash.pending != nil) != want {
875+ t.Fatalf("flash = %+v, want %v", c.flash.pending, want)
876+ }
877+ clock.now = clock.now.Add(time.Second)
878+ stepScenes(t, c, c.cur())
879+ if c.flashTick() != want {
880+ t.Fatalf("flash start: want %v", want)
881+ }
882+ })
883+ }
884+ }
885+}
@@ -389,11 +389,8 @@ func TestFlashNoImplicitFocusTriggers(t *testing.T) {
389 if err := c.pointerMotion(ctx, ports.PointerMotion{X: 50, Y: 50}); err != nil {389 if err := c.pointerMotion(ctx, ports.PointerMotion{X: 50, Y: 50}); err != nil {
390 t.Fatal(err)390 t.Fatal(err)
391 }391 }
392- if err := c.pointerButton(ctx, ports.PointerButton{Button: btnLeft, Pressed: true}); err != nil {
393- t.Fatal(err)
394- }
395 if c.flash.pending != nil {392 if c.flash.pending != nil {
396- t.Fatal("pointer focus flashed")393+ t.Fatal("hover focus flashed")
397 }394 }
398 }395 }
399 396
@@ -774,3 +771,115 @@ func TestFlashFollowsInnerBorderAndRadius(t *testing.T) {
774 })771 })
775 }772 }
776 }773 }
774+
775+func TestFlashClickFocus(t *testing.T) {
776+ for _, test := range []struct {
777+ name string
778+ target WindowID
779+ enabled bool
780+ want bool
781+ }{
782+ {"different window", 1, true, true},
783+ {"already focused", 3, true, false},
784+ {"disabled", 1, false, false},
785+ } {
786+ t.Run(test.name, func(t *testing.T) {
787+ c, clock := flashCore(t)
788+ c.cfg.Focus.Flash = test.enabled
789+ indicatorScene(t, c)
790+ if c.keyboardFocus() != 3 {
791+ t.Fatal("setup: expected window 3 focused")
792+ }
793+ c.pointer = test.target
794+ if err := c.pointerButton(context.Background(), ports.PointerButton{Button: btnLeft, Pressed: true}); err != nil {
795+ t.Fatal(err)
796+ }
797+ if c.keyboardFocus() != test.target {
798+ t.Fatalf("focus = %d, want %d", c.keyboardFocus(), test.target)
799+ }
800+ if (c.flash.pending != nil) != test.want {
801+ t.Fatalf("flash request = %+v, want flash %v", c.flash.pending, test.want)
802+ }
803+ // Let the click's viewport transition settle before the flash.
804+ clock.now = clock.now.Add(time.Second)
805+ stepScenes(t, c, c.cur())
806+ if c.flashTick() != test.want {
807+ t.Fatalf("flash start: want %v", test.want)
808+ }
809+ if test.want && (c.flash.running.window != test.target || c.flash.running.level != flashWindow || c.flashFor(c.cur()) == nil) {
810+ t.Fatalf("wrong running flash: %+v", c.flash.running)
811+ }
812+ })
813+ }
814+}
815+
816+func TestFlashClickModalFocus(t *testing.T) {
817+ for _, unchanged := range []bool{false, true} {
818+ t.Run(map[bool]string{false: "changed", true: "unchanged"}[unchanged], func(t *testing.T) {
819+ c, _ := flashCore(t)
820+ w := c.cur().mon.Current()
821+ c.windows.mapped(ports.WindowMapped{ID: 4}, c.now())
822+ w.AddDialog(4, 1, 100, 80)
823+ w.SetModal(4, true)
824+ if !unchanged {
825+ w.Click(2)
826+ }
827+ indicatorScene(t, c)
828+ c.pointer = 1
829+ if err := c.pointerButton(context.Background(), ports.PointerButton{Button: btnLeft, Pressed: true}); err != nil {
830+ t.Fatal(err)
831+ }
832+ if c.keyboardFocus() != 4 {
833+ t.Fatalf("focus = %d, want modal 4", c.keyboardFocus())
834+ }
835+ if unchanged {
836+ if c.flash.pending != nil {
837+ t.Fatalf("unchanged modal flashed: %+v", c.flash.pending)
838+ }
839+ } else if c.flash.pending == nil || c.flash.pending.window != 4 {
840+ t.Fatalf("want modal flash, got %+v", c.flash.pending)
841+ }
842+ })
843+ }
844+}
845+
846+func TestFlashModifierClickDrag(t *testing.T) {
847+ for _, resize := range []bool{false, true} {
848+ for _, mode := range []string{"changed", "unchanged", "disabled"} {
849+ t.Run(map[bool]string{false: "move", true: "resize"}[resize]+"/"+mode, func(t *testing.T) {
850+ c, clock := flashCore(t)
851+ w := c.cur().mon.Current()
852+ w.Click(1)
853+ if resize {
854+ w.Apply(ActionToggleFloating)
855+ }
856+ if mode != "unchanged" {
857+ w.Click(3)
858+ }
859+ c.cfg.Focus.Flash = mode != "disabled"
860+ indicatorScene(t, c)
861+ c.pointer = 1
862+ c.mods = c.cmdMod
863+ button := uint32(btnLeft)
864+ if resize {
865+ button = btnRight
866+ }
867+ if err := c.pointerButton(context.Background(), ports.PointerButton{Button: button, Pressed: true}); err != nil {
868+ t.Fatal(err)
869+ }
870+ if c.drag == nil || c.drag.id != 1 || c.drag.resize != resize {
871+ t.Fatalf("drag not started: %+v", c.drag)
872+ }
873+ want := mode == "changed"
874+ if (c.flash.pending != nil) != want {
875+ t.Fatalf("flash = %+v, want %v", c.flash.pending, want)
876+ }
877+ clock.now = clock.now.Add(time.Second)
878+ stepScenes(t, c, c.cur())
879+ if c.flashTick() != want {
880+ t.Fatalf("flash start: want %v", want)
881+ }
882+ })
883+ }
884+ }
885+}
M internal/core/input.go +13 -2
@@ -136,12 +136,23 @@ func (c *Core) clickFocus(ctx context.Context, id WindowID) error {
136136 }
137137 now := c.now()
138138 before := c.snapshot(now)
139- w.Click(id)
140- c.focusScreen = c.screenIndex(s.name())
139+ c.focusClick(s, w, id)
141140 c.transition(before, now)
142141 return c.publish(ctx)
143142 }
144143
144+// focusClick raises the clicked window and focuses its output. Modal dialogs
145+// can take the parent's focus, so flash the resolved focus, not the click target.
146+func (c *Core) focusClick(s *screen, w *Workspace, id WindowID) {
147+ before, _ := c.cur().mon.Focused()
148+ w.Click(id)
149+ c.focusScreen = c.screenIndex(s.name())
150+ after, _ := c.cur().mon.Focused()
151+ if after != 0 && after != before {
152+ c.requestFlash(flashWindow, after)
153+ }
154+}
155+
145156 func (c *Core) pointerButton(ctx context.Context, v ports.PointerButton) error {
146157 // Buttons a drag took stay in c.buttons until released, so the
147158 // security gate admits their release; no client sees it.
@@ -136,12 +136,23 @@ func (c *Core) clickFocus(ctx context.Context, id WindowID) error {
136 }136 }
137 now := c.now()137 now := c.now()
138 before := c.snapshot(now)138 before := c.snapshot(now)
139- w.Click(id)139+ c.focusClick(s, w, id)
140- c.focusScreen = c.screenIndex(s.name())
141 c.transition(before, now)140 c.transition(before, now)
142 return c.publish(ctx)141 return c.publish(ctx)
143 }142 }
144 143
144+// focusClick raises the clicked window and focuses its output. Modal dialogs
145+// can take the parent's focus, so flash the resolved focus, not the click target.
146+func (c *Core) focusClick(s *screen, w *Workspace, id WindowID) {
147+ before, _ := c.cur().mon.Focused()
148+ w.Click(id)
149+ c.focusScreen = c.screenIndex(s.name())
150+ after, _ := c.cur().mon.Focused()
151+ if after != 0 && after != before {
152+ c.requestFlash(flashWindow, after)
153+ }
154+}
155+
145 func (c *Core) pointerButton(ctx context.Context, v ports.PointerButton) error {156 func (c *Core) pointerButton(ctx context.Context, v ports.PointerButton) error {
146 // Buttons a drag took stay in c.buttons until released, so the157 // Buttons a drag took stay in c.buttons until released, so the
147 // security gate admits their release; no client sees it.158 // security gate admits their release; no client sees it.

Comments0 open / 0

no comments yet.

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
l
merge request: the list of files (j k move, enter opens, esc back)
M
merge request: the merge menu; there 1–3 pick the mode, D deletes the branch, enter merges
R
merge request: mark the head read
esc
close a dialog, leave a field

Branches 25

Tags 10

↑↓ moveenter openesc close