From 2d9f327da7502236a9c1afd2f3f21cee6c16fc90 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Mon, 12 Oct 2026 13:23:40 +0200 Subject: [PATCH] fix: the sidebar button follows the panel, and the wish survives having no source The toolbar button was the panel's only input: `update_side_visibility()` derived the panel from the button and never the other way, so the button went on saying "shown" in two cases where the panel was off -- while there were no signals to show, and after anything else moved the split view's state. Binding the two together is not the fix. AdwOverlaySplitView writes `show-sidebar` itself, at startup and when it collapses, with no user involved; a binding pushes that into the button, the button's handler reads it as the user's wish, and the sidebar then never comes back when a source connects -- a worse bug than the one reported, and one that a "was this our own write" flag does not catch either, because the toggle's signal may fire after the write returns. So the direction is stated instead of inferred: `show-sidebar` is an output, the button is the only input, and `App::sidebar_wanted` holds the wish between them. The wish is written by the toggle's handler alone (guarded while we set the button ourselves), the no-signals case forces the panel off without touching the wish, and tick() re-asserts both from the wish when the widget disagrees -- so a hide gesture does not stick either. [sidebar-probe] now reads the pair (shown vs button) in both states and against a hide that did not come from the button; shown=1 button=1 with a source, shown=0 button=0 without, and the source reconnecting brings both back is the reading this exists for. --- docs/claude/invariants-plot-and-layout.md | 11 ++++++++ docs/claude/probe-readings.md | 23 +++++++++++---- src/ui.cpp | 33 ++++++++++++++++++++-- src/ui_app.hpp | 10 +++++++ src/ui_probe.cpp | 34 ++++++++++++++++++----- 5 files changed, 95 insertions(+), 16 deletions(-) diff --git a/docs/claude/invariants-plot-and-layout.md b/docs/claude/invariants-plot-and-layout.md index fa45d42..66905c0 100644 --- a/docs/claude/invariants-plot-and-layout.md +++ b/docs/claude/invariants-plot-and-layout.md @@ -16,6 +16,17 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear from the window's width in `tick()`) to *overlay* the plot when there is no room beside it rather than being squeezed. `[sidebar-probe]` reads both states: `window=756 sidebar=320` under a 2x scale is exactly the case that used to be zero. +- **The sidebar's button is the only input; `AdwOverlaySplitView:show-sidebar` is an output.** The split + view writes that property *itself* — at startup, and when it collapses — with no user involved, so reading + it back as the user's wish is wrong: doing that made connecting a source leave the sidebar shut, because + the widget's own `false` was recorded as the user asking for it. A `GBinding` from the property to the + toggle is the same mistake in one line, and it cannot be told from a real change by a "is this our own + write" flag either, since the toggle's signal may fire after the write returns. Instead `App::sidebar_wanted` + holds the wish (written only by `on_sidebar_toggled`, guarded by `App::sidebar_syncing` around our own + `set_active`), `update_side_visibility()` is the single place that turns that wish plus "are there signals" + into the panel's *and* the button's state together, and `tick()` re-asserts them when the widget disagrees — + which is also why a hide *gesture* does not stick: the panel stays where the button says it is. The + no-signals case must leave the wish alone, or a source connecting later finds a sidebar the user never closed. - **`AdwOverlaySplitView:sidebar-width-fraction` is a 0..1 fraction of the window, whatever `sidebar-width-unit` says.** Setting a pixel count there (320) is a `GObject` critical, and it silently falls back to 0.25 — which reads as "the sidebar is mysteriously 560 px wide" on a wide window with no diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index 22f8ae0..46ea473 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -1046,19 +1046,30 @@ library measures and draws the rect in tests of its own. ## `[sidebar-probe]` ``` -[sidebar-probe] beside window=756 sidebar=320 collapsed=1 -[sidebar-probe] overlaid window=756 sidebar=320 collapsed=1 +[sidebar-probe] beside window=756 sidebar=320 collapsed=1 shown=1 button=1(sens=1) +[sidebar-probe] regained window=756 sidebar=320 collapsed=1 shown=1 button=1(sens=1) ``` -`UDPSCOPE_PROBE_SIDEBAR_MS` reads the sidebar's width in both of the split view's states: beside the plot, -and **overlaid** on it (`collapsed`, which is the state a window with no room beside the plot produces). +`UDPSCOPE_PROBE_SIDEBAR_MS` reads the sidebar in the two states a window can put it in, and against a hide +that did not come from the toolbar: `beside` first, then the sidebar is forced into the **overlaid** state +(`collapsed`, which is what a window with no room beside the plot produces), then the split view's own +`show-sidebar` is **turned off behind the button's back** — an overlay swipe, or the widget hiding itself — +and `regained` reads 400 ms later. - **`sidebar` must never be 0.** It was, for every window narrower than the 940 px the divider used to sit at, and the pane could only be found by dragging the divider back — the report this reading exists for. It must read its own width (320) in *both* states: `collapsed` changes where the sidebar is, never how wide it is. -- `collapsed=1` on the `beside` line means the window is narrower than `kSidebarRoom` (900), which is the - expected reading on a scaled display, where the logical width is half what the screen looks like. +- **`shown` and `button` must agree, always.** They are the panel and the toolbar button, and a button that + says "shown" over a hidden panel is the report this reading exists for. `shown=1 button=1` on `regained` + is also the rule that a hide not asked for through the button does not win. +- **`sens=0` with no source is correct.** The button is insensitive and un-pressed while there is nothing to + put in the sidebar; `button=0` is the button telling the truth about the panel rather than the panel + having forgotten the wish. A source reconnecting must give `shown=1 button=1` again — that is the reading + a `wanted` that got eaten by the no-signals frame shows as `shown=0 button=0(sens=1)`, which is the + regression. +- `collapsed=1` means the window is narrower than `kSidebarRoom` (900), which is the expected reading on a + scaled display, where the logical width is half what the screen looks like. ## `[prefs-probe]` diff --git a/src/ui.cpp b/src/ui.cpp index ae3c73b..b9edb7e 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -2751,13 +2751,20 @@ void on_right_click(GtkGestureClick* gesture, int, double x, double y, gpointer void update_side_visibility(App& app) { if (app.side_scroll == nullptr) return; const bool have_signals = !app.model->signals().empty(); - const bool want_column = - have_signals && gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(app.sidebar_toggle)); + // `have_signals` decides whether the sidebar *can* be on; `sidebar_wanted` is the user's last word, + // which is why it is kept apart from the button's own state -- that state follows the panel now, and + // the panel is off while there is nothing to show. A swipe that hides the sidebar fires the button's + // handler (the binding is bidirectional), so a gesture is a choice too. + gtk_widget_set_sensitive(app.sidebar_toggle, have_signals); + const bool want_column = have_signals && app.sidebar_wanted; // The split view owns the sidebar's visibility: it animates it, and when the window is too narrow it // shows it as an overlay rather than letting the column be squeezed to nothing. Hiding the scroller // instead left an empty column where the panels should be. if (app.side_view != nullptr) { + app.sidebar_syncing = true; adw_overlay_split_view_set_show_sidebar(ADW_OVERLAY_SPLIT_VIEW(app.side_view), want_column); + gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(app.sidebar_toggle), want_column); + app.sidebar_syncing = false; } gtk_widget_set_visible(app.signal_toggle, have_signals); gtk_widget_set_visible(app.trigger_toggle, have_signals); @@ -2765,10 +2772,13 @@ void update_side_visibility(App& app) { void on_sidebar_toggled(GtkToggleButton* button, gpointer user) { App* app = static_cast(user); - (void)button; + // Only a state we did not write is a wish: `update_side_visibility` sets this button (and the panel) + // from `sidebar_wanted`, so an unguarded change here is the user's. + if (!app->sidebar_syncing) app->sidebar_wanted = gtk_toggle_button_get_active(button) != FALSE; update_side_visibility(*app); } + // --- rebuild of the derived panels ----------------------------------------- @@ -3627,6 +3637,18 @@ gboolean tick(gpointer user) { // to nothing. `kSidebarRoom` is the sidebar's 320 plus the width below which the plot stops being // worth looking at. if (app->side_view != nullptr && app->window != nullptr) { + // The split view writes `show-sidebar` itself (at startup and when it collapses), so the wish is + // re-asserted here -- but only when they disagree, so a settled frame touches nothing. This is also + // why a hide *gesture* does not stick: the panel is where the button says it is. + const bool want = !app->model->signals().empty() && app->sidebar_wanted; + if (adw_overlay_split_view_get_show_sidebar(ADW_OVERLAY_SPLIT_VIEW(app->side_view)) != want) { + adw_overlay_split_view_set_show_sidebar(ADW_OVERLAY_SPLIT_VIEW(app->side_view), want); + if (gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(app->sidebar_toggle)) != want) { + app->sidebar_syncing = true; + gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(app->sidebar_toggle), want); + app->sidebar_syncing = false; + } + } const int window_w = gtk_widget_get_width(app->window); if (window_w > 0) { adw_overlay_split_view_set_collapsed(ADW_OVERLAY_SPLIT_VIEW(app->side_view), @@ -4437,6 +4459,11 @@ void on_activate(GtkApplication* application, gpointer user) { // this is where it went when the window moved into the file, and it is why the window came up small. gtk_window_set_default_size(GTK_WINDOW(app->window), 1280, 800); + // Nothing listens to the split view's own `show-sidebar`: it is an *output*. `AdwOverlaySplitView` + // writes it itself -- at startup, and when it collapses -- and reading that back as user input is what + // made connecting a source leave the sidebar shut. The button is the only input; `update_side_visibility` + // re-asserts the wish on top of the widget (see the tick). + // The badge and the text share one row so the badge sits at the line's head. It is a // sibling rather than part of the label's own markup because the label's text is // rewritten every frame and its markup is ellipsized -- a blinking span inside it would diff --git a/src/ui_app.hpp b/src/ui_app.hpp index b39850d..9f264da 100644 --- a/src/ui_app.hpp +++ b/src/ui_app.hpp @@ -408,6 +408,16 @@ struct App { /// The split view the sidebar lives in. Its width, its collapse and its overlay are that widget's /// business -- `AdwOverlaySplitView` -- and not a paned position's. GtkWidget* side_view = nullptr; + /// Whether the user wants the sidebar: their last word on the toolbar's button. Kept apart from the + /// button's own state because that state now *follows* the panel, and the panel is forced off while + /// there is nothing to show -- without this, connecting a source would leave the sidebar closed. + bool sidebar_wanted = true; + /// Set while we are writing the split view's or the button's state ourselves. A *binding* cannot + /// express this: the widget moves `show-sidebar` on its own when it collapses, the binding pushes that + /// into the button, and the button's handler then records it as the user's wish -- so the sidebar never + /// comes back when a source connects. An explicit flag is reliable precisely because our writes go + /// through `update_side_visibility`, which sets the flag around them. + bool sidebar_syncing = false; GtkBuilder* window_ui = nullptr; bool comparing = false; diff --git a/src/ui_probe.cpp b/src/ui_probe.cpp index 3c615ba..a495875 100644 --- a/src/ui_probe.cpp +++ b/src/ui_probe.cpp @@ -1634,7 +1634,8 @@ gboolean boxzoom_probe(gpointer user) { } -/// `UDPSCOPE_PROBE_SIDEBAR_MS`: the sidebar's width, in both of the split view's states. +/// `UDPSCOPE_PROBE_SIDEBAR_MS`: the sidebar's width and whether the toolbar button agrees with it, in +/// both of the split view's states and against a hide that did not come from the button. /// /// The small-window case is the one a divider position got wrong: with the sidebar's width derived from /// the window, a window narrower than that number gave it nothing at all. The split view takes its size @@ -1644,24 +1645,43 @@ gboolean boxzoom_probe(gpointer user) { gboolean sidebar_probe(gpointer user) { App& a = *static_cast(user); if (a.side_view == nullptr) return G_SOURCE_REMOVE; + // `shown` is the panel's own state and `button` the toolbar's; they must agree, or the button is + // telling the user something the window does not do. const auto report = [](App& app, const char* what) { const int side = app.side_box != nullptr ? gtk_widget_get_width(app.side_box) : -1; const int win = app.window != nullptr ? gtk_widget_get_width(app.window) : -1; - std::fprintf(stderr, "[sidebar-probe] %-10s window=%d sidebar=%d collapsed=%d\n", what, win, - side, adw_overlay_split_view_get_collapsed(ADW_OVERLAY_SPLIT_VIEW(app.side_view)) ? 1 - : 0); + std::fprintf(stderr, + "[sidebar-probe] %-10s window=%d sidebar=%d collapsed=%d shown=%d button=%d(sens=%d)\n", + what, win, side, + adw_overlay_split_view_get_collapsed(ADW_OVERLAY_SPLIT_VIEW(app.side_view)) ? 1 : 0, + adw_overlay_split_view_get_show_sidebar(ADW_OVERLAY_SPLIT_VIEW(app.side_view)) ? 1 : 0, + app.sidebar_toggle != nullptr && + gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(app.sidebar_toggle)) ? 1 : 0, + app.sidebar_toggle != nullptr && gtk_widget_get_sensitive(app.sidebar_toggle) ? 1 : 0); }; report(a, "beside"); // The state a window with no room produces: the sidebar goes over the plot, and its width is still its // own. If this ever reads 0, the sidebar is being given a share of a width that is not there. adw_overlay_split_view_set_collapsed(ADW_OVERLAY_SPLIT_VIEW(a.side_view), TRUE); + // A hide that did not come from the button -- an overlay swipe, or the split view's own collapsing -- + // must not win: the panel is where the button says it is. `shown` and `button` still reading 1 here is + // the point, and reading 0 is the regression (a button that no longer reflects the panel, or a panel + // that slips out from under it). + adw_overlay_split_view_set_show_sidebar(ADW_OVERLAY_SPLIT_VIEW(a.side_view), FALSE); g_timeout_add(400, +[](gpointer u) -> gboolean { App& app = *static_cast(u); const int side = app.side_box != nullptr ? gtk_widget_get_width(app.side_box) : -1; - std::fprintf(stderr, "[sidebar-probe] %-10s window=%d sidebar=%d collapsed=%d\n", "overlaid", - app.window != nullptr ? gtk_widget_get_width(app.window) : -1, side, - adw_overlay_split_view_get_collapsed(ADW_OVERLAY_SPLIT_VIEW(app.side_view)) ? 1 : 0); + std::fprintf(stderr, + "[sidebar-probe] %-10s window=%d sidebar=%d collapsed=%d shown=%d button=%d(sens=%d)\n", + "regained", app.window != nullptr ? gtk_widget_get_width(app.window) : -1, side, + adw_overlay_split_view_get_collapsed(ADW_OVERLAY_SPLIT_VIEW(app.side_view)) ? 1 : 0, + adw_overlay_split_view_get_show_sidebar(ADW_OVERLAY_SPLIT_VIEW(app.side_view)) ? 1 : 0, + app.sidebar_toggle != nullptr && + gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(app.sidebar_toggle)) ? 1 : 0, + app.sidebar_toggle != nullptr && gtk_widget_get_sensitive(app.sidebar_toggle) ? 1 : 0); adw_overlay_split_view_set_collapsed(ADW_OVERLAY_SPLIT_VIEW(app.side_view), FALSE); + // Back the way a person would: pressing the button is what records the wish, and the panel follows it. + gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(app.sidebar_toggle), TRUE); return G_SOURCE_REMOVE; }, &a); return G_SOURCE_REMOVE;