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