#2 fix(core): flash the output when an action moves focus to another screen

state: merged · e8a5ba6 · fast-forward
branch: fix/flash-cross-screen → main
opened: bnema · 4d ago
updated: 4d ago

An action that ends on another output (focus-column/window past the edge, move-column across outputs, focus-monitor, move-workspace-to-monitor) now flashes the destination output, even when it is empty. Before, edge crossings were treated as window/column flashes and needed a focused window, so an empty output showed nothing.

Same-output behaviour is unchanged (column/window flash; no flash on same-output workspace switch).

Checks: make check, make race, make mocks-check, staticcheck, TestFlash -race -count=10. Headless e2e on 2 outputs: 6/6 pass (empty and occupied destination, edge crossing and focus-monitor).

Commit	Subject	Author	Date	Signature	Status
e8a5ba6	fix(core): flash the output when an action moves focus to another screen	brice	4d ago	GPG Verified

Checks 2

3 files · +82 -7

2/2

M docs/config.md +3 -1


@@ -78,7 +78,9 @@ See [examples/config](../examples/config) for every key with its default.
78	78		`<output>` is a connector such as `DP-2`, or a monitor key (`make model serial`).
79	79
80	80		The focus flash marks the window, column or output an action changes with a brief
81		-	inner border. Workspace switches and mouse focus do not trigger a flash.
	81	+	inner border. When a flash-triggering action crosses screens, it marks the whole
	82	+	destination output, even if that output has no windows. Same-screen workspace
	83	+	switches and mouse focus do not trigger a flash.
82	84		It runs when `animations = on`.
83	85
84	86		## Window rules


@@ -78,7 +78,9 @@ See [examples/config](../examples/config) for every key with its default.
78		`<output>` is a connector such as `DP-2`, or a monitor key (`make model serial`).	78		`<output>` is a connector such as `DP-2`, or a monitor key (`make model serial`).
79			79
80		The focus flash marks the window, column or output an action changes with a brief	80		The focus flash marks the window, column or output an action changes with a brief
81	-	inner border. Workspace switches and mouse focus do not trigger a flash.	81	+	inner border. When a flash-triggering action crosses screens, it marks the whole
			82	+	destination output, even if that output has no windows. Same-screen workspace
			83	+	switches and mouse focus do not trigger a flash.
82		It runs when `animations = on`.	84		It runs when `animations = on`.
83			85
84		## Window rules	86		## Window rules

M internal/core/action.go +7 -6


@@ -248,6 +248,11 @@ func (c *Core) applyAction(a Action) Effect {
248	248		}
249	249		return
250	250		}
	251	+	// Crossing outputs marks the destination itself, even if it is empty.
	252	+	if sc != beforeScreen {
	253	+	c.requestFlash(flashMonitor, 0)
	254	+	return
	255	+	}
