Redesign manual-workout form: two-column grid, drop phone-only fields
Some checks are pending
pre-commit / pre-commit (push) Waiting to run
Tests / test (3.10) (push) Waiting to run
Tests / test (3.11) (push) Waiting to run
Tests / test (3.12) (push) Waiting to run

The manual-workout evidence form (shown on the lock screen and the status
window via the shared ManualWorkoutDialogMixin) was a cramped, narrow
centered column of tiny inputs with large empty margins, and included two
fields that make no sense to fill from a locked PC.

- Remove the vestigial "Google Maps link" (location_maps_link) and
  "Screenshot path" (proof_screenshot_path) fields from the form, the
  ManualWorkoutDraft dataclass, and build_entry(). Both were written to
  workout_log.json but never read by any code. The now-empty "Evidence"
  section is gone too. reservation_phone is kept.
- Lay the form out as a two-column grid that fills ~90% of the screen
  (the fullscreen container was shrink-wrapping to its widest child) with
  larger labels/inputs; reflection boxes span full width. Activity-specific
  fields now sit directly under their section header.
- Extract the low-level field/grid builders into a new
  ManualWorkoutFormWidgetsMixin (_manual_workout_widgets.py) so the dialog
  module stays under the 400-line cap; the dialog mixin inherits it, so the
  host classes need no change.

Tests updated for the removed fields and the pack->grid sport swap; added a
unit test for the grid cursor's odd-row bump. Full suite green at 100%.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PM3auNTifaVgcXWDizZrjH
This commit is contained in:
Krzysztof Rudnicki 2026-07-13 10:09:18 +02:00
parent 5390f2043c
commit 60d8c3361b
6 changed files with 264 additions and 134 deletions

View File

@ -142,9 +142,7 @@ class ManualWorkoutDraft:
went_well: str
to_improve: str
overall_feeling: str
location_maps_link: str = "" # optional — awkward to fill from a locked PC
reservation_phone: str = ""
proof_screenshot_path: str = "" # optional — phone-oriented field
techniques_practiced: str = ""
warm_up_minutes: str = ""
pain_or_injury: str = "none"
@ -278,11 +276,9 @@ def build_entry(draft: ManualWorkoutDraft) -> dict[str, object]:
"end_time": draft.end_time.strip(),
"duration_minutes": f"{duration:.1f}" if duration is not None else "",
"location_name": draft.location_name.strip(),
"location_maps_link": draft.location_maps_link.strip(),
"transport_method": draft.transport_method.strip(),
"cost": draft.cost.strip(),
"reservation_phone": draft.reservation_phone.strip(),
"proof_screenshot_path": draft.proof_screenshot_path.strip(),
"rpe": int(draft.rpe),
"techniques_practiced": draft.techniques_practiced.strip(),
"warm_up_minutes": draft.warm_up_minutes.strip(),

View File

