From 3e060dbf4358a69227640b147671d3718f963803 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 20 Aug 2026 18:21:39 +0200 Subject: [PATCH] fix(gui): pair the LAN switches, anchor the search row, and model pack order Three things, and the third is what lets the suite check the first two. The two LAN switches now share a row. They are one decision seen from two sides, and stacked they left half a card of nothing beside each while the second read as an afterthought under the first. A checkbox takes a whole row BY KIND (a long label clips in half a card), so Field.span became tri-state: None decides by kind, True claims a row, False pairs. Only three BOOL fields exist and narrow_filter keeps its own row, so the Settings window is untouched. The CI render guard measured buttons only, which is how a clipped CHECKBOX label could have shipped - and this change puts the two longest ones side by side. It measures checkbuttons now. Verified at 1366x768 in both languages with the widget count printed, so the OK is not vacuous: four per language, none clipped, the Polish pair asking 249 and 195 px and getting both. The Control search bar is on its THIRD shape and the first two failed the same way: everything packed left was "a stray label floating above the first section with the whole width empty beside it", and everything packed right drew the same report mirrored. The side was never the problem - a row with ONE cluster has a band of nothing beside it wherever the cluster goes. Both ends are anchored now, which is the shape the Connections toolbar already uses. Measured: the label lands on the section cards' own left edge, 12 px further left than before, and the count ends a pixel from their right. The idle state is the one the page is looked at in, so the right end carries Ctrl+F while the box is empty, in the same fixed-width label the count uses. The fake tkinter answered pack_slaves in CREATION order while its docstring claimed pack order. Measured: pack A, then pack B with before=a, and it still said [A, B]. So a widget re-packed above an existing sibling looked right in the suite and landed under the whole page on real Tk - exactly what the search bar does when a preference brings it back, and why that order could until now only be checked by rendering. The parent keeps an ordered list; pack honours before=/after= and MOVES an already-packed widget. Written against __dict__, because __getattr__ answers any unknown attribute with a no-op callable and the "not set yet" branch is otherwise unreachable - that bit the first draft. Guards: the harness ordering itself, the switches sharing a row, and the two search tests that were asserting "cleared = blank" as a proxy for "no position". Four MUTATIONS entries, all caught, plus one repaired: an older entry quoted span=True on lan_mode and the bookkeeping check caught it going stale. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 10 +++++ beantester/fields.py | 18 ++++++-- beantester/gui/form.py | 15 ++++++- beantester/gui/pages/control.py | 79 ++++++++++++++++++++++----------- tests/fake_tk.py | 48 +++++++++++++++++++- tests/test_gui_layout.py | 75 +++++++++++++++++++++++++++++-- tests/test_mutation_registry.py | 32 ++++++++++++- tests/test_prefs.py | 12 +++-- tools/ci_gui_render.py | 7 ++- 9 files changed, 253 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b35e5fe..bb83891 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol ## [Unreleased] +### Changed + +- **The two LAN switches sit side by side.** "LAN mode" and "Internet only" are one decision seen + from two sides, so they now share a row under "Traffic to modify" instead of being stacked with + half a card of empty space beside each. +- **The search box on the Control page moved to the left of its row**, where the Connections tab + keeps its own, and the right end of that row now shows `Ctrl+F` when you are not searching. The + box used to sit alone against the right margin with the rest of the row empty, which read as + something dropped onto the page rather than part of it. + ### Added - **"Internet only (no local network)" - a new checkbox under "Traffic to modify", the mirror of diff --git a/beantester/fields.py b/beantester/fields.py index 2225693..a0bd90f 100644 --- a/beantester/fields.py +++ b/beantester/fields.py @@ -66,7 +66,12 @@ class Field(NamedTuple): hint: str = "" # i18n key of the greyed hint next to the entry in_profile: bool = False # stored by a user profile / built-in preset preset_key: str = "" # short key a preset/profile stores it under ("" = key) - span: bool = False # takes a whole row in the section grid + # Whole row, or share one? ``None`` = decide from the kind, which is what + # nearly every field wants: a checkbox, a dropdown or an expression carries a + # long label and would be clipped in half a card (``gui/form.py::SPAN_KINDS``). + # Say ``False`` to override that and PAIR the field with its neighbour - the + # two LAN switches read as a pair and are short enough to sit side by side. + span: bool | None = None # True = own row, False = share, None = by kind cli: str = "" # argparse flag (without "--") overridden_by: str = "" # key of a field that makes this one inert override_note: str = "" # i18n key explaining the override, shown in the form @@ -86,8 +91,11 @@ class Field(NamedTuple): # like a scope ("only the local network") and is the exact opposite. decide() # step 2b DROPS every packet whose remote end is public, so this flag alone, # with the whole rest of the form at zero, cuts the machine's internet. + # span=False on both: they are one decision seen from two sides, so they sit + # side by side rather than stacked with a card's width of nothing beside each. + # A BOOL would take a whole row by kind - see fields.Field.span. Field("lan_mode", BOOL, "fields.lan_mode", "traffic", - tip="tips.lan_mode", span=True, cli="lan-mode", impairs=IMPAIRS_ALL), + tip="tips.lan_mode", span=False, cli="lan-mode", impairs=IMPAIRS_ALL), # The mirror of the line above, and IMPAIRS_ALL for the same reason: on its # own, with the whole rest of the form at zero, it cuts every local address # this machine talks to. Loopback survives (utils.is_lan_ip). @@ -97,7 +105,7 @@ class Field(NamedTuple): # with exit 2 - so naming it --lan-cut would silently break a documented # shortcut of --lan-mode (allow_abbrev stays on: ADR 2026-08-02). Field("internet_only", BOOL, "fields.internet_only", "traffic", - tip="tips.internet_only", span=True, cli="internet-only", + tip="tips.internet_only", span=False, cli="internet-only", impairs=IMPAIRS_ALL), # -- target process ---------------------------------------------------- # @@ -301,8 +309,10 @@ class Section(NamedTuple): # preset picker must be the first thing they see - not the last section after # a dozen panels of NAT/MTU/RST jargon. Section("profiles", "frames.profiles", (), columns=1, extra="profiles"), + # Two columns so the pair of LAN switches shares a row; the filter dropdown + # above them still spans both, because a CHOICE takes a row by kind. Section("traffic", "frames.traffic", ("filter", "lan_mode", "internet_only"), - columns=1), + columns=2), # No "enable" checkbox: an empty target already means "all traffic", so the # checkbox was a switch that did nothing but take a click (same for the two # sections below). diff --git a/beantester/gui/form.py b/beantester/gui/form.py index feaaddb..4f06bf0 100644 --- a/beantester/gui/form.py +++ b/beantester/gui/form.py @@ -37,6 +37,17 @@ SECTION_BY_ID = {s.id: s for s in SECTIONS} SPAN_KINDS = (F.CHOICE, F.BOOL, F.EXPR, F.SCHEDULE) + + +def _takes_a_row(field): + """Does this field get a row to itself, or does it share one? + + The kind decides by default, because these four carry a label long enough to + be clipped in half a card. A field may override it: ``span=True`` claims a + row a short kind would not get, ``span=False`` PAIRS a long kind with its + neighbour. Only the registry may say so - the caller reads the answer. + """ + return field.kind in SPAN_KINDS if field.span is None else field.span VALIDATED_KINDS = (F.NUMBER, F.EXPR, F.SCHEDULE, F.SEED) # Below this width a second column of sections would squeeze the wider rows @@ -176,7 +187,7 @@ def _rows_of(self, sec): rows, current = [], [] for key in sec.fields: field = FIELDS[key] - if field.span or field.kind in SPAN_KINDS: + if _takes_a_row(field): if current: rows.append(current) current = [] @@ -227,7 +238,7 @@ def _place_one(self, row, field, sec): app.filter_cb = widget return - span = field.span or field.kind in SPAN_KINDS + span = _takes_a_row(field) cell = ttk.Frame(row, style="Card.TFrame") cell.pack(side="left", fill="x", expand=span, padx=(0, scaled(6) if span else scaled(22))) diff --git a/beantester/gui/pages/control.py b/beantester/gui/pages/control.py index acba8ae..c9dfc17 100644 --- a/beantester/gui/pages/control.py +++ b/beantester/gui/pages/control.py @@ -81,31 +81,37 @@ def __init__(self, app, parent): # -- search -------------------------------------------------------------- # def _build_search_bar(self): - """The bar sits on the RIGHT, against the page's own margin. - - On the left it read as a stray label floating above the first section with - the whole width empty beside it. On the right it lines up with the page - margin, sits clear of the eye's path down the first column, and lands - where a find box is looked for. - - 🔴 The BOX is what the eye calls "the search", so the box is what has to - be flush with the margin. Measured on real Tk (1200x900): with the count - pinned to the right the entry ended 65 px short of the section cards - beside it, and a control that stops short of every other right edge on the - page reads as something dropped on top of the page rather than part of it. - So the count moved to the LEFT of the label, where it still cannot push - anything around: everything right of it has a fixed width. - - Packed right to left: entry (against the margin), its label, the count, - then the sentence that names another window - which is free to grow - leftwards into empty space without moving anything. + """A toolbar with BOTH ends anchored: box on the left, count on the right. + + 🔴 Third shape, and the first two were rejected for the SAME complaint - + "it looks like it was dropped on the page". Worth knowing before moving + it again, because the obvious fix is to move it back: + + * everything packed LEFT (until 2026-08-11) - "a stray label floating + above the first section with the whole width empty beside it"; + * everything packed RIGHT (until 2026-08-20) - the same, mirrored, and an + alignment fix in between did not settle it either. + + So the side was never the problem. A row with one cluster in it has a + band of nothing next to that cluster wherever the cluster goes. This one + has a group at each end, which is the shape the Connections toolbar + already uses and nobody has reported: the gap in the middle then reads as + the space BETWEEN two groups rather than as emptiness beside one. It also + puts "Szukaj" in the same place on both pages that have it. + + Nothing may move while typing, which is what the packing order protects: + the label and the box are pinned to the left margin, the count is pinned + to the right at a fixed width, and the sentence naming another window + grows leftwards into the middle, where there is nothing to push. """ bar = ttk.Frame(self.frame) self._bar = bar self._pack_bar() self.query_var = tk.StringVar(value=_LAST_QUERY[0]) + ttk.Label(bar, text=T("fields.search")).pack(side="left", + padx=(0, scaled(6))) entry = ttk.Entry(bar, textvariable=self.query_var, width=24) - entry.pack(side="right") + entry.pack(side="left") self._count = ttk.Label(bar, text="", style="Muted.TLabel", width=8, anchor="e") entry.bind("", self._on_key) @@ -114,11 +120,15 @@ def _build_search_bar(self): entry.bind("", lambda e: self.clear()) add_tooltip(entry, "tips.control_search", shortcut="Ctrl+F") self._entry = entry - ttk.Label(bar, text=T("fields.search")).pack(side="right", - padx=(scaled(6), scaled(5))) + # Pinned to the right margin at a fixed width, so the number changing + # from "1 / 9" to "10 / 90" cannot push anything. + # Filled at build, not left blank: a page that opens with an empty box + # must already show the shortcut, or the row starts life lopsided. + # `_targets` and `query_var` both exist by now (see __init__). + self._count.config(text=self._count_text()) self._count.pack(side="right") - # Free to grow leftwards: "..." is in the Settings window, or "nothing - # matches". Never to the right of the box, which must not move. + # Free to grow leftwards into the middle: "..." is in the Settings + # window, or "nothing matches". There is nothing to its left to shove. self._note = ttk.Label(bar, text="", style="Muted.TLabel", anchor="e") self._note.pack(side="right", padx=(0, scaled(10))) # Bound on the ROOT through the shared dispatcher, for the same reason the @@ -146,7 +156,7 @@ def _pack_bar(self, **extra): ``before=`` names a sibling and keeps children in creation order), so this one is answered by a live render, not by the suite. """ - self._bar.pack(side="top", fill="x", padx=(scaled(12), scaled(14)), + self._bar.pack(side="top", fill="x", padx=(0, scaled(14)), pady=(scaled(12), scaled(3)), **extra) def search_is_visible(self): @@ -230,7 +240,9 @@ def _apply(self): self._at = 0 if not query.strip(): self._restore_folds() - self._say("", "") + # Not blank: the idle right end carries the shortcut, which is what + # stops the row reading as one cluster in a band of nothing. + self._say(self._count_text(), "") return if self._targets: self._paint() @@ -267,7 +279,22 @@ def _say(self, count, note): self._note.config(text=note) def _count_text(self): - return "%d / %d" % (self._at + 1, len(self._targets)) if self._targets else "" + """What the right end of the bar says: the position, or the shortcut. + + 🔴 The shortcut is not decoration - it is what keeps the row anchored at + BOTH ends. With an empty box the count and the note are both blank, and a + toolbar with one cluster on the left and nothing else is exactly the + shape that has now been reported twice (see ``_build_search_bar``). The + idle state is the state the page is looked at in, so it is the one that + has to hold. + + Same label, same fixed width, so nothing moves when it turns into a + count. A key name is not translated - the tooltips pass these as literals + too. + """ + if self._targets: + return "%d / %d" % (self._at + 1, len(self._targets)) + return "" if self.query_var.get().strip() else "Ctrl+F" def _note_text(self, elsewhere): if elsewhere: diff --git a/tests/fake_tk.py b/tests/fake_tk.py index 2cc8fc1..f0efaf2 100644 --- a/tests/fake_tk.py +++ b/tests/fake_tk.py @@ -101,13 +101,57 @@ def pack(self, **kw): f"(its parent is {getattr(sibling, 'master', None)!r}, " f"ours is {self.master!r})") self.pack_info = dict(kw) + self._enter_pack_order(kw.get("before"), kw.get("after")) + + def _enter_pack_order(self, before=None, after=None): + """Take this widget's place in its parent's pack order. + + 🔴 Modelled, because until 2026-08-20 it was not and the docstring below + SAID it was. `pack_slaves` returned `self.children`, which is CREATION + order, so a widget packed above an existing sibling looked right here and + landed under the whole page on real Tk. A GUI that re-packs anything - + the Control page puts its search bar back when a preference returns it - + could therefore only be checked by rendering. + + Order is what pack cares about: appended in call order, or placed + relative to a named sibling. Re-packing an already-packed widget MOVES + it, exactly as Tk does. + """ + parent = self.master + if parent is None: + return + # 🔴 `__dict__`, never `getattr(..., None)`: `W.__getattr__` answers ANY + # unknown attribute with a no-op CALLABLE, so the default never arrives + # and the "not set yet" branch is unreachable. Same trap `winfo_toplevel` + # is explicit about. + order = parent.__dict__.get("_pack_order") + if order is None: + order = parent._pack_order = [] + if self in order: + order.remove(self) + index = len(order) + if before is not None and before in order: + index = order.index(before) + elif after is not None and after in order: + index = order.index(after) + 1 + order.insert(index, self) def pack_forget(self): self.pack_info = None + parent = self.master + order = parent.__dict__.get("_pack_order") if parent is not None else None + if order is not None and self in order: + order.remove(self) def pack_slaves(self): - """The children this widget currently manages with pack - in pack order.""" - return [c for c in self.children if getattr(c, "pack_info", None) is not None] + """The children this widget manages with pack, in PACK order. + + Creation order until 2026-08-20, when the claim was measured and found + false - see ``_enter_pack_order``. Anything not packed through ``pack`` + (grid, or never packed) is not here, which is also what Tk answers. + """ + return [c for c in self.__dict__.get("_pack_order", []) + if c.__dict__.get("pack_info") is not None] def winfo_ismapped(self): return 1 if self.pack_info is not None else 0 diff --git a/tests/test_gui_layout.py b/tests/test_gui_layout.py index ddc2f81..4029e27 100644 --- a/tests/test_gui_layout.py +++ b/tests/test_gui_layout.py @@ -5,6 +5,7 @@ report", "Copy CLI" and the whole event table were simply unreachable. Nothing could catch that, because the fake tkinter ignored geometry calls entirely. """ +from fakes import check from gui_harness import run_gui @@ -432,7 +433,10 @@ def test_clearing_the_search_puts_every_style_back(): page.clear() assert app.form.labels["loss"].cget("style") == before, ( app.form.labels["loss"].cget("style")) - assert page._note.cget("text") == "" and page._count.cget("text") == "" + # An empty box says the shortcut, never a position: the right end of the + # bar is what keeps the row anchored while nothing is being searched. + assert page._note.cget("text") == "" + assert page._count.cget("text") == "Ctrl+F", page._count.cget("text") """) @@ -661,12 +665,16 @@ def test_a_query_of_nothing_but_punctuation_is_not_a_search(): page.query_var.set(query) page._apply() if query.strip() in ("--", ""): - assert page._count.cget("text") == "", (query, page._count.cget("text")) + # No POSITION - which is the claim. A box that strips to nothing + # is idle and says the shortcut instead; one with characters in + # it that match nothing says nothing at all. + idle = "Ctrl+F" if not query.strip() else "" + assert page._count.cget("text") == idle, (query, page._count.cget("text")) assert not page._marks, query # whatever it finds, it must not raise and must not leave the page # marked once it is cleared page.clear() - assert not page._marks and page._count.cget("text") == "" + assert not page._marks and page._count.cget("text") == "Ctrl+F" """) @@ -864,3 +872,64 @@ class Ev: table._on_motion(Ev()) assert table._tip_column is None and table._tip_window is None """) + + +def test_the_harness_models_pack_order_so_layout_tests_can_ask_about_it(): + """The instrument that checks layout has to model the thing being checked. + + ``pack_slaves`` returned CREATION order while its docstring claimed pack + order (measured 2026-08-20). Nothing was red: a widget re-packed above an + existing sibling looked correct here and landed under the whole page on real + Tk, so every ordering question had to be answered by rendering. The Control + page re-packs its search bar whenever the preference brings it back, which is + exactly that shape. + + Four cases, because Tk answers all four: append, ``before=``, ``after=``, and + re-packing an already-packed widget (which MOVES it). + """ + import fake_tk + root = fake_tk.Root() + a, b, c = (fake_tk.W(root) for _ in range(3)) + order = lambda: root.pack_slaves() + + a.pack(side="top") + c.pack(side="top") + check("harness: pack appends in call order", order() == [a, c], f"{order()}") + + b.pack(side="top", before=c) + check("harness: before= puts it in front of the named sibling", + order() == [a, b, c], f"{order()}") + + b.pack_forget() + check("harness: forgetting takes it out of the order", order() == [a, c], + f"{order()}") + + b.pack(side="top", after=a) + check("harness: after= puts it behind the named sibling", + order() == [a, b, c], f"{order()}") + + a.pack(side="top") + check("harness: re-packing MOVES rather than duplicates", + order() == [b, c, a], f"{order()}") + + +def test_the_two_lan_switches_share_one_row(): + """One decision seen from two sides, so they sit side by side. + + Stacked, each of them had a card's width of nothing beside it, and the + second one read as an afterthought under the first rather than as its + mirror. A checkbox takes a whole row BY KIND (``gui/form.py::SPAN_KINDS`` - + a long label clips in half a card), so this only holds while the registry + overrides that with ``span=False``: one edit in ``fields.py`` puts them back + in a column with nothing red. + + The dropdown above them must NOT join in: it is a CHOICE, it is the widest + thing in the card, and it keeps its own row. + """ + run_gui(""" + lan = app.form.entries["lan_mode"] + net = app.form.entries["internet_only"] + assert lan.master is net.master, "the LAN switches are not in one row" + assert app.form.entries["filter"].master is not lan.master, ( + "the traffic dropdown was pulled into the checkbox row") + """) diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index 5ca2dc1..8851a15 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -393,8 +393,8 @@ { "label": "warning: LAN mode is demoted from impairment to scenery", "file": "beantester/fields.py", - "old": " tip=\"tips.lan_mode\", span=True, cli=\"lan-mode\", impairs=IMPAIRS_ALL),", - "new": " tip=\"tips.lan_mode\", span=True, cli=\"lan-mode\"),", + "old": " tip=\"tips.lan_mode\", span=False, cli=\"lan-mode\", impairs=IMPAIRS_ALL),", + "new": " tip=\"tips.lan_mode\", span=False, cli=\"lan-mode\"),", "test": "test_the_warning_names_lan_mode_which_reads_like_a_scope", }, { @@ -1326,6 +1326,34 @@ "new": " pass", "test": "test_every_setting_with_a_flag_reaches_the_reproduction_command", }, + { + # The harness itself. It claimed pack order for months while answering in + # creation order, so every ordering question had to go to a live render. + "label": "harness: the fake stops honouring before= when packing", + "file": "tests/fake_tk.py", + "old": " if before is not None and before in order:\n" + " index = order.index(before)", + "new": " if False:\n index = 0", + "test": "test_the_harness_models_pack_order_so_layout_tests_can_ask_about_it", + }, + { + # A checkbox takes a whole row BY KIND, so the pair goes back to a column + # the moment the registry's override stops being read. + "label": "gui: the form ignores a field's span override", + "file": "beantester/gui/form.py", + "old": " return field.kind in SPAN_KINDS if field.span is None else field.span", + "new": " return field.kind in SPAN_KINDS", + "test": "test_the_two_lan_switches_share_one_row", + }, + { + # Without the idle hint the row is one cluster in a band of nothing - + # the shape that has now been reported twice. + "label": "gui: the search bar loses its idle right-hand anchor", + "file": "beantester/gui/pages/control.py", + "old": ' return "" if self.query_var.get().strip() else "Ctrl+F"', + "new": ' return ""', + "test": "test_clearing_the_search_puts_every_style_back", + }, ] # The runner's own check: a patch that cannot compile must be reported as BROKEN, not diff --git a/tests/test_prefs.py b/tests/test_prefs.py index 674e71f..1e0d8cf 100644 --- a/tests/test_prefs.py +++ b/tests/test_prefs.py @@ -351,9 +351,12 @@ def test_the_control_search_bar_can_be_switched_off_and_back_on(): The order is the part that can go wrong silently: pack hands out space in CALL order, so a bar re-packed after the scroller exists lands UNDER the whole - page unless it names what to sit before. This asserts the call carries - ``before=``; whether Tk then draws it in the right place is a live-render - question the fake cannot answer (it keeps children in creation order). + page unless it names what to sit before. + + Upgraded 2026-08-20: the fake now models pack order, so this asserts the bar + is BACK ABOVE the page body rather than merely that the call carried + ``before=``. Until then the real question had to go to a live render, because + ``pack_slaves`` answered in creation order and could not tell the two apart. """ run_gui(""" page = app.pages["control"] @@ -369,6 +372,9 @@ def test_the_control_search_bar_can_be_switched_off_and_back_on(): assert page._bar.winfo_ismapped(), "the bar did not come back" assert page._bar.pack_info.get("before") is page.scroll.vsb, ( "re-packed without before= - it would sit under the page body") + packed = page.frame.pack_slaves() + assert packed.index(page._bar) < packed.index(page.scroll.canvas), ( + "the bar came back UNDER the page body: %r" % (packed,)) # an unrelated preference must not move it page.on_pref_changed("chart_seconds") diff --git a/tools/ci_gui_render.py b/tools/ci_gui_render.py index 7861bbc..170f6d8 100644 --- a/tools/ci_gui_render.py +++ b/tools/ci_gui_render.py @@ -52,7 +52,12 @@ from beantester.gui.windows import WINDOWS # noqa: E402 GEOMETRY = "1366x768" -BUTTON_CLASSES = ("TButton", "Button") +# 🔴 Checkbuttons are in here as of 2026-08-20, and they were the obvious gap: +# a checkbox is a label that cannot wrap, so it clips exactly like a button, and +# the two longest ones in the program sit side by side in one card. The class was +# simply never listed - the guard was written for the "Wesprzyj projekt" -> "Wesp" +# report, and that was a TButton. +BUTTON_CLASSES = ("TButton", "Button", "TCheckbutton", "Checkbutton") LABEL_CLASSES = ("TLabel", "Label") TOL = 1 # px slack against rounding