diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ae36157..a350ccf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -102,7 +102,7 @@ repos: additional_dependencies: - pytest - requests - - gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock + - gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.5.0#subdirectory=gatelock - crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync - mcp>=2,<3 - pydantic diff --git a/app/pubspec.yaml b/app/pubspec.yaml index 719b527..1ca4013 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -2,7 +2,7 @@ name: diet_guard_app description: "Companion phone app for diet_guard: log meals on the go." publish_to: 'none' -version: 1.0.9+1 +version: 1.0.10+1 environment: sdk: ^3.12.2 diff --git a/diet_guard/_gatelock_buttons.py b/diet_guard/_gatelock_buttons.py deleted file mode 100644 index c822e04..0000000 --- a/diet_guard/_gatelock_buttons.py +++ /dev/null @@ -1,94 +0,0 @@ -"""Shared primary/secondary/danger button builder for the diet_guard gate. - -Split out of :mod:`._gatelock_ui` to keep every gate module under the repo's -500-line limit. Centralizes what used to be 7 independent ``tk.Button(...)`` -calls (across :mod:`._gatelock_ui` and :mod:`._gatelock_calendar`) each -picking their own bg/fg/activebackground -- see ``DESIGN_AUDIT_TODO.md``. -""" - -from __future__ import annotations - -import tkinter as tk -from typing import TYPE_CHECKING, Literal - -from gatelock import LockConfig - -from diet_guard._gatelock_typography import BODY, LABEL - -if TYPE_CHECKING: - from collections.abc import Callable - -_COLORS = LockConfig() - - -def _lighten(hex_color: str, amount: float = 0.12) -> str: - """Blend ``hex_color`` toward white by ``amount``, for a button hover state.""" - red, green, blue = (int(hex_color[i : i + 2], 16) for i in (1, 3, 5)) - return "#" + "".join( - f"{round(channel + (255 - channel) * amount):02x}" - for channel in (red, green, blue) - ) - - -# Fill/text pairs per button role, read from the shared LockConfig palette -# instead of each call site inventing its own hex pair. "primary" is the -# one high-emphasis action per screen (rule 3); "secondary" and "danger" -# stay low-contrast/alarm respectively. All three use `on_fill` (never -# `fg`) for text-on-a-filled-surface, per tokens.md. -_BUTTON_FILLS: dict[str, tuple[str, str]] = { - "primary": (_COLORS.accent, _COLORS.on_fill), - "secondary": (_COLORS.field_bg, _COLORS.fg), - "danger": (_COLORS.danger, _COLORS.on_fill), -} -# The one high-emphasis button per screen reads larger than everything else -# (rule 3: size is another axis of prominence, alongside color) -- every -# other variant shares the same, smaller chrome size (tokens.md's type scale). -_BUTTON_SIZE: dict[str, int] = { - "primary": BODY, - "secondary": LABEL, - "danger": LABEL, -} -# Horizontal = 2x vertical (tokens.md "Buttons"; both on the 4px spacing scale). -_BUTTON_PADY = 12 -_BUTTON_PADX = 24 - - -def make_button( - parent: tk.Misc, - *, - text: str, - variant: Literal["primary", "secondary", "danger"], - command: Callable[[], None], - bold: bool = True, -) -> tk.Button: - """Build a button from the shared primary/secondary/danger palette. - - Binds ```` as well as relying on ``command``. Tk's X11 class - bindings give ``tk.Button`` only ````; there is no ```` - binding, so without this every button in the gate is Space-only and Enter - silently does nothing -- which is not the key a user reaches for. - """ - fill, text_color = _BUTTON_FILLS[variant] - size = _BUTTON_SIZE[variant] - font = (_COLORS.font_family, size, "bold") if bold else (_COLORS.font_family, size) - button = tk.Button( - parent, - text=text, - font=font, - bg=fill, - fg=text_color, - activebackground=_lighten(fill), - cursor="hand2", - padx=_BUTTON_PADX, - pady=_BUTTON_PADY, - command=command, - **_COLORS.focus_kwargs(), - ) - - def _activate(_event: tk.Event) -> str: - button.invoke() - return "break" - - button.bind("", _activate, add="+") - button.bind("", _activate, add="+") - return button diff --git a/diet_guard/_gatelock_calendar_ui.py b/diet_guard/_gatelock_calendar_ui.py index d8aade7..1109df1 100644 --- a/diet_guard/_gatelock_calendar_ui.py +++ b/diet_guard/_gatelock_calendar_ui.py @@ -13,9 +13,8 @@ import tkinter as tk from tkinter import ttk from typing import TYPE_CHECKING -from gatelock import LockConfig +from gatelock import ButtonStyle, LockConfig, make_button -from diet_guard._gatelock_buttons import make_button from diet_guard._gatelock_spacing import MD, SM, XS from diet_guard._gatelock_typography import BODY, CAPTION, LABEL, TITLE from diet_guard._gatelock_ui import ( @@ -130,9 +129,9 @@ def _build_budget_row( entry.pack(side="left", padx=(SM, SM), ipady=XS) edit_button = make_button( row, - text="Edit", - variant="primary", - command=on_edit_or_save_budget, + _COLORS, + "Edit", + on_edit_or_save_budget, ) edit_button.pack(side="left") status_label = tk.Label( @@ -156,9 +155,10 @@ def _build_month_nav( row.pack(pady=(XS, XS)) make_button( row, - text="◀", - variant="secondary", - command=callbacks.on_prev_month, + _COLORS, + "◀", + callbacks.on_prev_month, + ButtonStyle(variant="secondary"), ).pack(side="left", padx=XS) tk.Label( row, @@ -171,9 +171,10 @@ def _build_month_nav( ).pack(side="left") make_button( row, - text="▶", - variant="secondary", - command=callbacks.on_next_month, + _COLORS, + "▶", + callbacks.on_next_month, + ButtonStyle(variant="secondary"), ).pack(side="left", padx=XS) diff --git a/diet_guard/_gatelock_groups.py b/diet_guard/_gatelock_groups.py index a03f1a3..2e20f5e 100644 --- a/diet_guard/_gatelock_groups.py +++ b/diet_guard/_gatelock_groups.py @@ -24,10 +24,12 @@ from __future__ import annotations import contextlib import tkinter as tk -from typing import TYPE_CHECKING, Generic, TypeVar +from typing import TYPE_CHECKING, TypeVar + +from gatelock import WidgetGroup as SharedWidgetGroup if TYPE_CHECKING: - from collections.abc import Callable, Iterator + from collections.abc import Callable from diet_guard._gatelock_ui_types import GateVars, GateWidgets @@ -35,60 +37,57 @@ _W = TypeVar("_W", bound=tk.Misc) """The widget each group mirrors, so a subclass keeps its own methods typed.""" -class WidgetGroup(Generic[_W]): - """One logical widget, mirrored onto every monitor.""" +class WidgetGroup(SharedWidgetGroup[_W]): + """One logical widget, mirrored onto every monitor. + + The fan-out itself -- iteration, ``configure``, ``bind``, singular focus, + and skipping copies whose surface has been destroyed -- now lives in + :class:`gatelock.WidgetGroup`, which four apps had each reimplemented. + This subclass exists only to keep the positional ``list[_W]`` + construction the gate's own builders and specialized groups use; gatelock + stores ``(output_name, widget)`` pairs, because the parallel-list form the + donors shared could be built with widgets but no names. + + ``first`` is re-narrowed to ``_W``: the gate always builds at least one + copy before anything reads one, and the specialized groups below index + into it without a None check. + """ def __init__(self, widgets: list[_W]) -> None: """Wrap the per-monitor copies of one widget.""" - self._widgets = widgets - - def __iter__(self) -> Iterator[_W]: - """Iterate the per-monitor copies.""" - return iter(self._widgets) + super().__init__([("", widget) for widget in widgets]) @property def first(self) -> _W: """The primary monitor's copy, for reads that cannot fan out.""" - return self._widgets[0] - - def config(self, **kwargs: str) -> None: - """Apply the same configuration to every copy.""" - for widget in self._widgets: - with contextlib.suppress(tk.TclError): - widget.configure(**kwargs) - - configure = config + widget = super().first + if widget is None: + message = "widget group is empty" + raise IndexError(message) + return widget def bind( - self, sequence: str, callback: Callable[[tk.Event[tk.Misc]], object] + self, + sequence: str, + func: Callable[[tk.Event], object], + *, + add: bool = False, ) -> None: - """Bind the same handler on every copy. + """Bind the same handler on every copy, *replacing* any previous one. - Tk's ``bind`` replaces an existing binding for the same sequence, so - re-wiring after an output comes back is idempotent rather than - stacking duplicate handlers. + Only the default differs from :meth:`gatelock.WidgetGroup.bind`, and + it is inverted deliberately. ``MealGate._wire_events`` runs once per + ``build_surface`` **and** again on rebuild, so with an additive bind + every output change would stack a second handler on every surviving + widget -- and ```` would submit the meal twice. Replacing + makes re-wiring idempotent, which is what this gate needs; gatelock + defaults the other way because its callers bind once and must not + clobber a handler someone else set. """ - for widget in self._widgets: - with contextlib.suppress(tk.TclError): - widget.bind(sequence, callback) - - def focus_set(self) -> None: - """Focus the first copy that still exists. - - Focus is singular by nature: there is one keyboard, so this picks the - first live copy rather than fanning out. - """ - for widget in self._widgets: - with contextlib.suppress(tk.TclError): - widget.focus_set() - return - - def focus_force(self) -> None: - """Force focus onto the first copy that still exists.""" - for widget in self._widgets: - with contextlib.suppress(tk.TclError): - widget.focus_force() - return + # Spelled out rather than delegated: an inherited signature with a + # flipped default is invisible at the call site, and this one is + # load-bearing enough to be worth reading in place. + self._each(lambda widget: widget.bind(sequence, func, add="+" if add else "")) class EntryGroup(WidgetGroup[tk.Entry]): @@ -131,22 +130,22 @@ class TextGroup(WidgetGroup[tk.Text]): def get(self, start: str, end: str) -> str: """Return the content of whichever copy was actually typed into.""" - for widget in self._widgets: + for widget in self: with contextlib.suppress(tk.TclError): content = widget.get(start, end) if content.strip(): return str(content) - return str(self._widgets[0].get(start, end)) + return str(self.first.get(start, end)) def delete(self, start: str, end: str) -> None: """Clear every copy.""" - for widget in self._widgets: + for widget in self: with contextlib.suppress(tk.TclError): widget.delete(start, end) def insert(self, index: str, value: str) -> None: """Write the same content into every copy.""" - for widget in self._widgets: + for widget in self: with contextlib.suppress(tk.TclError): widget.insert(index, value) @@ -160,13 +159,13 @@ class ListboxGroup(WidgetGroup[tk.Listbox]): def delete(self, first: int | str, last: int | str | None = None) -> None: """Clear every copy.""" - for widget in self._widgets: + for widget in self: with contextlib.suppress(tk.TclError): widget.delete(first, last) def insert(self, index: int | str, *values: str) -> None: """Append the same entries to every copy.""" - for widget in self._widgets: + for widget in self: with contextlib.suppress(tk.TclError): widget.insert(index, *values) @@ -177,7 +176,7 @@ class ListboxGroup(WidgetGroup[tk.Listbox]): of them; a *mouse* pick lands on one copy, which is what ``curselection`` reads back. """ - for widget in self._widgets: + for widget in self: with contextlib.suppress(tk.TclError): if last is None: widget.selection_set(first) @@ -186,7 +185,7 @@ class ListboxGroup(WidgetGroup[tk.Listbox]): def selection_clear(self, first: int, last: int | None = None) -> None: """Clear the selection on every copy.""" - for widget in self._widgets: + for widget in self: with contextlib.suppress(tk.TclError): if last is None: widget.selection_clear(first) @@ -195,7 +194,7 @@ class ListboxGroup(WidgetGroup[tk.Listbox]): def curselection(self) -> tuple[int, ...]: """Return the selection from whichever copy the user clicked.""" - for widget in self._widgets: + for widget in self: with contextlib.suppress(tk.TclError): selection = widget.curselection() if selection: @@ -204,7 +203,7 @@ class ListboxGroup(WidgetGroup[tk.Listbox]): def size(self) -> int: """Number of rows; identical on every copy, so read the primary.""" - return int(self._widgets[0].size()) + return int(self.first.size()) class MacroGroup: diff --git a/diet_guard/_gatelock_ui.py b/diet_guard/_gatelock_ui.py index 784807d..9062697 100644 --- a/diet_guard/_gatelock_ui.py +++ b/diet_guard/_gatelock_ui.py @@ -19,9 +19,14 @@ from __future__ import annotations import tkinter as tk from typing import TYPE_CHECKING -from gatelock import LockConfig, ScrollableSurface, escape_text_tab_trap +from gatelock import ( + ButtonStyle, + LockConfig, + ScrollableSurface, + escape_text_tab_trap, + make_button, +) -from diet_guard._gatelock_buttons import make_button from diet_guard._gatelock_spacing import SM, XS from diet_guard._gatelock_typography import ( BODY, @@ -418,19 +423,19 @@ def build_layout( make_button( body, - text="Log & Continue", - variant="primary", - command=callbacks.on_submit, + _COLORS, + "Log & Continue", + callbacks.on_submit, ).pack(pady=(XS, XS)) # Manual pull for a meal already logged on another device (the phone) but # not yet propagated to this machine -- saves re-typing it to unlock. make_button( body, - text="⟳ Fetch from sync", - variant="secondary", - command=callbacks.on_fetch_sync, - bold=False, + _COLORS, + "⟳ Fetch from sync", + callbacks.on_fetch_sync, + ButtonStyle(variant="secondary", bold=False), ).pack(pady=(0, XS)) status_label = tk.Label( @@ -454,10 +459,10 @@ def build_layout( if demo_mode: make_button( root, - text="✕ Close Demo", - variant="danger", - command=callbacks.on_close, - bold=False, + _COLORS, + "✕ Close Demo", + callbacks.on_close, + ButtonStyle(variant="danger", bold=False), ).place(x=10, y=10) return GateWidgets( diff --git a/diet_guard/tests/conftest.py b/diet_guard/tests/conftest.py index 6fb527a..f320f90 100644 --- a/diet_guard/tests/conftest.py +++ b/diet_guard/tests/conftest.py @@ -26,11 +26,11 @@ from unittest.mock import MagicMock, patch from gatelock import Output, OutputRect from gatelock import _scrollable as _gatelock_scrollable +from gatelock import widgets as _gatelock_widgets import pytest from diet_guard import ( _gatelock, - _gatelock_buttons, _gatelock_calendar, _gatelock_calendar_ui, _gatelock_core, @@ -238,7 +238,9 @@ def _hmac_key(tmp_path: Path) -> Iterator[None]: _GATE_TK_MODULES = ( _gatelock, - _gatelock_buttons, + # The gate's buttons are built by gatelock now, so its widget module + # needs the fake tk too -- otherwise the gate tests open real windows. + _gatelock_widgets, _gatelock_calendar, _gatelock_calendar_ui, _gatelock_core, diff --git a/diet_guard/tests/test_gatelock_fanout.py b/diet_guard/tests/test_gatelock_fanout.py index 381ce3e..a6a0dd6 100644 --- a/diet_guard/tests/test_gatelock_fanout.py +++ b/diet_guard/tests/test_gatelock_fanout.py @@ -163,7 +163,40 @@ class TestGroupsInIsolation: WidgetGroup([dead, alive]).bind("", handler) - alive.bind.assert_called_once_with("", handler) + # ``add=""`` is Tk's own spelling for "replace any existing binding", + # which is what this gate needs: _wire_events runs once per surface + # *and* again on rebuild, so an additive bind would stack a second + # handler on every surviving widget and submit the meal twice. + alive.bind.assert_called_once_with("", handler, add="") + + def test_first_on_an_empty_group_raises(self) -> None: + """The gate always builds a copy before reading one. + + gatelock's ``first`` returns ``None`` for the zero-output case, which + is right for a library. Here it is re-narrowed to the widget type so + the specialized groups can read it without a None check, so the empty + case has to be an explicit error rather than a ``None`` that would + surface later as an ``AttributeError`` somewhere unrelated. + """ + with pytest.raises(IndexError, match="empty"): + _ = WidgetGroup([]).first + + def test_rebinding_replaces_rather_than_stacking(self) -> None: + """``_wire_events`` runs again on rebuild; it must stay idempotent. + + gatelock's own ``WidgetGroup.bind`` defaults to *additive*, because its + callers bind once and must not clobber someone else's handler. This + gate is the other case: every re-wire would otherwise leave a second + ```` handler attached and log the meal twice. + """ + widget = MagicMock() + handler = MagicMock() + group = WidgetGroup([widget]) + + group.bind("", handler) + group.bind("", handler) + + assert [call.kwargs["add"] for call in widget.bind.call_args_list] == ["", ""] def test_focus_when_every_copy_is_gone(self) -> None: """All surfaces destroyed: nothing to focus, and no exception.""" diff --git a/diet_guard/tests/test_gatelock_keyboard.py b/diet_guard/tests/test_gatelock_keyboard.py index 2a84f90..1e7d1dc 100644 --- a/diet_guard/tests/test_gatelock_keyboard.py +++ b/diet_guard/tests/test_gatelock_keyboard.py @@ -9,76 +9,27 @@ which is why these are correctness tests rather than polish. The reachability and height side of the same requirement is covered by ``measure_gate_layout.py``; the real focus-ring walk lives in ``probe_gate_keyboard.py``. + +The " activates a button" check moved to gatelock with the button +itself (``gatelock/tests/test_widgets.py``), where it runs against real Tk +rather than a fake widget that can only report the binding it was handed. """ from __future__ import annotations import tkinter as tk -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING import pytest -from diet_guard import _gatelock_buttons, _gatelock_typography, _gatelock_ui +from diet_guard import _gatelock_typography, _gatelock_ui from diet_guard._gatelock_ui import UNIT_GRAMS, UNIT_ITEMS from diet_guard.tests import _gate_keyboard_probe -from diet_guard.tests.conftest import _FAKE_TK, FakeRadiobutton if TYPE_CHECKING: from diet_guard._gatelock import MealGate -class TestButtonReturnKey: - """Enter must activate a button; Tk binds only Space on X11.""" - - def test_return_invokes_the_button(self, monkeypatch: pytest.MonkeyPatch) -> None: - """The handler invokes the button and swallows the event.""" - monkeypatch.setattr(_gatelock_buttons, "tk", _FAKE_TK) - fired: list[str] = [] - bound: dict[str, Any] = {} - - class _Button(FakeRadiobutton): - # Signature mirrors FakeWidget.bind so the override type-checks. - def bind(self, *args: object, **_kwargs: object) -> None: - bound[str(args[0])] = args[1] - - def invoke(self) -> None: - fired.append("hit") - - monkeypatch.setattr(_FAKE_TK, "Button", _Button) - _gatelock_buttons.make_button( - _FAKE_TK.Frame(), - text="Log & Continue", - variant="primary", - command=lambda: None, - ) - - assert bound[""](None) == "break" - assert fired == ["hit"] - - def test_keypad_enter_is_bound_too(self, monkeypatch: pytest.MonkeyPatch) -> None: - """The numeric keypad's Enter is a separate keysym from Return.""" - monkeypatch.setattr(_gatelock_buttons, "tk", _FAKE_TK) - bound: dict[str, Any] = {} - - class _Button(FakeRadiobutton): - # Signature mirrors FakeWidget.bind so the override type-checks. - def bind(self, *args: object, **_kwargs: object) -> None: - bound[str(args[0])] = args[1] - - def invoke(self) -> None: - pass - - monkeypatch.setattr(_FAKE_TK, "Button", _Button) - _gatelock_buttons.make_button( - _FAKE_TK.Frame(), - text="Close", - variant="danger", - command=lambda: None, - ) - - assert "" in bound - - class TestUnitSelectorIsKeyboardReachable: """The grams/items selector must not be a ``tk.OptionMenu``. diff --git a/pyproject.toml b/pyproject.toml index ff5ce26..c5f6110 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ version = "1.0.0" description = "Tkinter/systemd log-to-unlock meal gate with calorie/macro budget tracking" requires-python = ">=3.10" dependencies = [ - "gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock", + "gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.5.0#subdirectory=gatelock", "crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync", "requests>=2.31.0", ] diff --git a/requirements.txt b/requirements.txt index 05d4340..4540b2b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ bandit>=1.7.0 codespell>=2.2.0 coverage>=7.4.0 crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync -gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock +gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.5.0#subdirectory=gatelock mcp>=2,<3 mypy>=1.8.0 pre-commit>=3.6.0