@ -23,10 +23,8 @@ from screen_locker import _manual_workout
from screen_locker._constants import (
MANUAL_WORKOUT_DESCRIPTION_MIN_CHARS,
MANUAL_WORKOUT_REFLECTION_MIN_CHARS,
MANUAL_WORKOUT_RPE_MAX,
MANUAL_WORKOUT_RPE_MIN,
)
from screen_locker._ui_widgets import disable_paste
from screen_locker._manual_workout_widgets import ManualWorkoutFormWidgetsMixin
_logger = logging.getLogger(__name__)
@ -35,7 +33,7 @@ _SPORT_LABEL_TO_CODE = {
}
class ManualWorkoutDialogMixin:
class ManualWorkoutDialogMixin(ManualWorkoutFormWidgetsMixin):
"""Renders the manual-workout evidence form and handles submission."""
def _show_manual_workout_form(self) -> None:
@ -60,22 +58,53 @@ class ManualWorkoutDialogMixin:
self._text(_manual_workout.budget_summary(self.log_file), color="#88ccff")
self._build_manual_workout_form()
def _build_manual_workout_form(self) -> None:
"""Build the scrollable evidence form and submit/back buttons."""
def _mw_scrollable_form(self) -> tk.Frame:
"""Create the scrollable canvas + inner two-column form frame.
The canvas is given a concrete size from the toplevel so the fullscreen
lock screen's centered container can't shrink-wrap to a narrow column
(leaving huge empty margins); the ``<Configure>`` binding then stretches
the inner form to the viewport so its two columns split the full width.
Harmless on StatusWindow, whose container already fills its window.
"""
outer = tk.Frame(self.container, bg="#1a1a1a")
outer.pack(fill="both", expand=True, pady=10)
canvas = tk.Canvas(outer, bg="#1a1a1a", highlightthickness=0)
scrollbar = tk.Scrollbar(outer, orient="vertical", command=canvas.yview)
form = tk.Frame(canvas, bg="#1a1a1a")
form.grid_columnconfigure(0, weight=1, uniform="mw")
form.grid_columnconfigure(1, weight=1, uniform="mw")
form.bind(
"<Configure>",
lambda _e: canvas.configure(scrollregion=canvas.bbox("all")),
)
canvas.create_window((0, 0), window=form, anchor="nw")
self._mw_form_window = canvas.create_window((0, 0), window=form, anchor="nw")
canvas.bind(
"<Configure>",
lambda e: canvas.itemconfig(self._mw_form_window, width=e.width),
)
canvas.configure(yscrollcommand=scrollbar.set)
canvas.pack(side="left", fill="both", expand=True)
scrollbar.pack(side="right", fill="y")
top = self.container.winfo_toplevel()
top.update_idletasks()
canvas.configure(
width=int(top.winfo_width() * 0.9),
height=int(top.winfo_height() * 0.7),
)
return form
def _build_manual_workout_form(self) -> None:
"""Build the two-column evidence form and submit/back buttons.
Large labels/inputs and a screen-filling grid (see
``_mw_scrollable_form``) so the form is usable with a keyboard while
locked out, rather than a cramped narrow column.
"""
form = self._mw_scrollable_form()
# Independent 2-column grid cursor per master (form + sport sub-frames).
self._mw_grid_counters: dict[int, int] = {}
self._mw_vars: dict[str, tk.StringVar] = {}
self._mw_int_vars: dict[str, tk.IntVar] = {}
self._mw_rpe_var = tk.IntVar(value=5)
@ -88,10 +117,6 @@ class ManualWorkoutDialogMixin:
self._mw_section(form, "Location & logistics")
self._mw_vars["location_name"] = self._mw_entry(form, "Location name:")
self._mw_vars["location_maps_link"] = self._mw_entry(
form,
"Google Maps link (optional — easier to fill from your phone):",
)
self._mw_vars["transport_method"] = self._mw_entry(
form, "How did you get there?"
)
@ -100,16 +125,18 @@ class ManualWorkoutDialogMixin:
form, "Reservation phone number (if booked by phone, optional):"
)
self._mw_section(form, "Evidence")
self._mw_vars["proof_screenshot_path"] = self._mw_entry(
form,
"Screenshot path as proof of arrangement "
"(optional — phone-oriented field):",
)
self._mw_section(form, "Activity details")
# Both sport sub-frames occupy the same full-width grid slot; only the
# selected one is shown (see _on_mw_sport_changed).
act_row = self._mw_next_full_row(form)
self._mw_tt_frame = tk.Frame(form, bg="#1a1a1a")
self._mw_other_frame = tk.Frame(form, bg="#1a1a1a")
for sport_frame in (self._mw_tt_frame, self._mw_other_frame):
sport_frame.grid_columnconfigure(0, weight=1, uniform="mwact")
sport_frame.grid_columnconfigure(1, weight=1, uniform="mwact")
sport_frame.grid(
row=act_row, column=0, columnspan=2, sticky="ew", padx=12, pady=6
)
self._build_table_tennis_fields(self._mw_tt_frame)
self._build_other_sport_fields(self._mw_other_frame)
self._mw_rpe_row(form)
@ -158,28 +185,16 @@ class ManualWorkoutDialogMixin:
width=12,
).pack(side="left", padx=10)
def _mw_section(self, parent: tk.Widget, title: str) -> None:
"""Add a section heading inside the form."""
tk.Label(
parent,
text=title,
font=("Arial", 16, "bold"),
fg="#88ccff",
bg="#1a1a1a",
anchor="w",
).pack(fill="x", pady=(15, 2))
def _mw_sport_row(self, parent: tk.Widget) -> None:
"""Add the sport-selector dropdown; swaps the activity-details section."""
self._mw_sport_var = tk.StringVar(
value=_manual_workout.SPORT_LABELS[_manual_workout.SPORT_TABLE_TENNIS]
)
row = tk.Frame(parent, bg="#1a1a1a")
row.pack(pady=5, fill="x")
tk.Label(
row,
text="Sport:",
font=("Arial", 14),
font=("Arial", 16),
fg="white",
bg="#1a1a1a",
).pack(side="left", padx=5)
@ -189,18 +204,23 @@ class ManualWorkoutDialogMixin:
*_manual_workout.SPORT_LABELS.values(),
command=self._on_mw_sport_changed,
).pack(side="left", padx=5)
self._mw_grid(parent, row, full=True)
def _on_mw_sport_changed(self, selected_label: str) -> None:
"""Show the fields for the newly-selected sport, hide the other's."""
"""Show the fields for the newly-selected sport, hide the other's.
Both frames share one grid slot; ``grid_remove`` hides a frame while
remembering its cell options so ``grid`` restores it in place.
"""
sport = _SPORT_LABEL_TO_CODE.get(
selected_label, _manual_workout.SPORT_TABLE_TENNIS
)
if sport == _manual_workout.SPORT_TABLE_TENNIS:
self._mw_other_frame.pack_forget()
self._mw_tt_frame.pack(fill="x")
self._mw_other_frame.grid_remove()
self._mw_tt_frame.grid()
else:
self._mw_tt_frame.pack_forget()
self._mw_other_frame.pack(fill="x")
self._mw_tt_frame.grid_remove()
self._mw_other_frame.grid()
def _build_table_tennis_fields(self, parent: tk.Widget) -> None:
"""Build the table-tennis-specific score/equipment fields."""
@ -224,82 +244,6 @@ class ManualWorkoutDialogMixin:
parent, "Equipment used (optional):"
)
def _mw_entry(self, parent: tk.Widget, label: str) -> tk.StringVar:
"""Add a label+entry field and return its backing StringVar."""
var = tk.StringVar()
self._add_label_entry(parent, label=label, variable=var)
return var
def _mw_int_field(
self, parent: tk.Widget, label: str, *, frm: int = 0, to: int = 99
) -> tk.IntVar:
"""Add a label + numeric Spinbox field and return its backing IntVar."""
var = tk.IntVar(value=0)
row = tk.Frame(parent, bg="#1a1a1a")
row.pack(pady=5, fill="x")
tk.Label(
row,
text=label,
font=("Arial", 14),
fg="white",
bg="#1a1a1a",
).pack(side="left", padx=5)
tk.Spinbox(
row,
from_=frm,
to=to,
textvariable=var,
width=4,
font=("Arial", 14),
).pack(side="left", padx=5)
return var
def _mw_textbox(self, parent: tk.Widget, label: str) -> tk.Text:
"""Add a label + multi-line Text field and return the widget."""
tk.Label(
parent,
text=label,
font=("Arial", 14),
fg="white",
bg="#1a1a1a",
anchor="w",
).pack(fill="x", pady=(5, 0))
text_widget = tk.Text(
parent,
width=60,
height=4,
font=("Arial", 12),
bg="#2a2a2a",
fg="white",
insertbackground="white",
)
text_widget.pack(pady=2, fill="x")
disable_paste(text_widget)
return text_widget
def _mw_rpe_row(self, parent: tk.Widget) -> None:
"""Add the RPE (rate of perceived exertion) spinbox row."""
row = tk.Frame(parent, bg="#1a1a1a")
row.pack(pady=5, fill="x")
tk.Label(
row,
text=(
f"RPE — perceived exertion "
f"({MANUAL_WORKOUT_RPE_MIN}-{MANUAL_WORKOUT_RPE_MAX}):"
),
font=("Arial", 14),
fg="white",
bg="#1a1a1a",
).pack(side="left", padx=5)
tk.Spinbox(
row,
from_=MANUAL_WORKOUT_RPE_MIN,
to=MANUAL_WORKOUT_RPE_MAX,
textvariable=self._mw_rpe_var,
width=4,
font=("Arial", 14),
).pack(side="left", padx=5)
def _current_mw_sport(self) -> str:
"""Return the internal sport code for the currently-selected label."""
return _SPORT_LABEL_TO_CODE.get(
@ -331,9 +275,7 @@ class ManualWorkoutDialogMixin:
went_well=self._mw_text_widgets["went_well"].get("1.0", "end"),
to_improve=self._mw_text_widgets["to_improve"].get("1.0", "end"),
overall_feeling=self._mw_text_widgets["overall_feeling"].get("1.0", "end"),
location_maps_link=self._mw_vars["location_maps_link"].get(),
reservation_phone=self._mw_vars["reservation_phone"].get(),
proof_screenshot_path=self._mw_vars["proof_screenshot_path"].get(),
techniques_practiced=self._mw_vars["techniques_practiced"].get(),
warm_up_minutes=self._mw_vars["warm_up_minutes"].get(),
pain_or_injury=self._mw_vars["pain_or_injury"].get() or "none",

View File

@ -0,0 +1,176 @@
"""Low-level widget/grid builders for the manual-workout evidence form.
Split out of ``_manual_workout_dialog`` so the dialog module stays under the
repo's file-length cap. These helpers build individual form cells and place
them into a two-column grid; the orchestration (which fields, in what order,
and submission) lives in ``ManualWorkoutDialogMixin``, which inherits this.
The host is expected to have initialised ``self._mw_grid_counters`` (per-master
grid cursors) and, for ``_mw_rpe_row``, ``self._mw_rpe_var``.
"""
from __future__ import annotations
import tkinter as tk
from screen_locker._constants import (
MANUAL_WORKOUT_RPE_MAX,
MANUAL_WORKOUT_RPE_MIN,
)
from screen_locker._ui_widgets import disable_paste
class ManualWorkoutFormWidgetsMixin:
"""Two-column grid placement + individual field-cell builders."""
def _mw_grid(
self, parent: tk.Widget, widget: tk.Widget, *, full: bool = False
) -> None:
"""Place ``widget`` into ``parent``'s two-column grid cursor.
A half-width item (``full=False``) fills the next cell (left, then
right); a ``full`` item starts a fresh row and spans both columns. Each
master keeps its own cursor (keyed by ``id(parent)``) so nested sport
frames grid independently of the main form.
"""
counters = self._mw_grid_counters
idx = counters.get(id(parent), 0)
if full and idx % 2 == 1:
idx += 1 # a full-width item can't share a row — bump to a new one
row, col = divmod(idx, 2)
widget.grid(
row=row,
column=col,
columnspan=2 if full else 1,
sticky="ew",
padx=12,
pady=6,
)
counters[id(parent)] = idx + (2 if full else 1)
def _mw_next_full_row(self, parent: tk.Widget) -> int:
"""Reserve a fresh full-width row for a manually-gridded widget.
Used for the twin sport frames, which share one slot and are gridded
directly (not through ``_mw_grid``); this advances the cursor past that
row so following fields resume cleanly.
"""
counters = self._mw_grid_counters
idx = counters.get(id(parent), 0)
if idx % 2 == 1:
idx += 1
counters[id(parent)] = idx + 2
return idx // 2
def _mw_section(self, parent: tk.Widget, title: str) -> None:
"""Add a full-width section heading inside the form."""
label = tk.Label(
parent,
text=title,
font=("Arial", 20, "bold"),
fg="#88ccff",
bg="#1a1a1a",
anchor="w",
)
self._mw_grid(parent, label, full=True)
def _mw_entry(self, parent: tk.Widget, label: str) -> tk.StringVar:
"""Add a half-width label+entry cell and return its backing StringVar.
Builds its own cell inline (rather than reusing ``_add_label_entry``,
which is pack-based and shared with the sick dialog) so it slots into
the two-column grid. Paste stays disabled via ``disable_paste``.
"""
var = tk.StringVar()
cell = tk.Frame(parent, bg="#1a1a1a")
tk.Label(
cell,
text=label,
font=("Arial", 16),
fg="white",
bg="#1a1a1a",
anchor="w",
).pack(fill="x")
entry = tk.Entry(
cell,
textvariable=var,
font=("Arial", 18),
bg="#2a2a2a",
fg="white",
insertbackground="white",
)
entry.pack(fill="x", pady=2)
disable_paste(entry)
self._mw_grid(parent, cell)
return var
def _mw_int_field(
self, parent: tk.Widget, label: str, *, frm: int = 0, to: int = 99
) -> tk.IntVar:
"""Add a half-width label + numeric Spinbox and return its IntVar."""
var = tk.IntVar(value=0)
row = tk.Frame(parent, bg="#1a1a1a")
tk.Label(
row,
text=label,
font=("Arial", 16),
fg="white",
bg="#1a1a1a",
).pack(side="left", padx=5)
tk.Spinbox(
row,
from_=frm,
to=to,
textvariable=var,
width=4,
font=("Arial", 16),
).pack(side="left", padx=5)
self._mw_grid(parent, row)
return var
def _mw_textbox(self, parent: tk.Widget, label: str) -> tk.Text:
"""Add a full-width label + multi-line Text cell and return the widget."""
cell = tk.Frame(parent, bg="#1a1a1a")
tk.Label(
cell,
text=label,
font=("Arial", 16),
fg="white",
bg="#1a1a1a",
anchor="w",
).pack(fill="x", pady=(5, 0))
text_widget = tk.Text(
cell,
height=3,
font=("Arial", 14),
bg="#2a2a2a",
fg="white",
insertbackground="white",
)
text_widget.pack(pady=2, fill="x")
disable_paste(text_widget)
self._mw_grid(parent, cell, full=True)
return text_widget
def _mw_rpe_row(self, parent: tk.Widget) -> None:
"""Add the RPE (rate of perceived exertion) spinbox row."""
row = tk.Frame(parent, bg="#1a1a1a")
tk.Label(
row,
text=(
f"RPE — perceived exertion "
f"({MANUAL_WORKOUT_RPE_MIN}-{MANUAL_WORKOUT_RPE_MAX}):"
),
font=("Arial", 16),
fg="white",
bg="#1a1a1a",
).pack(side="left", padx=5)
tk.Spinbox(
row,
from_=MANUAL_WORKOUT_RPE_MIN,
to=MANUAL_WORKOUT_RPE_MAX,
textvariable=self._mw_rpe_var,
width=4,
font=("Arial", 16),
).pack(side="left", padx=5)
self._mw_grid(parent, row)

View File

@ -28,14 +28,14 @@ if TYPE_CHECKING:
from typing import Literal
# Every module that imports ``tkinter as tk`` and calls it directly. The UI was
# split across these modules, so each ``tk`` reference must be patched — both to
# guarantee no test can touch a real display and so a test holding ``mock_tk``
# sees widgets created on that same mock (not a divergent autouse mock).
# Every module that imports ``tkinter as tk`` and calls it directly. The UI is
# split across these, so each ``tk`` must be patched — so no test touches a real
# display and a ``mock_tk`` holder sees widgets made on that same mock.
_TK_MODULES = (
"screen_locker.screen_lock",
"screen_locker._sick_dialog",
"screen_locker._manual_workout_dialog",
"screen_locker._manual_workout_widgets",
"screen_locker._ui_widgets",
"screen_locker._window_setup",
"screen_locker.status_view",

View File

@ -291,10 +291,6 @@ class TestBuildEntry:
assert entry["location_name"] == "Solec"
assert entry["racket"] == "spin"
def test_optional_location_maps_link_defaults_empty(self) -> None:
entry = build_entry(_tt_draft())
assert entry["location_maps_link"] == ""
def test_pain_or_injury_defaults_to_none(self) -> None:
entry = build_entry(_tt_draft())
assert entry["pain_or_injury"] == "none"

View File

@ -50,8 +50,8 @@ class TestSportToggle:
locker._mw_other_frame = MagicMock()
locker._mw_sport_var = MagicMock()
locker._on_mw_sport_changed("Other")
locker._mw_tt_frame.pack_forget.assert_called_once()
locker._mw_other_frame.pack.assert_called_once_with(fill="x")
locker._mw_tt_frame.grid_remove.assert_called_once()
locker._mw_other_frame.grid.assert_called_once_with()
def test_selecting_table_tennis_shows_tt_frame(
self, mock_tk: MagicMock, mock_sys_exit: MagicMock, tmp_path: Path
@ -61,8 +61,8 @@ class TestSportToggle:
locker._mw_other_frame = MagicMock()
locker._mw_sport_var = MagicMock()
locker._on_mw_sport_changed("Table tennis")
locker._mw_other_frame.pack_forget.assert_called_once()
locker._mw_tt_frame.pack.assert_called_once_with(fill="x")
locker._mw_other_frame.grid_remove.assert_called_once()
locker._mw_tt_frame.grid.assert_called_once_with()
def test_unknown_label_defaults_to_table_tennis(
self, mock_tk: MagicMock, mock_sys_exit: MagicMock, tmp_path: Path
@ -75,6 +75,30 @@ class TestSportToggle:
assert locker._current_mw_sport() == _manual_workout.SPORT_TABLE_TENNIS
class TestGridCursor:
"""Tests for the two-column grid cursor helper _mw_next_full_row."""
def test_full_row_from_even_cursor_advances_by_a_row(
self, mock_tk: MagicMock, mock_sys_exit: MagicMock, tmp_path: Path
) -> None:
locker = create_locker(mock_tk, tmp_path)
locker._mw_grid_counters = {}
parent = object()
# Fresh (even) cursor: reserves row 0, then row 1 on the next call.
assert locker._mw_next_full_row(parent) == 0
assert locker._mw_next_full_row(parent) == 1
def test_full_row_from_odd_cursor_bumps_to_next_row(
self, mock_tk: MagicMock, mock_sys_exit: MagicMock, tmp_path: Path
) -> None:
locker = create_locker(mock_tk, tmp_path)
parent = object()
# An odd cursor (a half-width cell already placed on the left) must be
# bumped to a fresh row before a full-width item is placed.
locker._mw_grid_counters = {id(parent): 1}
assert locker._mw_next_full_row(parent) == 1
class TestSubmitManualWorkoutForm:
"""Tests for _submit_manual_workout_form."""
@ -90,11 +114,9 @@ class TestSubmitManualWorkoutForm:
"start_time": "12:00",
"end_time": "14:00",
"location_name": "Osrodek Solec",
"location_maps_link": "",
"transport_method": "bike",
"cost": "60 PLN",
"reservation_phone": "",
"proof_screenshot_path": "",
"techniques_practiced": "",
"warm_up_minutes": "",
"pain_or_injury": "none",
@ -120,11 +142,9 @@ class TestSubmitManualWorkoutForm:
"start_time",
"end_time",
"location_name",
"location_maps_link",
"transport_method",
"cost",
"reservation_phone",
"proof_screenshot_path",
"techniques_practiced",
"warm_up_minutes",
"pain_or_injury",