sneak/AutistMask
1
0
Fork 0
Code Issues 26 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix: flash messages longer than one line wrap and shift the layout, defeating the reserved-space policy #252

New Issue
Closed
opened 2026-08-12 10:36:17 +02:00 by clawbot · 3 comments
clawbot commented 2026-08-12 10:36:17 +02:00
Collaborator
Copy Link
Copy Source

src/popup/index.html:34-37 reserves min-h-[1.25rem] for #flash-msg — 20px, exactly ONE line at text-xs (12px/16px). Any flash string long enough to wrap renders at 32px and pushes the view below it down 12px, which is precisely what the README No Layout Shift policy exists to prevent.

Measured in the repo's own pinned Playwright container at 360x600 against the built popup, during the independent review of #243:

  • empty → 20px (baseline)
  • Please enter an RPC URL. (24 chars) → 20px, no shift
  • a 75-char message → 32px, shifts #view-settings and #settings-dust-threshold down 12px

Pre-existing, not introduced by that PR: the longest existing flash string — the invalid-xprv message in src/popup/views/addWallet.js, 70 chars — shifts by the identical 12px today. #243 is shortening its own message to fit rather than waiving the policy, which leaves this repo-wide gap open.

The reservation is also unenforced. Nothing in the suite can see wrapping: the only assertion that claims to pin it regex-matches min-h-\[ in the class attribute, which passes at any message length.

Implementation requirements

  • Decide the shape deliberately and say why in the PR body. The options are not equivalent: reserve two lines (costs 20px of vertical space on every screen, always); constrain every flash string to fit one line at 360px (cheap, but a rule nobody can see enforced); or make the flash overlay rather than displace (no shift at any length, but it covers content).
  • Whichever is chosen, it must be ENFORCED BY A TEST THAT MEASURES RENDERED HEIGHT, not by a class-attribute match. A test that passes at any message length is not coverage.
  • Audit every existing flash and error string against the chosen rule; the invalid-xprv message in addWallet.js is known to violate the current one.
  • Check the other reserved-space regions in the popup for the same unenforced-reservation pattern.

Definition of done

  • No flash or error message shifts the layout at 360px width, proven by a rendered-height measurement rather than a class assertion.
  • A message grown past the reservation makes the suite fail — demonstrated, with captured output.
  • Every existing flash and error string is audited against the chosen rule and listed in the PR body.
  • TODO.md updated in the same commit.
  • make check passes.
`src/popup/index.html:34-37` reserves `min-h-[1.25rem]` for `#flash-msg` — 20px, exactly ONE line at `text-xs` (12px/16px). Any flash string long enough to wrap renders at 32px and pushes the view below it down 12px, which is precisely what the README No Layout Shift policy exists to prevent. Measured in the repo's own pinned Playwright container at 360x600 against the built popup, during the independent review of https://git.eeqj.de/sneak/AutistMask/pulls/243: - empty → 20px (baseline) - `Please enter an RPC URL.` (24 chars) → 20px, no shift - a 75-char message → 32px, shifts `#view-settings` and `#settings-dust-threshold` down 12px Pre-existing, not introduced by that PR: the longest existing flash string — the invalid-xprv message in `src/popup/views/addWallet.js`, 70 chars — shifts by the identical 12px today. https://git.eeqj.de/sneak/AutistMask/pulls/243 is shortening its own message to fit rather than waiving the policy, which leaves this repo-wide gap open. The reservation is also unenforced. Nothing in the suite can see wrapping: the only assertion that claims to pin it regex-matches `min-h-\[` in the class attribute, which passes at any message length. ## Implementation requirements - Decide the shape deliberately and say why in the PR body. The options are not equivalent: reserve two lines (costs 20px of vertical space on every screen, always); constrain every flash string to fit one line at 360px (cheap, but a rule nobody can see enforced); or make the flash overlay rather than displace (no shift at any length, but it covers content). - Whichever is chosen, it must be ENFORCED BY A TEST THAT MEASURES RENDERED HEIGHT, not by a class-attribute match. A test that passes at any message length is not coverage. - Audit every existing flash and error string against the chosen rule; the invalid-xprv message in `addWallet.js` is known to violate the current one. - Check the other reserved-space regions in the popup for the same unenforced-reservation pattern. ## Definition of done - [ ] No flash or error message shifts the layout at 360px width, proven by a rendered-height measurement rather than a class assertion. - [ ] A message grown past the reservation makes the suite fail — demonstrated, with captured output. - [ ] Every existing flash and error string is audited against the chosen rule and listed in the PR body. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.
clawbot commented 2026-08-12 11:32:19 +02:00
Author
Collaborator
Copy Link
Copy Source

Another reserved region with the same unenforced reservation, found while measuring for #172. Adding it here rather than filing separately, since auditing the other regions is already in this issue's scope.

#approve-tx-error and #approve-sign-error (src/popup/index.html:1512, :1589) carry border border-dashed p-1 on top of min-h-[1.25rem]. Under box-sizing: border-box that 20px reservation has to cover 8px of padding and 2px of border, leaving 10px for a 16px line, so any message overflows it — no wrapping required.

Measured in the pinned Playwright container at 360x600 against the built popup, on next at c6a1f97:

  • empty and hidden: 20px
  • one line of text shown: 26px, and the button row below moves down 6px

This is live today with the existing "Please enter your password." on an empty field, so it is not specific to any one string. Fix shape differs from the wrapping case: these need the reservation to account for padding and border, not a longer reservation.

Another reserved region with the same unenforced reservation, found while measuring for [#172](https://git.eeqj.de/sneak/AutistMask/issues/172). Adding it here rather than filing separately, since auditing the other regions is already in this issue's scope. `#approve-tx-error` and `#approve-sign-error` (`src/popup/index.html:1512`, `:1589`) carry `border border-dashed p-1` on top of `min-h-[1.25rem]`. Under `box-sizing: border-box` that 20px reservation has to cover 8px of padding and 2px of border, leaving 10px for a 16px line, so **any** message overflows it — no wrapping required. Measured in the pinned Playwright container at 360x600 against the built popup, on `next` at `c6a1f97`: - empty and hidden: 20px - one line of text shown: 26px, and the button row below moves down 6px This is live today with the existing `"Please enter your password."` on an empty field, so it is not specific to any one string. Fix shape differs from the wrapping case: these need the reservation to account for padding and border, not a longer reservation.
clawbot added this to the 1.0.0 milestone 2026-09-21 09:20:01 +02:00
clawbot commented 2026-10-03 14:19:02 +02:00
Author
Collaborator
Copy Link
Copy Source

Plan. One PR on a branch cut from next, base next.

  • Scope: #flash-msg and the audit of every flash string, as the issue body says. The two approval error containers in the comment above are #297, a separate unit that follows this one and reuses its measuring test; leave them alone here.
  • Pick the fix shape from the three options in the issue and give the reason in the PR body. Default reading, unless measuring shows it does not work: keep the one-line reservation and make every flash string fit one line at 360px, shortening the ones that do not.
  • The rule is enforced by a test in the existing Playwright end-to-end suite that measures the rendered height of #flash-msg with the longest message, not a class match; show that a longer message fails it.
  • The PR body lists every flash string audited.

Model: opus-5-5

Plan. One PR on a branch cut from `next`, base `next`. - Scope: `#flash-msg` and the audit of every flash string, as the issue body says. The two approval error containers in the comment above are https://git.eeqj.de/sneak/AutistMask/issues/297, a separate unit that follows this one and reuses its measuring test; leave them alone here. - Pick the fix shape from the three options in the issue and give the reason in the PR body. Default reading, unless measuring shows it does not work: keep the one-line reservation and make every flash string fit one line at 360px, shortening the ones that do not. - The rule is enforced by a test in the existing Playwright end-to-end suite that measures the rendered height of `#flash-msg` with the longest message, not a class match; show that a longer message fails it. - The PR body lists every flash string audited. Model: opus-5-5
clawbot commented 2026-10-04 06:29:22 +02:00
Author
Collaborator
Copy Link
Copy Source

Built in #420: every flash message is reworded to fit the one reserved line (at most 50 characters), and a new end-to-end test measures the line's height with the longest one, in the monospace font Firefox draws. Chromium draws the popup in its system font instead, filed as #418.

Model: opus-5-5

Built in https://git.eeqj.de/sneak/AutistMask/pulls/420: every flash message is reworded to fit the one reserved line (at most 50 characters), and a new end-to-end test measures the line's height with the longest one, in the monospace font Firefox draws. Chromium draws the popup in its system font instead, filed as https://git.eeqj.de/sneak/AutistMask/issues/418. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No labels
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#252