From ba549f8f05c44abbd121ca7b0e5fcc59d3ee0464 Mon Sep 17 00:00:00 2001 From: Krzysztof Rudnicki Date: Mon, 3 Aug 2026 15:50:34 +0200 Subject: [PATCH] gatelock 0.4.0: the viewport moves for the user and nobody else On 2026-08-03 the screen locker's lock screen scrolled to mid-content and back with nobody touching the keyboard, on a 1366x768 panel. Measured under Xvfb with the real app: one programmatic focus_set() produced three scrolls, frac 0.000 -> 0.436 -> 0.145. Two causes, both here: * ScrollableSurface followed every . The apps focus a widget on every repaint (and gatelock re-asserts focus from its recovery tick), and Tk delivers to the focused widget's whole ancestor chain when the toplevel takes X focus -- so each repaint aimed a scroll at containers taller than the screen. Now a focus move scrolls only when it followed a real keypress, or a click on the widget that took focus, and only for a leaf. Tab onto a clipped field still reveals it; nothing else moves. * finalize()'s reset-to-top cancelled the centring, because centring moved the canvas item down and the region's top edge with it. The region is now pinned to the viewport whenever the content fits, so the view cannot move at all, the scrollbar is taken down, and centring survives the repaint. Content that does not fit is a layout defect, not a mode to live in, so finalize() reports it at warning and gatelock.measure_fit() lets each app gate on it before shipping (scripts/verify_screen_fits.py in all four). Also: LockConfig's type/space scales now compact below 768px (_density), which is where a screen has no room to be at design-system size and no scrollbar to make up for it. 768 and up is verified to fit at full size, so it is never compacted. Tests: _scrollable had none; the regression is now stated as code (programmatic focus must not move yview; Tab must). 323 pass, coverage 95.38% -> 99.44%. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BsXnTHyuuCUZsqetTaajcF --- gatelock/gatelock/__init__.py | 14 +- gatelock/gatelock/_density.py | 196 +++++++++++++++ gatelock/gatelock/_fitcheck.py | 138 ++++++++++ gatelock/gatelock/_scrollable.py | 262 ++++++++++++++++--- gatelock/gatelock/_window.py | 18 +- gatelock/gatelock/tests/test_density.py | 187 ++++++++++++++ gatelock/gatelock/tests/test_fitcheck.py | 80 ++++++ gatelock/gatelock/tests/test_keyboard.py | 102 ++++++++ gatelock/gatelock/tests/test_scrollable.py | 278 +++++++++++++++++++++ gatelock/pyproject.toml | 2 +- 10 files changed, 1232 insertions(+), 45 deletions(-) create mode 100644 gatelock/gatelock/_density.py create mode 100644 gatelock/gatelock/_fitcheck.py create mode 100644 gatelock/gatelock/tests/test_density.py create mode 100644 gatelock/gatelock/tests/test_fitcheck.py create mode 100644 gatelock/gatelock/tests/test_keyboard.py create mode 100644 gatelock/gatelock/tests/test_scrollable.py diff --git a/gatelock/gatelock/__init__.py b/gatelock/gatelock/__init__.py index 5f144c4..ebd1307 100644 --- a/gatelock/gatelock/__init__.py +++ b/gatelock/gatelock/__init__.py @@ -24,6 +24,7 @@ from gatelock._escape import ( EscapePolicy, EscapeTracker, ) +from gatelock._fitcheck import FitResult, measure_fit, report_fit from gatelock._guards import assert_not_under_pytest, wait_for_x_server from gatelock._keyboard import bind_activate, bind_cancel, escape_text_tab_trap from gatelock._outputs import ( @@ -47,7 +48,13 @@ from gatelock._surfaces import ( needs_backdrop_root, ) from gatelock._vt import disable_vt_switching, restore_vt_switching -from gatelock._window import LockConfig, LockWindow, LockWindowHooks +from gatelock._window import ( + LockConfig, + LockWindow, + LockWindowHooks, + SpaceStep, + TypeRole, +) __all__ = [ "RANK_DIET_GUARD", @@ -60,6 +67,7 @@ __all__ = [ "EscapeHistory", "EscapePolicy", "EscapeTracker", + "FitResult", "GateRoot", "LockConfig", "LockWindow", @@ -73,10 +81,12 @@ __all__ = [ "RecoveryLoop", "RecoveryReport", "ScrollableSurface", + "SpaceStep", "SurfaceBuilder", "SurfaceDelta", "SurfaceInfo", "SurfaceSet", + "TypeRole", "assert_not_under_pytest", "bind_activate", "bind_cancel", @@ -85,9 +95,11 @@ __all__ = [ "enumerate_outputs", "escape_text_tab_trap", "grab_strength", + "measure_fit", "mirror_text_widgets", "needs_backdrop_root", "parse_xrandr_query", + "report_fit", "restore_vt_switching", "wait_for_x_server", ] diff --git a/gatelock/gatelock/_density.py b/gatelock/gatelock/_density.py new file mode 100644 index 0000000..394eebe --- /dev/null +++ b/gatelock/gatelock/_density.py @@ -0,0 +1,196 @@ +"""Screen-height-derived compaction for the design-system type/space scales. + +The token scales in :class:`~gatelock._window.LockConfig` are authored in +pixels, and every screen in every app fits a 1366x768 panel at full size -- +which is checked, per app, by ``scripts/verify_screen_fits.py``. So 768px and +up is *not* compacted: shrinking type a screen has room for buys nothing. + +Below 768px there is no such evidence, and a lock surface cannot solve +"content taller than the screen" by scrolling -- it is a full-screen, grabbed +window whose whole job is to present one screen's worth of decision. There the +scale is compacted as a best-effort so those panels degrade gracefully rather +than hiding a submit button. + +This module resolves one process-wide factor from the display height and +applies it inside ``type_px()``/``space()``, which is what lets it reach the +module-level ``LockConfig()`` constants the apps build at import time, before +any Tk root exists. + +The factor is deliberately *not* a continuous function of height: two +breakpoints keep the rendering predictable and testable, and keep every +supported screen looking exactly as the design system specifies. +""" + +from __future__ import annotations + +from contextlib import contextmanager +import logging +import os +import tkinter as tk +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Iterator + +_logger = logging.getLogger(__name__) + +# Set to a float to force a factor -- used by the fit-check harness to render a +# target resolution's layout on whatever display it happens to run on, and +# available as a user escape hatch. +ENV_OVERRIDE = "GATELOCK_UI_DENSITY" + +# (minimum screen height, factor). First match wins, tallest first. +# 768 and up is verified to fit at full size, so it is never compacted. +# Anything shorter is best-effort: compacted, but not covered by any app's +# fit check. +_BREAKPOINTS: tuple[tuple[int, float], ...] = ( + (768, 1.0), + (0, 0.7), +) + +# Nothing may shrink below these, whatever the factor says: text stops being +# readable and touching gaps stop reading as separation. +_MIN_TYPE_PX = 11 +_MIN_SPACE_PX = 2 + +# One-slot cache. A dict rather than a module global with ``global`` writes, +# so the module has no rebindable state and needs no lint escape hatch. +_cache: dict[str, float] = {} + + +def factor_for_height(height_px: int) -> float: + """Return the compaction factor for a display of ``height_px``.""" + for minimum, factor in _BREAKPOINTS: + if height_px >= minimum: + return factor + # Unreachable: the last breakpoint matches every non-negative height. A + # negative height means Tk reported nonsense; treat it as "do not compact" + # rather than silently picking the smallest scale. + _logger.warning( + "screen height %dpx is not a real height; leaving the type scale uncompacted", + height_px, + ) + return 1.0 + + +def _usable_height(raw: object) -> int | None: + """Return ``raw`` if it is a real pixel height, else None with a warning. + + Deliberately a type check rather than ``int(raw)`` in a ``try``: the + consuming apps replace the whole ``tkinter`` module with a fake in their + test suites, so ``winfo_screenheight()`` can return a mock that neither + converts nor raises anything catchable -- and an ``except`` clause naming + a mocked ``tk.TclError`` is itself a TypeError. + """ + if isinstance(raw, int) and not isinstance(raw, bool): + return raw + _logger.warning( + "screen height came back as %r, not a number; the type scale stays " + "uncompacted, which can overflow a short screen", + raw, + ) + return None + + +def _screen_height() -> int | None: + """Return the display height in px, or None when there is no display.""" + root = getattr(tk, "_default_root", None) + if root is not None: + return _usable_height(root.winfo_screenheight()) + try: + probe = tk.Tk() + except tk.TclError as exc: + _logger.warning( + "no display available to size the type scale (%s); leaving it " + "uncompacted -- expected in headless tests, a bug anywhere else", + exc, + ) + return None + try: + return _usable_height(probe.winfo_screenheight()) + finally: + probe.destroy() + + +def density() -> float: + """Return the process-wide compaction factor, resolving it once. + + Cached because it is read for every font and every pad on every repaint, + and because a lock surface's display cannot change under it without the + surfaces being rebuilt anyway. + """ + cached = _cache.get("factor") + if cached is not None: + return cached + override = _forced_factor() + if override is not None: + _cache["factor"] = override + _logger.info("type scale forced to %.2f by %s", override, ENV_OVERRIDE) + return override + height = _screen_height() + factor = 1.0 if height is None else factor_for_height(height) + _cache["factor"] = factor + if factor != 1.0: + _logger.info( + "compacting the type/space scale to %.2f for a %dpx screen", + factor, + height, + ) + return factor + + +def _forced_factor() -> float | None: + """Return the env-forced factor, or None when unset or unusable.""" + override = os.environ.get(ENV_OVERRIDE) + if not override: + return None + try: + return float(override) + except ValueError: + _logger.warning( + "%s=%r is not a number; ignoring it and sizing from the screen instead", + ENV_OVERRIDE, + override, + ) + return None + + +def reset_cache() -> None: + """Forget the resolved factor. For tests and the fit-check harness.""" + _cache.clear() + + +def scale_type(px: int) -> int: + """Compact a type-scale value, never below :data:`_MIN_TYPE_PX`.""" + return max(_MIN_TYPE_PX, round(px * density())) + + +def scale_space(px: int) -> int: + """Compact a spacing value, never below :data:`_MIN_SPACE_PX`. + + A zero stays zero: "no gap" is a deliberate choice, not a small gap. + """ + if px <= 0: + return px + return max(_MIN_SPACE_PX, round(px * density())) + + +@contextmanager +def forced(factor: float) -> Iterator[None]: + """Force the compaction factor for the duration of the block. + + For the fit-check harness and tests: it lets a 1080p workstation render + exactly what a 768px panel would, which is the whole point of checking + the fit somewhere other than the target machine. + """ + previous = os.environ.get(ENV_OVERRIDE) + os.environ[ENV_OVERRIDE] = str(factor) + reset_cache() + try: + yield + finally: + if previous is None: + os.environ.pop(ENV_OVERRIDE, None) + else: + os.environ[ENV_OVERRIDE] = previous + reset_cache() diff --git a/gatelock/gatelock/_fitcheck.py b/gatelock/gatelock/_fitcheck.py new file mode 100644 index 0000000..e5b2196 --- /dev/null +++ b/gatelock/gatelock/_fitcheck.py @@ -0,0 +1,138 @@ +"""Measure whether a lock screen fits its surface, at a chosen resolution. + +A lock surface is fullscreen, grabbed and (on the production path) VT-locked: +whatever does not fit is content the user cannot reach except by scrolling a +screen that should never have needed scrolling. :meth:`ScrollableSurface. +finalize` reports overflow at runtime; this module is the half that runs +*before* shipping, so a consuming app can assert in CI that every one of its +screens fits on the smallest display it supports. + +Usage from an app's ``scripts/verify_screen_fits.py``:: + + from gatelock import measure_fit + + result = measure_fit("retry", paint_retry, width=1366, height=768) + if not result.fits: + ... + +The measurement runs against real Tk on a real (throwaway) display -- widget +heights depend on the font engine, so nothing short of rendering can answer +this honestly. +""" + +from __future__ import annotations + +from dataclasses import dataclass +import logging +import tkinter as tk +from typing import TYPE_CHECKING + +from gatelock import _density +from gatelock._scrollable import ScrollableSurface +from gatelock._window import LockConfig + +if TYPE_CHECKING: + from collections.abc import Callable + +_logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class FitResult: + """What one screen measured at one resolution.""" + + name: str + width: int + height: int + content_px: int + viewport_px: int + + @property + def overflow_px(self) -> int: + """Pixels of content past the bottom of the viewport (0 when it fits).""" + return max(0, self.content_px - self.viewport_px) + + @property + def fits(self) -> bool: + """Whether the whole screen is visible without scrolling.""" + return self.overflow_px == 0 + + def describe(self) -> str: + """One line for a verification script's output.""" + verdict = "fits" if self.fits else f"OVERFLOWS by {self.overflow_px}px" + return ( + f"{self.name} @ {self.width}x{self.height}: " + f"{self.content_px}px in {self.viewport_px}px -- {verdict}" + ) + + +def measure_fit( + name: str, + build: Callable[[ScrollableSurface], tk.Misc | None], + *, + width: int, + height: int, + config: LockConfig | None = None, +) -> FitResult: + """Build one screen at ``width``x``height`` and measure its height. + + The type/space scale is forced to whatever a display of ``height`` would + resolve to, so the result is what the target machine renders rather than + what the machine running the check renders. + + Args: + name: Screen name, for the report. + build: Paints the screen. Receives the surface; build into + ``surface.content``. May return the widget whose height *is* the + screen's height -- needed for a layout that ``place``s a centred + frame, since ``place`` does not propagate a child's size to its + parent and the parent would otherwise measure as empty. + width: Surface width in px. + height: Surface height in px. + config: Token source. Defaults to a stock :class:`LockConfig`. + + Returns: + The measurement. Never raises for a screen that merely overflows -- + overflow is the thing being reported, not an error. + """ + with _density.forced(_density.factor_for_height(height)): + return _measure(name, build, width, height, config) + + +def _measure( + name: str, + build: Callable[[ScrollableSurface], tk.Misc | None], + width: int, + height: int, + config: LockConfig | None, +) -> FitResult: + """Render the screen once and read its height back.""" + root = tk.Tk() + try: + root.geometry(f"{width}x{height}+0+0") + root.update_idletasks() + surface = ScrollableSurface(root, config or LockConfig(), center_when_fits=True) + surface.container.place(relx=0, rely=0, relwidth=1, relheight=1) + root.update() + measured = build(surface) + surface.finalize() + root.update() + return FitResult( + name=name, + width=width, + height=height, + content_px=(measured or surface.content).winfo_reqheight(), + viewport_px=surface.canvas.winfo_height(), + ) + finally: + root.destroy() + + +def report_fit(results: list[FitResult]) -> bool: + """Log every measurement and return whether all of them fit.""" + for result in sorted(results, key=lambda r: (-r.overflow_px, r.name)): + if result.fits: + _logger.info(" %s", result.describe()) + else: + _logger.error(" %s", result.describe()) + return all(result.fits for result in results) diff --git a/gatelock/gatelock/_scrollable.py b/gatelock/gatelock/_scrollable.py index 70cd7ce..334691f 100644 --- a/gatelock/gatelock/_scrollable.py +++ b/gatelock/gatelock/_scrollable.py @@ -24,17 +24,29 @@ oversights in any one app: child, so every scrolled-out-of-view field stays ``winfo viewable`` and stays in the tab chain, while Tk never scrolls to follow focus. Tabbing therefore lands on fields the user cannot see and cannot bring into view. - :meth:`ScrollableSurface.track_focus` fixes that by scrolling the focused - widget into view. + :meth:`ScrollableSurface.scroll_into_view` fixes that, but only for focus + the *user* moved. Wheel scrolling is bound too, but as an addition -- never as the only path. + +**Scrolling is the fallback, not the design.** A lock screen is supposed to +fit its screen; content that overflows is a layout defect, and :meth:`finalize` +logs it at ``warning`` so it gets fixed rather than lived with. While the +content fits -- the normal case -- the scroll region is pinned to the viewport +and the scrollbar is hidden, so the view cannot move at all. When it does +overflow, the view still only moves in response to a keypress, a click or the +wheel: an earlier version followed *programmatic* focus too, and since the apps +re-focus a widget on every repaint, the screen scrolled itself to mid-content +and back while the user was doing nothing (reported 2026-08-03, 1366x768). """ from __future__ import annotations import logging +import time import tkinter as tk from typing import TYPE_CHECKING +import weakref if TYPE_CHECKING: from gatelock._window import LockConfig @@ -49,6 +61,14 @@ _ARROW_UNITS = 3 # Extra px kept visible around a widget scrolled into view, so a focused # field never sits flush against the viewport edge. _FOCUS_MARGIN_PX = 12 +# How long a keypress or click keeps ownership of the viewport. Focus that +# arrives within this window is treated as the user's own navigation (Tab, +# clicking a field, a button press that opens the next screen) and may scroll; +# focus set outside it is the app's own doing and must not move anything. +_USER_INPUT_WINDOW_S = 0.25 +# Tk reports 1px for a widget it has not laid out yet. Treating that as a real +# viewport height would report every not-yet-mapped surface as overflowing. +_UNMAPPED_VIEWPORT_PX = 1 class ScrollableSurface: @@ -76,9 +96,12 @@ class ScrollableSurface: Args: parent: The surface (or notebook tab) to build inside. config: Token source for colors and the focus ring. - show_scrollbar: Draw a scrollbar alongside the canvas. Even when - False the content stays keyboard-scrollable; the bar is a - pointer affordance and a visual hint, not the mechanism. + show_scrollbar: Allow a scrollbar alongside the canvas. It is only + ever shown while the content actually overflows -- a bar with + nothing to scroll is chrome that says the screen is bigger than + it is. Even when False the content stays keyboard-scrollable; + the bar is a pointer affordance and a visual hint, not the + mechanism. center_when_fits: Vertically centre the content while it fits, and fall back to top-anchored scrolling once it does not. This lets a viewport replace a ``place``-centred frame without changing @@ -114,7 +137,9 @@ class ScrollableSurface: # second way to scroll. **config.focus_kwargs(), ) - self._scrollbar.pack(side="right", fill="y") + # Deliberately not packed here: _apply_fit_state() packs it the + # moment the content is found to overflow, and nothing else should + # put a scrollbar on a screen that fits. self.canvas.configure(yscrollcommand=self._scrollbar.set) self.canvas.pack(side="left", fill="both", expand=True) @@ -126,12 +151,23 @@ class ScrollableSurface: ) self.content.bind("", self._on_content_resize, add="+") self.canvas.bind("", self._on_canvas_resize, add="+") + # `content` is bound once, here, because it outlives every repaint -- + # binding it in finalize() instead piled up a handler per repaint. + # Its descendants are bound in finalize(), each exactly once: Tk sends + # to the ancestor chain only when the *toplevel* takes X + # focus, not when focus moves between two fields inside it, so a + # binding on `content` alone would miss every Tab. + self.content.bind("", self._on_descendant_focus, add="+") + self._focus_bound: weakref.WeakSet[tk.Misc] = weakref.WeakSet() + self._last_user_input = 0.0 + self._clicked: tk.Misc | None = None + self._overflowing: bool | None = None self._bind_scroll_keys() + self._bind_user_input() def _on_content_resize(self, _event: tk.Event) -> None: """Keep the scroll region matched to the content's real size.""" - self.canvas.configure(scrollregion=self.canvas.bbox("all")) - self._reposition() + self._apply_fit_state() def _on_canvas_resize(self, event: tk.Event) -> None: """Center the content horizontally and match the viewport width. @@ -140,16 +176,114 @@ class ScrollableSurface: behave the same as they did before the viewport was introduced. """ self.canvas.itemconfigure(self._content_id, width=event.width) - self._reposition() + self._apply_fit_state() - def _reposition(self) -> None: - """Place the content window, centring vertically only while it fits.""" - width = self.canvas.winfo_width() - top = 0 - if self._center_when_fits: - spare = self.canvas.winfo_height() - self.content.winfo_reqheight() - top = max(0, spare // 2) - self.canvas.coords(self._content_id, width / 2, top) + def _apply_fit_state(self) -> None: + """Switch between "fits" and "overflows" layout. + + Fitting is the normal, intended state: the scroll region is pinned to + the viewport so *nothing* can scroll, the content is centred, and the + scrollbar is taken down because there is nothing for it to do. + + Scrolling only exists as the fallback for content that genuinely does + not fit, and that is a defect worth reporting (see :meth:`finalize`), + not a layout mode to design for. + + Pinning the region is also what makes centring survive. It used to be + cancelled: content was centred by moving the canvas *item* down, which + moved the scroll region's top edge down with it, and the reset in + :meth:`finalize` then scrolled to that top edge -- putting the content + back flush against the screen's top edge on every repaint. + """ + view_h = self.canvas.winfo_height() + view_w = self.canvas.winfo_width() + wanted_h = self.content.winfo_reqheight() + overflowing = wanted_h > view_h + if overflowing: + self.canvas.coords(self._content_id, view_w / 2, 0) + self.canvas.configure(scrollregion=self.canvas.bbox("all")) + else: + top = max(0, (view_h - wanted_h) // 2) if self._center_when_fits else 0 + self.canvas.coords(self._content_id, view_w / 2, top) + # Region == the *scrollable* area, so yview is exactly (0.0, 1.0) + # and nothing -- key, wheel or scrollbar -- can move it. The focus + # ring is drawn outside that area, so it has to come off both + # dimensions or the region is a hair taller than the viewport and + # a page-down still nudges the screen. + inset = 2 * self._border_px() + self.canvas.configure( + scrollregion=(0, 0, max(1, view_w - inset), max(1, view_h - inset)) + ) + if overflowing != self._overflowing: + self._overflowing = overflowing + self._show_scrollbar(visible=overflowing) + + def _border_px(self) -> int: + """Width of the canvas's own edge: focus ring plus border.""" + return int(self.canvas.cget("highlightthickness")) + int( + self.canvas.cget("borderwidth") + ) + + def _show_scrollbar(self, *, visible: bool) -> None: + """Show the scrollbar only while there is something to scroll.""" + if self._scrollbar is None: + return + if visible: + # Re-packed before the canvas would put it on the wrong side, so + # the canvas is re-packed after it to keep left/right ordering. + self._scrollbar.pack(side="right", fill="y") + self.canvas.pack_forget() + self.canvas.pack(side="left", fill="both", expand=True) + else: + self._scrollbar.pack_forget() + + def _bind_user_input(self) -> None: + """Record when the user last pressed a key or a mouse button. + + Focus-following scrolling is gated on this: the app focuses widgets + programmatically on every repaint (and gatelock re-asserts focus from + its recovery tick), and a viewport that follows *those* moves the + screen under a user who has touched nothing. + """ + targets: list[tk.Misc] = [self.canvas, self.content] + try: + targets.append(self.canvas.winfo_toplevel()) + except (tk.TclError, KeyError, AttributeError): + _logger.warning( + "no toplevel for scroll viewport; user-input tracking is " + "bound to the viewport only, so focus-following scrolling " + "stays off unless the viewport itself saw the event" + ) + for target in targets: + target.bind("", self._note_key, add="+") + target.bind("