From 68b3f2bdd09c38bc571868beef8c36985acd004e Mon Sep 17 00:00:00 2001 From: The_miro Date: Wed, 5 Aug 2026 23:15:34 +0200 Subject: [PATCH] fix(astro-menu): stop the taskbar poll resetting the window list's scroll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workspace/window panel couldn't be scrolled: it snapped back to the top about once a second. Nothing to do with the hologram overlay (a DrawingArea with can_target=False that only ever queue_draw()s itself) — it was the 1 Hz refresh that start_polling() runs while the menu is open. Both of its callbacks called _rebuild_panel(), which did _clear(self._panel) and rebuilt every widget, including a fresh Gtk.ScrolledWindow for the list. Emptying scrolled content collapses the scroll adjustment's upper bound, so its value is clamped back to 0 and the viewport jumps to the top. The panel scaffolding (header, layout stack + switcher, list scroller) is now built once in _build_panel_scaffold() and refreshed in place by _update_panel: * The layout tabs are the same for every workspace — only the selection is per-workspace, and _sync_layout_controls() already refreshes that — so they no longer get torn down either. * _update_window_list() compares the sorted window addresses with the last set. Unchanged: row contents (title, ws N, icon) are updated in place, so a retitled window can't move the viewport. Changed: only the rows are rebuilt, and both scrollers' offsets are saved and restored on idle, once the new rows are allocated and the adjustment's upper is no longer stale. * _rebuild_strip() gets the same signature guard, so the poll stops resetting the compact strip's horizontal scroll and no longer tears down a grouped app's instance popover while it's open. It keys on addresses only: strip titles are tooltip/popover-only and refresh on the next real change. Applied identically to hyprlua and hyprdrive. Co-Authored-By: Claude Opus 5 --- .../hyprdrive/astro-menu/ui/taskbar.py | 127 ++++++++++++++---- desktopenvs/hyprlua/astal-menu/ui/taskbar.py | 127 ++++++++++++++---- 2 files changed, 202 insertions(+), 52 deletions(-) diff --git a/desktopenvs/hyprdrive/astro-menu/ui/taskbar.py b/desktopenvs/hyprdrive/astro-menu/ui/taskbar.py index bb0e3e2..9275fbd 100644 --- a/desktopenvs/hyprdrive/astro-menu/ui/taskbar.py +++ b/desktopenvs/hyprdrive/astro-menu/ui/taskbar.py @@ -55,6 +55,11 @@ class Taskbar(Gtk.Box): self._dir_dds = {} self._fit_sws = {} self._poll_id: int | None = None + # Content signatures, so a poll that found nothing new rebuilds nothing + # (see _build_panel_scaffold for why that matters). None = never built. + self._strip_sig: list | None = None + self._list_order: list | None = None + self._rows: dict = {} # window address -> row widgets, for in-place updates # The workspace/window panel. It is NOT appended here: the menu window mounts # `panel_widget` into the quad region so that expanding COLLAPSES this strip's @@ -64,6 +69,7 @@ class Taskbar(Gtk.Box): self._panel_scroll = Gtk.ScrolledWindow( hscrollbar_policy=Gtk.PolicyType.NEVER, vexpand=True) self._panel_scroll.set_child(self._panel) + self._build_panel_scaffold() header = Gtk.CenterBox() title = Gtk.Label(label="Open windows", xalign=0.0) @@ -166,21 +172,31 @@ class Taskbar(Gtk.Box): def _on_ws(self, ok: bool, data) -> None: if ok and isinstance(data, dict): self._active_ws = data.get("id") - self._rebuild_panel() + self._update_panel() def _on_clients(self, ok: bool, data) -> None: self._clients = data if ok and isinstance(data, list) else [] self._rebuild_strip() - self._rebuild_panel() + self._update_panel() # -- compact strip ----------------------------------------------------- def _rebuild_strip(self) -> None: - self._clear(self._row) groups: dict[str, list] = {} for w in self._clients: if not w.get("mapped", True) or not w.get("class"): continue groups.setdefault(w["class"], []).append(w) + # Same reasoning as the panel list: rebuild only when the strip's contents + # actually change, so a poll can't reset its horizontal scroll — or tear down + # a grouped app's instance popover while it is open. Keyed on addresses only: + # a retitled window doesn't change anything visible here (titles appear in the + # tooltip/popover, refreshed on the next real change). + sig = [(cls, tuple(w.get("address") for w in wins)) + for cls, wins in sorted(groups.items())] + if sig == self._strip_sig: + return + self._strip_sig = sig + self._clear(self._row) if not groups: self._row.append(Gtk.Label(label="No open windows")) return @@ -219,16 +235,22 @@ class Taskbar(Gtk.Box): return btn # -- pop-open panel: layout controls + window list --------------------- - def _rebuild_panel(self) -> None: - self._clear(self._panel) - + # The panel scaffolding is built ONCE, here, and every later refresh updates it in + # place. It used to be rebuilt from scratch on each refresh (_clear(self._panel) + + # fresh widgets, including a fresh ScrolledWindow for the window list) — and since + # start_polling() refreshes once a second while the menu is open, that made the + # list impossible to scroll: emptying the scrolled content collapses the scroll + # adjustment's upper bound, so its value is clamped back to 0 and the viewport + # snapped to the top roughly once a second. + def _build_panel_scaffold(self) -> None: # tabbed layout selector for the active workspace: a tab per layout, and a # per-layout options page underneath that switches with the tab. Selecting a - # tab applies that layout to the currently focused workspace. - ws = self._active_ws - hdr = Gtk.Label(label=f"Workspace {ws if ws is not None else '?'} layout", xalign=0.0) - hdr.add_css_class("section-title") - self._panel.append(hdr) + # tab applies that layout to the currently focused workspace. The tabs are the + # same for every workspace — only the *selection* is per-workspace, and that is + # what _sync_layout_controls() refreshes — so this is built once too. + self._ws_hdr = Gtk.Label(label="Workspace ? layout", xalign=0.0) + self._ws_hdr.add_css_class("section-title") + self._panel.append(self._ws_hdr) self._dir_dds = {} # layout name -> (Gtk.DropDown, [dir values]) self._fit_sws = {} # layout name -> Gtk.Switch @@ -240,23 +262,72 @@ class Taskbar(Gtk.Box): self._panel.append(switcher) self._panel.append(self._layout_stack) self._layout_stack.connect("notify::visible-child-name", self._on_tab_switch) - self._sync_layout_controls() self._panel.append(Gtk.Separator()) # per-window rows: [focus/jump] [pull here] + self._listing = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=4) + self._list_scroll = Gtk.ScrolledWindow( + vexpand=True, hscrollbar_policy=Gtk.PolicyType.NEVER) + self._list_scroll.set_child(self._listing) + self._panel.append(self._list_scroll) + + def _update_panel(self) -> None: + ws = self._active_ws + self._ws_hdr.set_label(f"Workspace {ws if ws is not None else '?'} layout") + self._sync_layout_controls() + if getattr(self, "_cols_lbl", None) is not None: + self._cols_lbl.set_label(str(self._read_cols())) + self._update_window_list() + + def _update_window_list(self) -> None: wins = [w for w in self._clients if w.get("mapped", True) and w.get("class")] wins.sort(key=lambda w: (w.get("workspace", {}).get("id", 0), (w.get("title") or w.get("class") or "").lower())) - if not wins: - self._panel.append(Gtk.Label(label="No open windows", xalign=0.0)) + order = [w.get("address") for w in wins] + if order == self._list_order: + # Same windows in the same order: refresh the row contents in place, so a + # retitled window (browser tab switch, shell cwd) costs no rebuild and + # therefore cannot move the scroll position. + for w in wins: + refs = self._rows.get(w.get("address")) + if refs is not None: + self._set_row_content(refs, w) return - listing = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=4) - scroller = Gtk.ScrolledWindow(vexpand=True, hscrollbar_policy=Gtk.PolicyType.NEVER) - scroller.set_child(listing) + + # The list really did change shape. Rebuild just the rows (not the scroller) + # and put the viewport back where the user left it, once the new rows have been + # allocated — until then the adjustment's upper bound is still stale. Both + # scrollers are saved: which one actually scrolls depends on how much height + # the mounted panel gets, and restoring an already-zero offset is a no-op. + saved = [(sw.get_vadjustment(), sw.get_vadjustment().get_value()) + for sw in (self._panel_scroll, self._list_scroll)] + self._list_order = order + self._rows = {} + self._clear(self._listing) + if not wins: + self._listing.append(Gtk.Label(label="No open windows", xalign=0.0)) for w in wins: - listing.append(self._window_row(w)) - self._panel.append(scroller) + self._listing.append(self._window_row(w)) + GLib.idle_add(self._restore_offsets, saved) + + @staticmethod + def _restore_offsets(saved) -> bool: + for adj, value in saved: + adj.set_value(min(value, max(0.0, adj.get_upper() - adj.get_page_size()))) + return False + + def _set_row_content(self, refs, w) -> None: + icon, title_lbl, ws_lbl = refs + title = w.get("title") or w.get("class") or "?" + if title_lbl.get_label() != title: + title_lbl.set_label(title) + ws_text = f"ws {w.get('workspace', {}).get('id')}" + if ws_lbl.get_label() != ws_text: + ws_lbl.set_label(ws_text) + icon_name = self._icon_for(w.get("class", "")) + if icon.get_icon_name() != icon_name: + icon.set_from_icon_name(icon_name) def _window_row(self, w) -> Gtk.Widget: row = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=8) @@ -271,8 +342,12 @@ class Taskbar(Gtk.Box): lbl.append(icon) wl = Gtk.Label(label=f"{title}", xalign=0.0, hexpand=True, ellipsize=3, max_width_chars=32) lbl.append(wl) - lbl.append(Gtk.Label(label=f"ws {wsid}", xalign=1.0)) + ws_lbl = Gtk.Label(label=f"ws {wsid}", xalign=1.0) + lbl.append(ws_lbl) name.set_child(lbl) + # Keep the mutable bits reachable so a refresh can update this row instead of + # replacing it (the address is the row's identity and never changes). + self._rows[w.get("address")] = (icon, wl, ws_lbl) name.set_tooltip_text("Jump to window") name.connect("clicked", lambda *_a: self._focus(w)) row.append(name) @@ -367,21 +442,21 @@ class Taskbar(Gtk.Box): # Reflect the current per-ws layout / direction / focus_fit in the tabs+options. # # This fires 3 concurrent hyprctl subprocess calls with no guaranteed resolution - # order, and refresh() can trigger _rebuild_panel() (hence this) twice in a row - # (once from the clients query, once from the activeworkspace one) — each - # rebuilding a fresh _layout_stack/_dir_dds/_fit_sws. Two failure modes existed: + # order, and refresh() can trigger _update_panel() (hence this) twice in a row + # (once from the clients query, once from the activeworkspace one), so several + # syncs can be in flight at once. Two failure modes existed: # 1. The 3 callbacks used to each toggle the shared `self._syncing` flag # independently (True at start, False at end). Whichever resolved *first* # dropped the guard while the others — in particular the tab-selection one, # which programmatically flips the visible stack child — were still in # flight, letting `_on_tab_switch`/`_on_opt_change` misfire as if the user # had clicked and re-apply stale data via `layouts.set(...)`. - # 2. A second `_sync_layout_controls()` call (from the redundant rebuild) could + # 2. A second `_sync_layout_controls()` call (from the redundant refresh) could # land while the first's callbacks were still pending, so a stale callback # from sync #1 could decrement sync #2's pending-count or apply sync #1's - # (possibly wrong-workspace) data into sync #2's freshly built widgets. + # (possibly wrong-workspace) data into sync #2's widgets. # A generation token invalidates any sync superseded by a newer one outright — - # if the user switches workspaces (or a redundant rebuild fires) mid-flight, only + # if the user switches workspaces (or a redundant refresh fires) mid-flight, only # the latest call's callbacks are allowed to touch anything, and self._syncing # only lifts once *that* generation's 3 calls have all resolved. self._sync_generation = getattr(self, "_sync_generation", 0) + 1 diff --git a/desktopenvs/hyprlua/astal-menu/ui/taskbar.py b/desktopenvs/hyprlua/astal-menu/ui/taskbar.py index c8558ea..e9bf442 100644 --- a/desktopenvs/hyprlua/astal-menu/ui/taskbar.py +++ b/desktopenvs/hyprlua/astal-menu/ui/taskbar.py @@ -55,6 +55,11 @@ class Taskbar(Gtk.Box): self._dir_dds = {} self._fit_sws = {} self._poll_id: int | None = None + # Content signatures, so a poll that found nothing new rebuilds nothing + # (see _build_panel_scaffold for why that matters). None = never built. + self._strip_sig: list | None = None + self._list_order: list | None = None + self._rows: dict = {} # window address -> row widgets, for in-place updates # The workspace/window panel. It is NOT appended here: the menu window mounts # `panel_widget` into the quad region so that expanding COLLAPSES this strip's @@ -64,6 +69,7 @@ class Taskbar(Gtk.Box): self._panel_scroll = Gtk.ScrolledWindow( hscrollbar_policy=Gtk.PolicyType.NEVER, vexpand=True) self._panel_scroll.set_child(self._panel) + self._build_panel_scaffold() header = Gtk.CenterBox() title = Gtk.Label(label="Open windows", xalign=0.0) @@ -166,21 +172,31 @@ class Taskbar(Gtk.Box): def _on_ws(self, ok: bool, data) -> None: if ok and isinstance(data, dict): self._active_ws = data.get("id") - self._rebuild_panel() + self._update_panel() def _on_clients(self, ok: bool, data) -> None: self._clients = data if ok and isinstance(data, list) else [] self._rebuild_strip() - self._rebuild_panel() + self._update_panel() # -- compact strip ----------------------------------------------------- def _rebuild_strip(self) -> None: - self._clear(self._row) groups: dict[str, list] = {} for w in self._clients: if not w.get("mapped", True) or not w.get("class"): continue groups.setdefault(w["class"], []).append(w) + # Same reasoning as the panel list: rebuild only when the strip's contents + # actually change, so a poll can't reset its horizontal scroll — or tear down + # a grouped app's instance popover while it is open. Keyed on addresses only: + # a retitled window doesn't change anything visible here (titles appear in the + # tooltip/popover, refreshed on the next real change). + sig = [(cls, tuple(w.get("address") for w in wins)) + for cls, wins in sorted(groups.items())] + if sig == self._strip_sig: + return + self._strip_sig = sig + self._clear(self._row) if not groups: self._row.append(Gtk.Label(label="No open windows")) return @@ -219,16 +235,22 @@ class Taskbar(Gtk.Box): return btn # -- pop-open panel: layout controls + window list --------------------- - def _rebuild_panel(self) -> None: - self._clear(self._panel) - + # The panel scaffolding is built ONCE, here, and every later refresh updates it in + # place. It used to be rebuilt from scratch on each refresh (_clear(self._panel) + + # fresh widgets, including a fresh ScrolledWindow for the window list) — and since + # start_polling() refreshes once a second while the menu is open, that made the + # list impossible to scroll: emptying the scrolled content collapses the scroll + # adjustment's upper bound, so its value is clamped back to 0 and the viewport + # snapped to the top roughly once a second. + def _build_panel_scaffold(self) -> None: # tabbed layout selector for the active workspace: a tab per layout, and a # per-layout options page underneath that switches with the tab. Selecting a - # tab applies that layout to the currently focused workspace. - ws = self._active_ws - hdr = Gtk.Label(label=f"Workspace {ws if ws is not None else '?'} layout", xalign=0.0) - hdr.add_css_class("section-title") - self._panel.append(hdr) + # tab applies that layout to the currently focused workspace. The tabs are the + # same for every workspace — only the *selection* is per-workspace, and that is + # what _sync_layout_controls() refreshes — so this is built once too. + self._ws_hdr = Gtk.Label(label="Workspace ? layout", xalign=0.0) + self._ws_hdr.add_css_class("section-title") + self._panel.append(self._ws_hdr) self._dir_dds = {} # layout name -> (Gtk.DropDown, [dir values]) self._fit_sws = {} # layout name -> Gtk.Switch @@ -240,23 +262,72 @@ class Taskbar(Gtk.Box): self._panel.append(switcher) self._panel.append(self._layout_stack) self._layout_stack.connect("notify::visible-child-name", self._on_tab_switch) - self._sync_layout_controls() self._panel.append(Gtk.Separator()) # per-window rows: [focus/jump] [pull here] + self._listing = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=4) + self._list_scroll = Gtk.ScrolledWindow( + vexpand=True, hscrollbar_policy=Gtk.PolicyType.NEVER) + self._list_scroll.set_child(self._listing) + self._panel.append(self._list_scroll) + + def _update_panel(self) -> None: + ws = self._active_ws + self._ws_hdr.set_label(f"Workspace {ws if ws is not None else '?'} layout") + self._sync_layout_controls() + if getattr(self, "_cols_lbl", None) is not None: + self._cols_lbl.set_label(str(self._read_cols())) + self._update_window_list() + + def _update_window_list(self) -> None: wins = [w for w in self._clients if w.get("mapped", True) and w.get("class")] wins.sort(key=lambda w: (w.get("workspace", {}).get("id", 0), (w.get("title") or w.get("class") or "").lower())) - if not wins: - self._panel.append(Gtk.Label(label="No open windows", xalign=0.0)) + order = [w.get("address") for w in wins] + if order == self._list_order: + # Same windows in the same order: refresh the row contents in place, so a + # retitled window (browser tab switch, shell cwd) costs no rebuild and + # therefore cannot move the scroll position. + for w in wins: + refs = self._rows.get(w.get("address")) + if refs is not None: + self._set_row_content(refs, w) return - listing = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=4) - scroller = Gtk.ScrolledWindow(vexpand=True, hscrollbar_policy=Gtk.PolicyType.NEVER) - scroller.set_child(listing) + + # The list really did change shape. Rebuild just the rows (not the scroller) + # and put the viewport back where the user left it, once the new rows have been + # allocated — until then the adjustment's upper bound is still stale. Both + # scrollers are saved: which one actually scrolls depends on how much height + # the mounted panel gets, and restoring an already-zero offset is a no-op. + saved = [(sw.get_vadjustment(), sw.get_vadjustment().get_value()) + for sw in (self._panel_scroll, self._list_scroll)] + self._list_order = order + self._rows = {} + self._clear(self._listing) + if not wins: + self._listing.append(Gtk.Label(label="No open windows", xalign=0.0)) for w in wins: - listing.append(self._window_row(w)) - self._panel.append(scroller) + self._listing.append(self._window_row(w)) + GLib.idle_add(self._restore_offsets, saved) + + @staticmethod + def _restore_offsets(saved) -> bool: + for adj, value in saved: + adj.set_value(min(value, max(0.0, adj.get_upper() - adj.get_page_size()))) + return False + + def _set_row_content(self, refs, w) -> None: + icon, title_lbl, ws_lbl = refs + title = w.get("title") or w.get("class") or "?" + if title_lbl.get_label() != title: + title_lbl.set_label(title) + ws_text = f"ws {w.get('workspace', {}).get('id')}" + if ws_lbl.get_label() != ws_text: + ws_lbl.set_label(ws_text) + icon_name = self._icon_for(w.get("class", "")) + if icon.get_icon_name() != icon_name: + icon.set_from_icon_name(icon_name) def _window_row(self, w) -> Gtk.Widget: row = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL, spacing=8) @@ -271,8 +342,12 @@ class Taskbar(Gtk.Box): lbl.append(icon) wl = Gtk.Label(label=f"{title}", xalign=0.0, hexpand=True, ellipsize=3, max_width_chars=32) lbl.append(wl) - lbl.append(Gtk.Label(label=f"ws {wsid}", xalign=1.0)) + ws_lbl = Gtk.Label(label=f"ws {wsid}", xalign=1.0) + lbl.append(ws_lbl) name.set_child(lbl) + # Keep the mutable bits reachable so a refresh can update this row instead of + # replacing it (the address is the row's identity and never changes). + self._rows[w.get("address")] = (icon, wl, ws_lbl) name.set_tooltip_text("Jump to window") name.connect("clicked", lambda *_a: self._focus(w)) row.append(name) @@ -367,21 +442,21 @@ class Taskbar(Gtk.Box): # Reflect the current per-ws layout / direction / focus_fit in the tabs+options. # # This fires 3 concurrent hyprctl subprocess calls with no guaranteed resolution - # order, and refresh() can trigger _rebuild_panel() (hence this) twice in a row - # (once from the clients query, once from the activeworkspace one) — each - # rebuilding a fresh _layout_stack/_dir_dds/_fit_sws. Two failure modes existed: + # order, and refresh() can trigger _update_panel() (hence this) twice in a row + # (once from the clients query, once from the activeworkspace one), so several + # syncs can be in flight at once. Two failure modes existed: # 1. The 3 callbacks used to each toggle the shared `self._syncing` flag # independently (True at start, False at end). Whichever resolved *first* # dropped the guard while the others — in particular the tab-selection one, # which programmatically flips the visible stack child — were still in # flight, letting `_on_tab_switch`/`_on_opt_change` misfire as if the user # had clicked and re-apply stale data via `layouts.set(...)`. - # 2. A second `_sync_layout_controls()` call (from the redundant rebuild) could + # 2. A second `_sync_layout_controls()` call (from the redundant refresh) could # land while the first's callbacks were still pending, so a stale callback # from sync #1 could decrement sync #2's pending-count or apply sync #1's - # (possibly wrong-workspace) data into sync #2's freshly built widgets. + # (possibly wrong-workspace) data into sync #2's widgets. # A generation token invalidates any sync superseded by a newer one outright — - # if the user switches workspaces (or a redundant rebuild fires) mid-flight, only + # if the user switches workspaces (or a redundant refresh fires) mid-flight, only # the latest call's callbacks are allowed to touch anything, and self._syncing # only lifts once *that* generation's 3 calls have all resolved. self._sync_generation = getattr(self, "_sync_generation", 0) + 1