251	256		level := flashWindow
252	257		switch a {
253	258		case ActionFocusWindowUp, ActionFocusWindowDown, ActionMoveWindowUp, ActionMoveWindowDown,

@@ -264,19 +269,15 @@ func (c *Core) applyAction(a Action) Effect {
264	269		level = flashColumn
265	270		case ActionFocusMonitorLeft, ActionFocusMonitorRight, ActionFocusMonitorUp, ActionFocusMonitorDown,
266	271		ActionMoveWorkspaceLeft, ActionMoveWorkspaceRight, ActionMoveWorkspaceToMonitorUp, ActionMoveWorkspaceToMonitorDown:
267		-	if sc == beforeScreen {
268		-	return
269		-	}
270		-	c.requestFlash(flashMonitor, 0)
271	272		return
272	273		default:
273	274		return
274	275		}
275		-	if sc == beforeScreen && sc.mon.Current() != beforeWS {
	276	+	if sc.mon.Current() != beforeWS {
276	277		return
277	278		}
278	279		after := sc.mon.Layout()
279		-	changed := sc != beforeScreen \|\| sc.mon.Current() != beforeWS \|\| id != beforeID \|\| !slices.EqualFunc(before, after, func(a, b Placement) bool {
	280	+	changed := id != beforeID \|\| !slices.EqualFunc(before, after, func(a, b Placement) bool {
280	281		return a.ID == b.ID && a.Rect == b.Rect && a.Focused == b.Focused && a.Hidden == b.Hidden && a.Floating == b.Floating
281	282		})
282	283		if changed && id != 0 {

 			}
 			return
 		}
+		// Crossing outputs marks the destination itself, even if it is empty.
+		if sc != beforeScreen {
+			c.requestFlash(flashMonitor, 0)
+			return
+		}
 		level := flashWindow
 		switch a {
 		case ActionFocusWindowUp, ActionFocusWindowDown, ActionMoveWindowUp, ActionMoveWindowDown,
 			level = flashColumn
 		case ActionFocusMonitorLeft, ActionFocusMonitorRight, ActionFocusMonitorUp, ActionFocusMonitorDown,
 			ActionMoveWorkspaceLeft, ActionMoveWorkspaceRight, ActionMoveWorkspaceToMonitorUp, ActionMoveWorkspaceToMonitorDown:
-			if sc == beforeScreen {
-				return
-			}
-			c.requestFlash(flashMonitor, 0)
 			return
 		default:
 			return
 		}
-		if sc == beforeScreen && sc.mon.Current() != beforeWS {
+		if sc.mon.Current() != beforeWS {
 			return
 		}
 		after := sc.mon.Layout()
-		changed := sc != beforeScreen || sc.mon.Current() != beforeWS || id != beforeID || !slices.EqualFunc(before, after, func(a, b Placement) bool {
+		changed := id != beforeID || !slices.EqualFunc(before, after, func(a, b Placement) bool {
 			return a.ID == b.ID && a.Rect == b.Rect && a.Focused == b.Focused && a.Hidden == b.Hidden && a.Floating == b.Floating
 		})
 		if changed && id != 0 {

M internal/core/flash_test.go +72 -0


@@ -598,3 +598,75 @@ func TestFlashUnchangedSampleIsReused(t *testing.T) {
598	598		t.Fatal("changed sample mutated prior scene")
599	599		}
600	600		}
	601	+
	602	+	func TestFlashCrossScreenActionsMarkDestinationOutput(t *testing.T) {
	603	+	for _, tc := range []struct {
	604	+	name string
	605	+	action Action
	606	+	occupied bool
	607	+	}{
	608	+	{"column focus to empty", ActionFocusColumnRight, false},
	609	+	{"column focus to occupied", ActionFocusColumnRight, true},
	610	+	{"monitor focus to empty", ActionFocusMonitorRight, false},
	611	+	{"column move to empty", ActionMoveColumnRight, false},
	612	+	{"column move to occupied", ActionMoveColumnRight, true},
	613	+	} {
	614	+	t.Run(tc.name, func(t *testing.T) {
	615	+	c, clock := flashCore(t)
	616	+	a := c.cur()
	617	+	c.addScreen(ports.OutputInfo{Name: "B", Width: 400, Height: 300})
	618	+	b := c.screens[1]
	619	+	if tc.occupied {
	620	+	c.windows.mapped(ports.WindowMapped{ID: 4}, clock.now)
	621	+	b.mon.AddWindow(4)
	622	+	}
	623	+	c.applyAction(tc.action)
	624	+	if c.cur() != b {
	625	+	t.Fatal("action did not cross screens")
	626	+	}
	627	+	if c.flash.pending == nil \|\| *c.flash.pending != (flashTarget{level: flashMonitor, output: "B"}) {
	628	+	t.Fatalf("destination request %+v", c.flash.pending)
	629	+	}
	630	+	clock.now = clock.now.Add(flashSettle)
	631	+	if !c.flashTick() {
	632	+	t.Fatal("destination flash did not start")
	633	+	}
	634	+	scenes := stepScenes(t, c, b)
	635	+	if scenes["A"].Flash != nil {
	636	+	t.Fatal("source output flashed")
	637	+	}
	638	+	f := scenes["B"].Flash
	639	+	if f == nil \|\| f.Rect != (Rect{W: 400, H: 300}) \|\| !scenes["B"].Animating {
	640	+	t.Fatalf("destination scene flash %+v", f)
	641	+	}
	642	+	if !tc.occupied && tc.action != ActionMoveColumnRight && len(scenes["B"].Windows) != 0 {
	643	+	t.Fatal("setup: destination is not empty")
	644	+	}
	645	+	clock.now = clock.now.Add(c.cfg.Focus.FlashDuration)
	646	+	scenes = stepScenes(t, c, b)
	647	+	if scenes["B"].Flash != nil \|\| c.flashing(a) \|\| c.animating() {
	648	+	t.Fatal("destination flash did not end")
	649	+	}
	650	+	})
	651	+	}
	652	+	}
	653	+
	654	+	func TestFlashSameScreenColumnFocusKeepsColumnLevel(t *testing.T) {
	655	+	c, clock := flashCore(t)
	656	+	sc := c.cur()
	657	+	c.cur().mon.Current().FocusID(1)
	658	+	c.applyAction(ActionFocusColumnRight)
	659	+	if c.cur() != sc \|\| c.flash.pending == nil \|\| c.flash.pending.level != flashColumn {
	660	+	t.Fatalf("same-screen request %+v", c.flash.pending)
	661	+	}
	662	+	clock.now = clock.now.Add(flashSettle)
	663	+	if !c.flashTick() {
	664	+	t.Fatal("column flash did not start")
	665	+	}
	666	+	s := indicatorScene(t, c)
	667	+	for _, w := range s.Windows {
	668	+	if w.ID == c.flash.running.window && s.Flash.Rect != w.Rect {
	669	+	t.Fatalf("column rect %+v want %+v", s.Flash.Rect, w.Rect)
	670	+	}
	671	+	}
	672	+	}


@@ -598,3 +598,75 @@ func TestFlashUnchangedSampleIsReused(t *testing.T) {
598	t.Fatal("changed sample mutated prior scene")	598		t.Fatal("changed sample mutated prior scene")
599	}	599		}
600	}	600		}
		601	+
		602	+	func TestFlashCrossScreenActionsMarkDestinationOutput(t *testing.T) {
		603	+	for _, tc := range []struct {
		604	+	name string
		605	+	action Action
		606	+	occupied bool
		607	+	}{
		608	+	{"column focus to empty", ActionFocusColumnRight, false},
		609	+	{"column focus to occupied", ActionFocusColumnRight, true},
		610	+	{"monitor focus to empty", ActionFocusMonitorRight, false},
		611	+	{"column move to empty", ActionMoveColumnRight, false},
		612	+	{"column move to occupied", ActionMoveColumnRight, true},
		613	+	} {
		614	+	t.Run(tc.name, func(t *testing.T) {
		615	+	c, clock := flashCore(t)
		616	+	a := c.cur()
		617	+	c.addScreen(ports.OutputInfo{Name: "B", Width: 400, Height: 300})
		618	+	b := c.screens[1]
		619	+	if tc.occupied {
		620	+	c.windows.mapped(ports.WindowMapped{ID: 4}, clock.now)
		621	+	b.mon.AddWindow(4)
		622	+	}
		623	+	c.applyAction(tc.action)
		624	+	if c.cur() != b {
		625	+	t.Fatal("action did not cross screens")
		626	+	}
		627	+	if c.flash.pending == nil \|\| *c.flash.pending != (flashTarget{level: flashMonitor, output: "B"}) {
		628	+	t.Fatalf("destination request %+v", c.flash.pending)
		629	+	}
		630	+	clock.now = clock.now.Add(flashSettle)
		631	+	if !c.flashTick() {
		632	+	t.Fatal("destination flash did not start")
		633	+	}
		634	+	scenes := stepScenes(t, c, b)
		635	+	if scenes["A"].Flash != nil {
		636	+	t.Fatal("source output flashed")
		637	+	}
		638	+	f := scenes["B"].Flash
		639	+	if f == nil \|\| f.Rect != (Rect{W: 400, H: 300}) \|\| !scenes["B"].Animating {
		640	+	t.Fatalf("destination scene flash %+v", f)
		641	+	}
		642	+	if !tc.occupied && tc.action != ActionMoveColumnRight && len(scenes["B"].Windows) != 0 {
		643	+	t.Fatal("setup: destination is not empty")
		644	+	}
		645	+	clock.now = clock.now.Add(c.cfg.Focus.FlashDuration)
		646	+	scenes = stepScenes(t, c, b)
		647	+	if scenes["B"].Flash != nil \|\| c.flashing(a) \|\| c.animating() {
		648	+	t.Fatal("destination flash did not end")
		649	+	}
		650	+	})
		651	+	}
		652	+	}
		653	+
		654	+	func TestFlashSameScreenColumnFocusKeepsColumnLevel(t *testing.T) {
		655	+	c, clock := flashCore(t)
		656	+	sc := c.cur()
		657	+	c.cur().mon.Current().FocusID(1)
		658	+	c.applyAction(ActionFocusColumnRight)
		659	+	if c.cur() != sc \|\| c.flash.pending == nil \|\| c.flash.pending.level != flashColumn {
		660	+	t.Fatalf("same-screen request %+v", c.flash.pending)
		661	+	}
		662	+	clock.now = clock.now.Add(flashSettle)
		663	+	if !c.flashTick() {
		664	+	t.Fatal("column flash did not start")
		665	+	}
		666	+	s := indicatorScene(t, c)
		667	+	for _, w := range s.Windows {
		668	+	if w.ID == c.flash.running.window && s.Flash.Rect != w.Rect {
		669	+	t.Fatalf("column rect %+v want %+v", s.Flash.Rect, w.Rect)
		670	+	}
		671	+	}
		672	+	}

Comments0 open / 0

no comments yet.

Merge settings

Comments0 open / 0