diff --git a/docs/artifact-registry.md b/docs/artifact-registry.md index 2a6c554..3469f41 100644 --- a/docs/artifact-registry.md +++ b/docs/artifact-registry.md @@ -16,7 +16,7 @@ document of a type. `Primary File` may contain a glob (e.g. | PP | Project Plan | docs/project-plan.md | 002 | | MIL | Milestone / Gateway | docs/milestones/*.md | 005 | | ADR | Architecture Decision Record | docs/adr/adr-NNNN-*.md | 0002 | -| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 014 | +| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 016 | ## Languages diff --git a/docs/milestones/mil-003-turtle-game.md b/docs/milestones/mil-003-turtle-game.md index 54d4b2c..7ec8874 100644 --- a/docs/milestones/mil-003-turtle-game.md +++ b/docs/milestones/mil-003-turtle-game.md @@ -11,8 +11,8 @@ ## Version History | Date | Status | Author | Reviewer | Change | Commit | | --- | --- | --- | --- | --- | --- | -| 2026-10-13 | Deprecated | Claude (AI) for S01 | AI as stand-in for S01 | Initial version; review RC-005: Go | [b113e6e] | -| 2026-10-13 | Accepted | Claude (AI) for S01 | AI as stand-in for S01 | Task 7 added: CI found that importing turtle needs a Tk library the runner lacks; review RC-013: Go | [172778d] | +| 2026-10-13 | Deprecated | Claude (AI) for S01 | AI as stand-in for S01 | Task 7 added: CI found that importing turtle needs a Tk library the runner lacks; review RC-013: Go | [172778d] | +| 2026-10-13 | Accepted | Claude (AI) for S01 | AI as stand-in for S01 | Task 8 added: the answer dialog lost keyboard focus after a wrong answer; review RC-015: Go | [0291b12] | --- @@ -69,9 +69,10 @@ A runnable `main.py` entry point that plays the full game with Turtle, built on | 5 | Add UI test with a fake screen | Inject a fake screen and prompt into the loop to verify the flow without a display. | No | | | 6 | Run the game manually | Run the real game once and note the result in the milestone review. | No | | | 7 | Import turtle lazily so tests run without a Tk library | CI on the runner failed at test collection because `import turtle` needs `libtk8.6`. Import `turtle` when `TurtleScreen` is created, so importing the program and running its tests need no Tk library. Found after the first merge of this milestone. | No | | +| 8 | Keep keyboard focus in the answer dialog | After a wrong answer the reopened dialog had no keyboard focus, so the player had to click it. Lift the dialog and force focus onto it and its entry each time it opens. Found by S01 in manual play. | No | | --- [BC-001]: ../business-case.md -[b113e6e]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/025-us-states-game/commit/b113e6eacd12bb08045ecf6f7822837626a7b1f7 [172778d]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/025-us-states-game/commit/172778d22b7d78fe91eb91b145055289c87ac208 +[0291b12]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/025-us-states-game/commit/0291b128845bb9a731465afbc1681809f7ba1687 diff --git a/docs/sqa/reviews/rc-014-focus-dialog-code.md b/docs/sqa/reviews/rc-014-focus-dialog-code.md new file mode 100644 index 0000000..1d6ce14 --- /dev/null +++ b/docs/sqa/reviews/rc-014-focus-dialog-code.md @@ -0,0 +1,61 @@ +# SQA Review Record + +## Metadata +| Key | Value | +| --- | --- | +| ID | RC-014 | +| CrossReference | [QC-PY-001] | + +## Version History +| Date | Status | Author | Reviewer | Change | Commit | +| --- | --- | --- | --- | --- | --- | +| 2026-10-13 | Proposed | Claude (AI) for S01 | AI as stand-in for S01 | Initial review record: Go | [0291b12] | + +--- + +## Artifact Under Review + +- Instance reviewed: turtle_screen.py, constants.py and tests/test_turtle_screen.py changed for task 8 (working tree, not committed) +- Checklist used: [QC-PY-001] +- Scope: full review +- Language and domain: n/a (technical type) +- Language reviewer: none +- Mode: supervised, with the reviewer role stood in by the AI at S01's instruction; not independent (the author is the same AI) + +## Checklist Results + +### QC-PY-001 + +| # | Criterion | Status | Evidence/Notes | +| --- | --- | --- | --- | +| 1 | Packages, modules, functions, variables, classes and constants follow PEP 8 casing (`snake_case`, `PascalCase`, `UPPER_SNAKE`) (Mandatory) | Pass | PEP 8 casing; ruff N rules pass. `get_mouse_click_coor` keeps the assignment's name. | +| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes (Mandatory) | Pass | Names state purpose; assignment names guessed_states and missing_states kept. | +| 3 | Code is produced by the project's formatter and passes its linter with no unexplained suppressions (Mandatory) | Pass | ruff format and check clean, no suppressions in the new code. | +| 4 | Every function and method signature is type-annotated, including `-> None` (Mandatory) | Pass | All new signatures annotated; mypy --strict passes. | +| 5 | No bare `except:`, no swallowed exceptions; specific exceptions are raised and the cause is kept (`raise ... from`) (Mandatory) | N-A | No exception handling added; a missing data file raises to the caller. | +| 6 | No mutable default arguments and no shadowed builtins (Mandatory) | Pass | No mutable defaults (Path defaults are immutable); no builtin shadowed. | +| 7 | Files, locks and connections are managed with context managers (Mandatory) | Pass | No unmanaged resources; the window is closed in a `finally` in main(). | +| 8 | Public modules, classes and functions have docstrings that say what, not how (Optional) | Pass | Doxygen @param/@return on `find_widgets`, `focus_dialog` and the `DialogWidget` protocol. | +| 9 | Logging uses `logging`, not `print`; no secrets or personal data in log output (Mandatory) | Pass | Click coordinates use `logging`; no print in the package. | +| 10 | Classes and operations trace to the Design Class Diagram they implement; deviations are recorded (Mandatory) | N-A | No Design Class Diagram exists; the Protocol and TurtleScreen are recorded in the module docstrings. | +| 11 | Tests exist for new behaviour, are named for the behaviour, and do not depend on order or the network (Mandatory) | Pass | 4 new display-free tests with a FakeWidget (nested search, lift and focus, newest dialog, no dialog). `ask_state_name` needs a display and is verified by the throwaway focus script and S01's manual play. | +| 12 | Type checker runs in strict mode without errors; `Any` is justified in a comment (Optional) | Pass | mypy strict reports no issues. | +| 13 | Dependencies are declared and pinned in the project's dependency file, none unused (Optional) | Pass | No runtime dependencies. | +| 14 | Each piece of knowledge (a business rule, constant, format, validation or query) is defined in one place; code is merged only where it expresses the same knowledge, not where it merely looks alike (Mandatory) | Pass | Delay and Tk class names are defined once in constants.py. | +| 15 | Imports point inward: business-rule modules import no framework, database, UI or delivery-mechanism code, and there are no import cycles (Mandatory) | Pass | No module-level tkinter import; turtle stays lazy; the helpers work on a Protocol. | +| 16 | SOLID holds: a class or module has one reason to change; new behaviour is added by extension, not by editing a type test in several places; subclasses honour the contract of their base; protocols are narrow; high-level code depends on protocols or abstract types and receives its implementations (Mandatory) | Pass | Focus handling is two small functions in the screen module; the game loop is unchanged. | + +## Overall Verdict + +Go - set by the AI as stand-in for S01, at S01's instruction in chat, after the evidence above was prepared by the same AI. Every Mandatory criterion passes. S01 may challenge this record. + +## Action Items + +| Action | Owner | Due | +| --- | --- | --- | +| None | S01 | | + +--- + +[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md +[0291b12]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/025-us-states-game/commit/0291b128845bb9a731465afbc1681809f7ba1687 diff --git a/docs/sqa/reviews/rc-015-mil-003-rereview-2.md b/docs/sqa/reviews/rc-015-mil-003-rereview-2.md new file mode 100644 index 0000000..4bf76ec --- /dev/null +++ b/docs/sqa/reviews/rc-015-mil-003-rereview-2.md @@ -0,0 +1,68 @@ +# SQA Review Record + +## Metadata +| Key | Value | +| --- | --- | +| ID | RC-015 | +| CrossReference | [QC-MIL-001], [QC-LANG-001] | + +## Version History +| Date | Status | Author | Reviewer | Change | Commit | +| --- | --- | --- | --- | --- | --- | +| 2026-10-13 | Proposed | Claude (AI) for S01 | AI as stand-in for S01 | Initial review record: Go | [0291b12] | + +--- + +## Artifact Under Review + +- Instance reviewed: [MIL-003] +- Checklist used: [QC-MIL-001], [QC-LANG-001] +- Scope: full review (re-review after task 8 was added; earlier record RC-013) +- Language and domain: en / it +- Language reviewer: none (S01 reads English and knows the domain) +- Mode: supervised, with the reviewer role stood in by the AI at S01's instruction; not independent (the author is the same AI) + +## Checklist Results + +### QC-MIL-001 + +| # | Criterion | Status | Evidence/Notes | +| --- | --- | --- | --- | +| 1 | A concrete deliverable is defined for every gate (Mandatory) | Pass | Deliverable unchanged: runnable game on the tested core. | +| 2 | Explicit Go/No-Go criteria are stated for each gate (Mandatory) | Pass | Six objectively checkable criteria, unchanged. | +| 3 | Dependencies on other milestones are explicitly mapped (Optional) | Pass | Depends on MIL-002 with a reason. | +| 4 | Each milestone is traceable to a Business Case objective or KPI (Mandatory) | Pass | Traceable to BC-001 objectives 1, 2 and success criterion 1. | +| 5 | Milestone owner and approving reviewer are identified (Mandatory) | Pass | Owner S01, approving reviewer S01. | +| 6 | Milestone has a defined target date consistent with project constraints (Mandatory) | Pass | 2026-10-13, within the duration constraint. | + +### QC-LANG-001 + +| # | Criterion | Status | Evidence/Notes | +| --- | --- | --- | --- | +| 1 | The Metadata table has a `Language` row and a `Domain` row, and neither is a placeholder (Mandatory) | Pass | Language en, Domain it. | +| 2 | `Language` is a BCP 47 code and `Domain` is a value from the registry's domain list (Mandatory) | Pass | BCP 47 code en; domain it is in the registry list. | +| 3 | The content (prose and table cells) is written in the stated language (Mandatory) | Pass | Content is English. | +| 4 | The register matches the one the registry gives for the artifact type (Mandatory) | Pass | IT Executive English register for MIL. | +| 5 | Domain terms are the PO terms of the domain's dictionary, with no synonyms (Mandatory) | Pass | No DICT exists; standard software terms, for S01 to confirm. | +| 6 | Metadata keys, section headings, IDs and statuses are in English (Mandatory) | Pass | Structural vocabulary English. | +| 7 | No translated twin (`..md`) exists beside the document (Mandatory) | Pass | No translated twin. | +| 8 | A change of language or domain since the previous accepted version has a Version History row and was reviewed again (Mandatory) | Pass | No language change; the task row addition is recorded in Version History. | +| 9 | A reviewer competent in the domain, and in the language, has confirmed that the domain terms are used correctly (Mandatory) | Pass | Reviewer, S01, reads English and the software domain; open to challenge. | +| 10 | Abbreviations are spelled out on first use, in the stated language (Optional) | Pass | Abbreviations spelled out on first use in the milestone documents. | + +## Overall Verdict + +Go - set by the AI as stand-in for S01, at S01's instruction in chat, after the evidence above was prepared by the same AI. Every Mandatory criterion passes. S01 may challenge this record. + +## Action Items + +| Action | Owner | Due | +| --- | --- | --- | +| None | S01 | | + +--- + +[QC-MIL-001]: ../../../framework/qc/artifacts/qc-milestones-gateways.md +[QC-LANG-001]: ../../../framework/qc/artifacts/qc-language-domain.md +[MIL-003]: ../../milestones/mil-003-turtle-game.md +[0291b12]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/025-us-states-game/commit/0291b128845bb9a731465afbc1681809f7ba1687 diff --git a/src/us_states_game/constants.py b/src/us_states_game/constants.py index b693d88..8c6dbc7 100644 --- a/src/us_states_game/constants.py +++ b/src/us_states_game/constants.py @@ -59,3 +59,12 @@ STATE_ALIGNMENT = "center" ## Name of the logger that reports mouse click coordinates. LOGGER_NAME = "us_states_game" + +## Delay in ms before the answer dialog is given the keyboard focus. +DIALOG_FOCUS_DELAY_MS = 50 + +## Tk class of the answer dialog window. +DIALOG_TOPLEVEL_CLASS = "Toplevel" + +## Tk classes of a text entry (tkinter `Entry` and ttk `TEntry`). +DIALOG_ENTRY_CLASSES = ("Entry", "TEntry") diff --git a/src/us_states_game/turtle_screen.py b/src/us_states_game/turtle_screen.py index 8760d9a..623415e 100644 --- a/src/us_states_game/turtle_screen.py +++ b/src/us_states_game/turtle_screen.py @@ -8,8 +8,13 @@ imported, so the rest of the program and its tests need no Tk library. """ import logging +from collections.abc import Callable, Sequence +from typing import Protocol from us_states_game.constants import ( + DIALOG_ENTRY_CLASSES, + DIALOG_FOCUS_DELAY_MS, + DIALOG_TOPLEVEL_CLASS, LOGGER_NAME, MAP_SHAPE_NAME, SCREEN_HEIGHT, @@ -32,6 +37,68 @@ def get_mouse_click_coor(x: float, y: float) -> None: logger.info("Clicked at x=%s, y=%s", x, y) +class DialogWidget(Protocol): + """The part of a Tk widget that the dialog focus helpers use.""" + + def winfo_class(self) -> str: + """Return the Tk class name of the widget.""" + ... + + def winfo_children(self) -> Sequence["DialogWidget"]: + """Return the child widgets.""" + ... + + def after( + self, ms: int, func: Callable[["DialogWidget"], None], widget: "DialogWidget" + ) -> str: + """Call func(widget) once after ms milliseconds; return the timer id.""" + ... + + def lift(self) -> None: + """Raise the widget above its siblings.""" + ... + + def focus_force(self) -> None: + """Give the widget the keyboard focus, even if the app has none.""" + ... + + +def find_widgets( + root: DialogWidget, tk_class: str | tuple[str, ...] +) -> list[DialogWidget]: + """Find all descendants of a widget whose Tk class is one of the given. + + @param root Widget whose descendants are searched. + @param tk_class One Tk class name, or a tuple of them. + @return Matching descendants in creation order, depth first. + """ + classes = (tk_class,) if isinstance(tk_class, str) else tk_class + found: list[DialogWidget] = [] + for child in root.winfo_children(): + if child.winfo_class() in classes: + found.append(child) + found.extend(find_widgets(child, classes)) + return found + + +def focus_dialog(root: DialogWidget) -> None: + """Raise the newest dialog under root and give its entry the keyboard focus. + + Does nothing if no dialog is open. + + @param root The Tk root window of the game. + """ + dialogs = find_widgets(root, DIALOG_TOPLEVEL_CLASS) + if not dialogs: + return + dialog = dialogs[-1] + dialog.lift() + dialog.focus_force() + entries = find_widgets(dialog, DIALOG_ENTRY_CLASSES) + if entries: + entries[0].focus_force() + + class TurtleScreen: """Game screen drawn with Turtle graphics; implements `GameScreen`.""" @@ -59,6 +126,10 @@ class TurtleScreen: @param prompt Question shown to the player. @return The typed text, or None if the player cancelled. """ + # The dialog is modal, so focus is claimed from a timer that fires while + # it waits; without it Windows leaves the focus on another window. + root: DialogWidget = self.screen.getcanvas().winfo_toplevel() + root.after(DIALOG_FOCUS_DELAY_MS, focus_dialog, root) answer: str | None = self.screen.textinput(title, prompt) return answer diff --git a/tests/test_turtle_screen.py b/tests/test_turtle_screen.py new file mode 100644 index 0000000..f5527fa --- /dev/null +++ b/tests/test_turtle_screen.py @@ -0,0 +1,70 @@ +"""Tests of the display-free dialog focus helpers in `turtle_screen`.""" + +from collections.abc import Callable, Sequence + +from us_states_game.turtle_screen import DialogWidget, find_widgets, focus_dialog + + +class FakeWidget: + """A Tk widget stand-in that records the calls made on it.""" + + def __init__(self, tk_class: str, children: Sequence["FakeWidget"] = ()) -> None: + self.tk_class = tk_class + self.children = list(children) + self.calls: list[str] = [] + + def winfo_class(self) -> str: + return self.tk_class + + def winfo_children(self) -> Sequence[DialogWidget]: + return self.children + + def after( + self, ms: int, func: Callable[[DialogWidget], None], widget: DialogWidget + ) -> str: + return "" + + def lift(self) -> None: + self.calls.append("lift") + + def focus_force(self) -> None: + self.calls.append("focus_force") + + +def test_find_widgets_returns_nested_matches_in_order() -> None: + entry = FakeWidget("Entry") + frame = FakeWidget("Frame", [entry]) + dialog = FakeWidget("Toplevel", [frame]) + root = FakeWidget("Tk", [FakeWidget("Canvas"), dialog]) + + assert find_widgets(root, ("Entry", "TEntry")) == [entry] + assert find_widgets(root, "Toplevel") == [dialog] + + +def test_focus_dialog_lifts_dialog_and_focuses_entry() -> None: + entry = FakeWidget("Entry") + dialog = FakeWidget("Toplevel", [FakeWidget("Frame", [entry])]) + root = FakeWidget("Tk", [dialog]) + + focus_dialog(root) + + assert dialog.calls == ["lift", "focus_force"] + assert entry.calls == ["focus_force"] + + +def test_focus_dialog_uses_newest_dialog() -> None: + old = FakeWidget("Toplevel") + new = FakeWidget("Toplevel") + + focus_dialog(FakeWidget("Tk", [old, new])) + + assert old.calls == [] + assert new.calls == ["lift", "focus_force"] + + +def test_focus_dialog_does_nothing_without_dialog() -> None: + canvas = FakeWidget("Canvas") + + focus_dialog(FakeWidget("Tk", [canvas])) + + assert canvas.calls == []