Repository navigation
gui: Let the home window choose its height - #78
BenWestgate wants to merge 2 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
BenWestgate
added
gui
ChatGPT Codex Connector
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated release-gate review, posted at the maintainer's request.
Code ACK d510621.
The diff removes only the fixed default_height=620; the 880px default width, navigation, page content, and scrolling hierarchy are unchanged. GTK4 documents that an unset default dimension uses the window's natural size request, while the window remains resizable. Every workflow page is still hosted by the existing Gtk.ScrolledWindow, so longer pages remain usable at the smaller initial home height.
Manual qualification remains appropriate: verify the home opens compactly and a representative long page scrolls at the supported Tails guest resolution. No further automated Codex review request is warranted for this one-line behavior change.
d510621 to
7b82ebc
Compare
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Updated release-gate review for rebased head 7b82ebc.
Code ACK. The PR is again exactly one commit / one file / one behavioral change on top of current #77: remove default_height=620 while retaining default_width=880. The prior review rationale is unchanged. Manual Tails confirmation of compact home height plus scrolling on a representative long page remains the only qualification item.
7b82ebc to
38373b3
Compare
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Updated release-gate review for rebased head 38373b3.
Code ACK. The PR remains exactly one commit / one file / one behavioral change on top of current #77: remove default_height=620 while retaining default_width=880. Manual Tails confirmation of compact home height plus scrolling on a representative long page remains the only qualification item.
|
Agent release-gate review at exact head |
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated review (Codex), posted at the maintainer's request.
ACK 38373b3. Removing the forced window height is safe.
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head review at 38373b3: no findings. Removing the fixed default height lets the existing navigation content determine window height.
Restore the Codex32 Book checksum worksheet command removed in 1a14000, following BlockstreamResearch/codex32 discussion #78: checksum generation is acceptable when it is clearly separate from validation and correction and is never used on completed shares. Compared with the removed command: - it accepts only the Book checksum worksheet's 32 squares; input of any complete ms string length is refused with a pointer to check, so a damaged complete string can no longer be checksummed as a longer unchecksummed payload; - the warning also forbids replacing the checksum of an existing backup, and help text names check and correct for existing backups; - it stays ms32-only, interactive-only, needs two matching entries with the screen cleared between them, and gives no format hints. The library keeps no public completion API; the command builds the string with the existing private _from_parts. Claude-Session: https://claude.ai/code/session_01C8Ur3dhqpcWU3gEYsweXNM
Restore ms32 checksum, removed in 1a14000, as a generation-only command separate from check and correct, following codex32 discussion #78. The command accepts only the Codex32 Book checksum worksheet's 32 non-pink squares after the ms prefix and refuses every complete ms string length, so a damaged complete string cannot be mistaken for a longer unchecksummed payload. It requires an interactive terminal, warns against re-checksumming an existing backup or entering raw dice, seed words, hex seeds or passwords, and gives no format hints. The non-pink squares are entered twice with the screen cleared between entries. The completed string is then shown, the checksum is copied into the pink squares, and after another clear only those 13 squares are typed back. Only then does the string reach stdout. Every entry must be single-case ASCII; --plain keeps the input case. Security: checksum generation for new data. The repeated entries catch typing errors but not errors already on the worksheet, and nothing can prove the data was generated well; the warning and docs say so. Validation: ruff, ruff format, mypy, python -m pytest -q and python -O -m pytest -q. Claude-Session: https://claude.ai/code/session_01C8Ur3dhqpcWU3gEYsweXNM
Restore ms32 checksum, removed in 1a14000, as a generation-only command separate from check and correct, following codex32 discussion #78. The command accepts only the Codex32 Book checksum worksheet's 32 non-pink squares after the ms prefix and refuses every complete ms string length. This narrows, but cannot remove, the chance that a damaged existing backup is taken for new unchecksummed data: one that lost or gained characters can still reach the accepted length. It requires an interactive terminal, warns against re-checksumming an existing backup or entering raw dice, seed words, hex seeds or passwords, and gives no format hints. The non-pink squares are entered twice with the screen cleared between entries. The completed string is then shown, the checksum is copied into the pink squares, and after another clear only those 13 squares are typed back. Only then does the string reach stdout. Every entry must be single-case ASCII; --plain keeps the input case. Security: checksum generation for new data. The repeated entries catch typing errors but not errors already on the worksheet, and nothing can prove the data was generated well; the warning and docs say so. Validation: ruff, ruff format, mypy, python -m pytest -q and python -O -m pytest -q. Claude-Session: https://claude.ai/code/session_01C8Ur3dhqpcWU3gEYsweXNM
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38373b39b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Is this PR really necessary? What problem is it trying to solve? |
Let the home page scroller contribute its natural height to the initial window size. Other workflow pages still scroll without requesting their full height. refs #75
|
Yes—#75 records the Tails home-window sizing defect. The reviewer was right: removing |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Closes #75.
Remove the fixed 620px default window height, retain its 880px width, and let only the six-action home page propagate its natural scroller height. Other workflow pages keep their scrolling behavior. This follows GTK4's size-request behavior without a release-specific height.
b347792fixes the current-head review finding: the home scroller previously did not propagate its content height, so simply removing the default left the initial window undersized. The change affects two GUI files; no wallet or recovery behavior changes.Validation: 71 GUI tests, Ruff lint/format, bytecode compilation, and
git diff --checkpassed. Supported-Tails minimum-resolution sizing and long-page scrolling remain manual release gates.AI assistance was used for these authorized changes. Human review remains required.