Add a visible window switcher

Super+Tab already cycled windows, but nothing was drawn, so you chose blind and could only confirm the choice by arriving. A visible switcher is muscle memory for anyone arriving from macOS or GNOME, and it was the last item of roadmap phase 03 that did not need coordination. Ordered most-recently-used, not by creation, because that is what makes the gesture useful: one Tab returns to the window you just came from. Hyprland does not report an MRU order, so it is tracked from focus changes and keyed by address, which is the only property stable for a window's lifetime. The gesture needs three binds rather than two. Tab steps the selection, and the switch is committed on Super RELEASE -- the only way the compositor can say the gesture is over. That bind is on the bare modifier, so it fires on every Super release in the session; commit() returns immediately when nothing is open, which is what makes it affordable. A list of names rather than thumbnails: at a glance you are looking for "the other terminal", and a row of live previews is slower to read and far more expensive to draw than this gesture deserves. The interesting part is the bug. The overlay was built, mapped nothing, and logged absolutely nothing -- because it declared `required property var screen` while Variants supplies `modelData`. shell.qml has carried a comment warning about exactly this since the Bar hit it, and I read that comment earlier in the same session and still walked into it. A comment that does not stop the person who read it is an argument for a test, so per-screen-surface-contract now checks every per-screen delegate takes its screen from modelData. Verified it catches the exact mistake. Also fixes a regression from 8be3fc2: settings-pages-contract still required vitalsIntervalMs on Home, where it no longer is. That contract was pinning the split-across-two-pages arrangement the same commit fixed, and I pushed without running it. Claude-Session: https://claude.ai/code/session_01BRvzt4H8XXLPVH5MyYdk9L
2026-08-18 14:56:15 -04:00
parent 8be3fc2fdd
commit 1360a80f07
6 changed files with 364 additions and 4 deletions
@@ -195,9 +195,21 @@ bind(mod .. " + SHIFT + U", hl.dsp.window.resize({ x = 0, y = step, relative = t
 bind(mod .. " + SHIFT + P", hl.dsp.window.resize({ x = 0, y = -step, relative = true }), { repeating = true, description = "Shorter" })
 bind(mod .. " + SHIFT + N", hl.dsp.window.resize({ x = 0, y = -step, relative = true }), { repeating = true, description = "Shorter" })

-- Window cycling (GNOME: cycle-windows on SUPER+Tab).
-bind(mod .. " + Tab", hl.dsp.window.cycle_next({ next = true }), { description = "Next window" })
-bind(mod .. " + SHIFT + Tab", hl.dsp.window.cycle_next({ next = false }), { description = "Previous window" })
+-- Window cycling (GNOME: cycle-windows on SUPER+Tab), now with an overlay
+-- showing what you are choosing between.
+--
+-- The gesture needs three binds, not two. Tab steps the selection, and the
+-- switch is only COMMITTED when the modifier is released -- which is the sole
+-- way the compositor can tell the gesture is finished. That release bind is on
+-- the bare modifier, so it fires on EVERY Super release in the session; the
+-- handler returns immediately when no switch is open, which is why this is
+-- affordable.
+--
+-- The release bind carries no description on purpose: it is not a shortcut
+-- anyone would look up or rebind, and the Shortcuts page lists what it finds.
+bind(mod .. " + Tab", hl.dsp.exec_cmd(qs("switcher", "next")), { description = "Next window" })
+bind(mod .. " + SHIFT + Tab", hl.dsp.exec_cmd(qs("switcher", "previous")), { description = "Previous window" })
+bind(mod, hl.dsp.exec_cmd(qs("switcher", "commit")), { release = true, description = "Commit window switch" })
 -- Jump back to the previously focused window.
 bind(mod .. " + SHIFT + grave", hl.dsp.focus({ last = true }), { description = "Last window" })

@@ -0,0 +1,129 @@
+// The Alt-Tab overlay.
+//
+// Deliberately a list of names rather than thumbnails: at a glance you are
+// looking for "the other terminal", and a row of small live previews is both
+// slower to read and considerably more expensive to draw than the gesture
+// deserves. The dock already renders app identity this way, so the two agree.
+//
+// Only present while a switch is in progress -- there is nothing to keep alive
+// between gestures, and a hidden always-loaded overlay is a surface that can
+// go wrong while nobody is looking at it.
+
+import Quickshell
+import Quickshell.Wayland
+import QtQuick
+import qs.config
+import qs.services
+import qs.widgets
+
+Loader {
+    id: root
+
+    // Plain `modelData`, not `required property var screen`. Variants supplies
+    // modelData, and shell.qml's own comment warns about exactly this: declaring
+    // `required property var screen` means the screen never resolves, the window
+    // is constructed and silently never maps, and NOTHING is logged. Bar and
+    // Dock both take the screen this way.
+    property var modelData: null
+
+    active: WindowSwitcherState.open
+    asynchronous: false
+
+    sourceComponent: PanelWindow {
+        screen: root.modelData
+        // Overlay so it sits above the focused window it is describing.
+        WlrLayershell.layer: WlrLayer.Overlay
+        WlrLayershell.namespace: "qs-switcher"
+        // Nothing here is clickable: the gesture is driven entirely from the
+        // keyboard, and taking input would steal focus from the compositor
+        // mid-switch, which is the one thing that would break it.
+        WlrLayershell.keyboardFocus: WlrKeyboardFocus.None
+        exclusionMode: ExclusionMode.Ignore
+        color: "transparent"
+
+        anchors { top: true; bottom: true; left: true; right: true }
+
+        Rectangle {
+            anchors.centerIn: parent
+            width: Math.min(560, parent.width - 96)
+            implicitHeight: layout.implicitHeight + 24
+            radius: Theme.cardRadius
+            color: Theme.alpha(Theme.bgPopover, 0.97)
+            border.width: 1
+            border.color: Theme.alpha(Theme.fg, 0.09)
+
+            Column {
+                id: layout
+                anchors.left: parent.left
+                anchors.right: parent.right
+                anchors.verticalCenter: parent.verticalCenter
+                anchors.leftMargin: 12
+                anchors.rightMargin: 12
+                spacing: 2
+
+                Repeater {
+                    model: WindowSwitcherState.windows
+
+                    Rectangle {
+                        id: row
+
+                        required property var modelData
+                        required property int index
+
+                        readonly property bool current: row.index === WindowSwitcherState.index
+                        readonly property string appId: row.modelData?.wayland?.appId ?? ""
+                        readonly property var entry: DesktopEntries.heuristicLookup(row.appId)
+
+                        width: parent.width
+                        height: 44
+                        radius: 10
+                        border.width: 0
+                        color: row.current ? Theme.alpha(Theme.accent, 0.20) : "transparent"
+
+                        Image {
+                            id: icon
+                            anchors.left: parent.left
+                            anchors.leftMargin: 10
+                            anchors.verticalCenter: parent.verticalCenter
+                            width: 24
+                            height: 24
+                            sourceSize.width: 24
+                            sourceSize.height: 24
+                            source: row.entry?.icon ? Quickshell.iconPath(row.entry.icon, true) : ""
+                            visible: source !== ""
+                        }
+
+                        Text {
+                            anchors.left: icon.visible ? icon.right : parent.left
+                            anchors.leftMargin: icon.visible ? 12 : 14
+                            anchors.right: appName.left
+                            anchors.rightMargin: 12
+                            anchors.verticalCenter: parent.verticalCenter
+                            // A window with no title yet is still a window you
+                            // can switch to; naming it after its application is
+                            // better than an empty row.
+                            text: row.modelData?.title || row.entry?.name || row.appId
+                            elide: Text.ElideRight
+                            color: row.current ? Theme.fg : Theme.fgDim
+                            font.family: Theme.fontFamily
+                            font.pixelSize: Theme.fontSize
+                            font.weight: row.current ? Font.DemiBold : Font.Normal
+                        }
+
+                        Text {
+                            id: appName
+                            anchors.right: parent.right
+                            anchors.rightMargin: 12
+                            anchors.verticalCenter: parent.verticalCenter
+                            visible: (row.entry?.name ?? "") !== "" && row.entry.name !== row.modelData?.title
+                            text: row.entry?.name ?? ""
+                            color: Theme.fgMuted
+                            font.family: Theme.fontFamily
+                            font.pixelSize: Theme.fontSizeSmall
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
@@ -0,0 +1,134 @@
+pragma Singleton
+
+// Alt-Tab, with something on screen while you do it.
+//
+// Named WindowSwitcherState rather than WindowSwitcher: the overlay component
+// in modules/switcher already owns that name, and a singleton sharing it is
+// silently shadowed wherever both are imported -- the same failure that made an
+// earlier Locale singleton resolve to QML's built-in type instead.
+//
+// Super+Tab already cycled windows; nothing was drawn, so you were choosing
+// blind and could only confirm by arriving. This holds the selection while a
+// switch is in progress and lets the overlay render it.
+//
+// MOST-RECENTLY-USED ORDER
+//
+// The list is ordered by when each window last had focus, not by when it was
+// opened, because that is what makes the gesture useful: one Tab returns to the
+// window you just came from, which is the overwhelmingly common case. Creation
+// order would send you to whichever window happens to be first in Hyprland's
+// list, which is arbitrary from the user's point of view.
+//
+// Hyprland does not report an MRU order, so it is tracked here: every time a
+// toplevel becomes active it moves to the front. Addresses are used as the key
+// because they are stable for a window's lifetime, where titles and app ids are
+// not.
+//
+// HOW A SWITCH ENDS
+//
+// The compositor fires a bind on Super RELEASE, which commits. That is the only
+// way to know the gesture is over -- there is no "modifier released" signal
+// otherwise. It means close() runs on every Super release in the session, so it
+// must be cheap and a no-op when nothing is open.
+
+import Quickshell
+import Quickshell.Hyprland
+import QtQuick
+
+Singleton {
+    id: root
+
+    property bool open: false
+    property int index: 0
+
+    // Window addresses, most recently focused first.
+    property var recent: []
+
+    // The switch candidates, resolved fresh each time the gesture starts.
+    property var windows: []
+
+    readonly property var selected: (root.index >= 0 && root.index < root.windows.length)
+        ? root.windows[root.index] : null
+
+    // Ordered by the MRU list, with anything unseen appended in Hyprland's own
+    // order so a brand new window is still reachable.
+    function orderedWindows(): var {
+        const all = (Hyprland.toplevels?.values ?? []).filter(t => t && t.wayland && t.wayland.appId);
+        const byAddress = {};
+        for (const toplevel of all)
+            byAddress[String(toplevel.address)] = toplevel;
+
+        const ordered = [];
+        for (const address of root.recent) {
+            const match = byAddress[address];
+            if (match) {
+                ordered.push(match);
+                delete byAddress[address];
+            }
+        }
+        for (const toplevel of all)
+            if (byAddress[String(toplevel.address)])
+                ordered.push(toplevel);
+        return ordered;
+    }
+
+    // Starts the gesture if it is not already running, then steps. The first
+    // Tab lands on the PREVIOUS window rather than the current one, which is
+    // what every other implementation of this gesture does.
+    function step(forward: bool): void {
+        if (!root.open) {
+            root.windows = root.orderedWindows();
+            if (root.windows.length < 2)
+                return;
+            root.open = true;
+            root.index = forward ? 1 : root.windows.length - 1;
+            return;
+        }
+
+        if (root.windows.length === 0)
+            return;
+        const count = root.windows.length;
+        root.index = forward
+            ? (root.index + 1) % count
+            : (root.index - 1 + count) % count;
+    }
+
+    // Runs on every Super release in the session, so it does as little as
+    // possible when no switch is in progress.
+    function commit(): void {
+        if (!root.open)
+            return;
+        const target = root.selected;
+        root.open = false;
+        root.windows = [];
+        root.index = 0;
+        if (target && target.wayland)
+            target.wayland.activate();
+    }
+
+    function cancel(): void {
+        root.open = false;
+        root.windows = [];
+        root.index = 0;
+    }
+
+    // Focus changes maintain the MRU order. This runs whether or not a switch
+    // is in progress, because ordinary clicking between windows is most of how
+    // the order is established.
+    Connections {
+        target: Hyprland
+        function onActiveToplevelChanged(): void {
+            const active = Hyprland.activeToplevel;
+            if (!active || !active.address)
+                return;
+            const address = String(active.address);
+            const next = [address];
+            for (const existing of root.recent)
+                if (existing !== address)
+                    next.push(existing);
+            // Bounded: a session can accumulate a lot of closed addresses, and
+            // this list is only ever used to order what is currently open.
+            root.recent = next.slice(0, 64);
+        }
+    }
+}
@@ -28,6 +28,7 @@ import qs.config
 import qs.services
 import qs.modules.bar
 import qs.modules.dock
+import qs.modules.switcher
 import qs.modules.overview
 import qs.modules.quicksettings
 import qs.modules.notifications
@@ -68,6 +69,13 @@ ShellRoot {
        Dock {}
    }

+    // The Alt-Tab overlay. Present only while a switch is in progress; the
+    // Loader inside it keeps the window unbuilt the rest of the time.
+    Variants {
+        model: Quickshell.screens
+        WindowSwitcher {}
+    }
+
    Variants {
        model: Quickshell.screens
        SignalGlass {}
@@ -103,6 +111,18 @@ ShellRoot {
    // Every parameter AND the return type must be annotated, or Quickshell
    // silently declines to register the function — it will not warn you.

+    // Driven entirely from keybinds: Super+Tab steps, and a bind on Super
+    // RELEASE commits. `commit` therefore runs on every Super release in the
+    // session, so it returns immediately when no switch is open.
+    IpcHandler {
+        target: "switcher"
+
+        function next(): void { WindowSwitcherState.step(true); }
+        function previous(): void { WindowSwitcherState.step(false); }
+        function commit(): void { WindowSwitcherState.commit(); }
+        function cancel(): void { WindowSwitcherState.cancel(); }
+    }
+
    IpcHandler {
        target: "overview"
        function toggle(): void { ShellState.toggle("overview"); }
@@ -0,0 +1,60 @@
+#!/usr/bin/env bash
+
+# Anything instantiated per screen must take its screen from `modelData`.
+#
+# Variants supplies each delegate a `modelData` holding the screen. A component
+# that instead declares `required property var screen` is constructed, never
+# receives a screen, and its window silently never maps -- with nothing logged,
+# no error, and no visible failure beyond the surface simply not being there.
+#
+# shell.qml has warned about this in a comment since the Bar hit it. The comment
+# did not stop the window switcher hitting it again, which is the argument for a
+# test: the failure is invisible, so review does not catch it either.
+
+set -uo pipefail
+
+repo_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+shell_file="$repo_dir/config/dot/quickshell/shell.qml"
+modules="$repo_dir/config/dot/quickshell/modules"
+
+fail() {
+    printf 'per-screen surface contract: %s\n' "$1" >&2
+    exit 1
+}
+
+[[ -r "$shell_file" ]] || fail "cannot read shell.qml"
+
+# The component named inside each `Variants { model: Quickshell.screens ... }`.
+delegates="$(awk '
+    /Variants \{/ { inside = 1; next }
+    inside && /model: Quickshell.screens/ { armed = 1; next }
+    armed && /^[[:space:]]*[A-Z][A-Za-z]* *\{/ {
+        match($0, /[A-Z][A-Za-z]*/)
+        print substr($0, RSTART, RLENGTH)
+        armed = 0; inside = 0
+    }
+' "$shell_file" | sort -u)"
+
+[[ -n "$delegates" ]] || fail 'found no per-screen delegates -- this contract is not reading shell.qml correctly'
+
+checked=0
+while read -r name; do
+    [[ -n "$name" ]] || continue
+
+    file="$(find "$modules" -name "$name.qml" -print -quit 2>/dev/null)"
+    [[ -n "$file" ]] || fail "shell.qml instantiates $name per screen, but $name.qml was not found"
+
+    if grep -qE '^\s*required property var screen\b' "$file"; then
+        fail "$name declares 'required property var screen', but Variants supplies modelData -- the window is built and never maps, silently. Use 'property var modelData' and bind screen to it, as Bar and Dock do."
+    fi
+
+    grep -qE '^\s*property var modelData' "$file" \
+        || fail "$name is instantiated per screen but never declares 'property var modelData', so it cannot know which screen it is on"
+
+    grep -qE 'screen: (root\.)?modelData' "$file" \
+        || fail "$name declares modelData but never binds a screen to it"
+
+    checked=$((checked + 1))
+done <<<"$delegates"
+
+printf 'per-screen surface contract: PASS (%d per-screen surfaces)\n' "$checked"
@@ -50,7 +50,12 @@ PY
 home_page="$repo_dir/config/dot/quickshell/modules/settings/HomePage.qml"
 require_row "$home_page" ChoiceRow temperatureUnit
 require_row "$home_page" SliderRow weatherRefreshMinutes
-require_row "$home_page" SliderRow vitalsIntervalMs
+# vitalsIntervalMs moved to Appearance, beside the toggles it governs. It sat
+# on Home while showCpu/showMemory/showGpu sat on Appearance -- one concept
+# across two pages, which the ownership rule forbids and which made a search
+# for it open a page that did not contain it.
+appearance_page="$repo_dir/config/dot/quickshell/modules/settings/AppearancePage.qml"
+require_row "$appearance_page" SliderRow vitalsIntervalMs

 notifications_page="$repo_dir/config/dot/quickshell/modules/settings/NotificationsPage.qml"
 for setting in notificationTimeoutMs notificationTimeoutCriticalMs notificationHistoryLimit maxVisibleToasts; do