From 78b074673607343f454bb6bb4e652ed7297f2dda Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 4 Oct 2026 03:58:29 +0000 Subject: [PATCH] fix: keep every flash message on the one line it reserves (closes #252) The flash line reserves one line of height, so a message that wrapped pushed the screen below it down. Every message is now at most 50 characters, one line of the popup's monospace font: the longer ones are reworded, and the ones that carried a wallet name or text from a server no longer do. The rule is written at showFlash(). A new end-to-end test drives the longest message and fails if the line's rendered height grows. It measures in the monospace font, which Firefox draws and Chromium does not. Model: opus-5-5 --- TODO.md | 14 +++++++ src/popup/dustThreshold.js | 10 ++--- src/popup/views/addToken.js | 4 +- src/popup/views/addWallet.js | 33 +++++------------ src/popup/views/helpers.js | 7 ++++ src/popup/views/send.js | 6 +-- src/popup/views/settings.js | 10 +---- src/popup/views/settingsAddToken.js | 4 +- src/shared/walletDefects.js | 10 ++--- tests/e2e/run.js | 57 +++++++++++++++++++++++++++++ 10 files changed, 102 insertions(+), 53 deletions(-) diff --git a/TODO.md b/TODO.md index f06c9c2..0329924 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,20 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-04: Every flash message fits on the one line the flash line reserves, + and a test measures it + ([#252](https://git.eeqj.de/sneak/AutistMask/issues/252)). A message that + wrapped pushed the whole screen below it down. Rather than reserve a second + line on every screen or let the flash cover the screen, every message is now + at most 50 characters, one line of the popup's monospace font: the longer ones + are reworded, and messages that carried a wallet name or text from a server no + longer do, since neither has a length limit. The rule is written at + `showFlash()` in `src/popup/views/helpers.js`. A new test in + `tests/e2e/run.js` drives the longest message and fails if the line's rendered + height grows. It measures in the monospace font, because Chromium draws the + popup in its narrower system font and only Firefox shows the wrap. The two + approval-screen error boxes are left to + [#297](https://git.eeqj.de/sneak/AutistMask/issues/297). - 2026-10-04: Removing an address or deleting a wallet ends every site connection approved without "Remember" for the addresses removed ([#245](https://git.eeqj.de/sneak/AutistMask/issues/245)). Such a connection diff --git a/src/popup/dustThreshold.js b/src/popup/dustThreshold.js index 7b64c60..e735fb8 100644 --- a/src/popup/dustThreshold.js +++ b/src/popup/dustThreshold.js @@ -19,13 +19,9 @@ // that the user did not type — the same silent substitution the visible // rejection message exists to end. -// Must render on ONE line of #flash-msg, whose reserved height -// (min-h-[1.25rem]) is exactly one line at text-xs. A string long enough to -// wrap to two lines pushes the settings view down, which the No Layout Shift -// policy forbids. Do not lengthen this without re-running the layout test in -// tests/e2e/run.js, which measures the flash line and goes red on a shift. -const DUST_THRESHOLD_MESSAGE = - "Please enter a whole number of gwei, zero or greater."; +// Must render on ONE line of #flash-msg; see showFlash() in +// src/popup/views/helpers.js for how long that is. +const DUST_THRESHOLD_MESSAGE = "Enter a whole number of gwei, zero or greater."; // Returns the threshold in gwei, or null if the input is not one. function parseDustThresholdGwei(raw) { diff --git a/src/popup/views/addToken.js b/src/popup/views/addToken.js index a408d9b..65a0fb7 100644 --- a/src/popup/views/addToken.js +++ b/src/popup/views/addToken.js @@ -28,9 +28,7 @@ function init(ctx) { $("btn-add-token-confirm").addEventListener("click", async () => { const contractAddr = $("add-token-address").value.trim(); if (!contractAddr || !contractAddr.startsWith("0x")) { - showFlash( - "Please enter a valid contract address starting with 0x.", - ); + showFlash("Enter a valid contract address starting with 0x."); return; } const already = state.trackedTokens.find( diff --git a/src/popup/views/addWallet.js b/src/popup/views/addWallet.js index 1eaf55e..c9b7087 100644 --- a/src/popup/views/addWallet.js +++ b/src/popup/views/addWallet.js @@ -142,15 +142,13 @@ function validatePassword() { async function importMnemonic(ctx) { const mnemonic = $("wallet-mnemonic").value.trim(); if (!mnemonic) { - showFlash("Enter a recovery phrase or press the die to generate one."); + showFlash("Enter a recovery phrase, or press the die."); return; } const words = mnemonic.split(/\s+/); if (words.length !== 12 && words.length !== 24) { showFlash( - "Recovery phrase must be 12 or 24 words. You entered " + - words.length + - ".", + "Recovery phrase must be 12 or 24 words, not " + words.length + ".", ); return; } @@ -163,14 +161,12 @@ async function importMnemonic(ctx) { const { xpub, firstAddress } = hdWalletFromMnemonic(mnemonic); const xpubDup = findWalletByXpub(xpub); if (xpubDup) { - showFlash( - "This recovery phrase is already added (" + xpubDup.name + ").", - ); + showFlash("This recovery phrase is already added."); return; } const addrDup = findWalletByAddress(firstAddress); if (addrDup) { - showFlash("Address already exists in wallet (" + addrDup.name + ")."); + showFlash("Address already exists in a wallet."); return; } const encrypted = await encryptWithPassword(mnemonic, pw); @@ -229,9 +225,7 @@ async function importPrivateKey(ctx) { if (!pw) return; const duplicate = findWalletByAddress(addr); if (duplicate) { - showFlash( - "This address already exists in wallet (" + duplicate.name + ").", - ); + showFlash("This address already exists in a wallet."); return; } const encrypted = await encryptWithPassword(key, pw); @@ -258,36 +252,29 @@ async function importXprvKey(ctx) { return; } if (!isValidXprv(xprv)) { - showFlash( - "That extended private key is not valid. Please check it and try again.", - ); + showFlash("That extended private key is not valid."); return; } if (!isMasterExtendedKey(xprv)) { - showFlash( - "That is an account-level or child key, which cannot be imported. " + - "Please paste the master extended private key for the wallet.", - ); + showFlash("Please paste the master key, not a child key."); return; } let result; try { result = hdWalletFromXprv(xprv); } catch { - showFlash( - "That extended private key is not valid. Please check it and try again.", - ); + showFlash("That extended private key is not valid."); return; } const { xpub, firstAddress } = result; const xpubDup = findWalletByXpub(xpub); if (xpubDup) { - showFlash("This key is already added (" + xpubDup.name + ")."); + showFlash("This key is already added."); return; } const addrDup = findWalletByAddress(firstAddress); if (addrDup) { - showFlash("Address already exists in wallet (" + addrDup.name + ")."); + showFlash("Address already exists in a wallet."); return; } const pw = validatePassword(); diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index f1e5f9f..4b8917c 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -225,6 +225,13 @@ function clearFlash() { $("flash-msg").textContent = ""; } +// The flash line reserves the height of exactly one line, so a message that +// wraps pushes the whole screen below it down, which README's No Layout Shift +// rule forbids. Every message must fit on one line of the popup's monospace +// font: 50 characters at most, and nothing of unbounded length, such as a +// wallet name or text from a server, may be put into one. The longest message +// is measured by "the longest flash message fits on one line (#252)" in +// tests/e2e/run.js; point that test at any message longer than it. function showFlash(msg, duration = 2000) { clearFlash(); $("flash-msg").textContent = msg; diff --git a/src/popup/views/send.js b/src/popup/views/send.js index fee2895..1600d14 100644 --- a/src/popup/views/send.js +++ b/src/popup/views/send.js @@ -63,13 +63,13 @@ function validateToAddress(value) { if (checksummed !== v) { return { valid: false, - error: "Address checksum is invalid. Please double-check the address.", + error: "Address checksum is invalid. Check the address.", }; } } catch { return { valid: false, - error: "Address checksum is invalid. Please double-check the address.", + error: "Address checksum is invalid. Check the address.", }; } } @@ -211,7 +211,7 @@ function init(_ctx) { const provider = getProvider(state.rpcUrl, state.networkId); const resolved = await provider.resolveName(to); if (!resolved) { - showFlash("Could not resolve " + to); + showFlash("That ENS name has no address."); return; } resolvedTo = resolved; diff --git a/src/popup/views/settings.js b/src/popup/views/settings.js index 06fd3d4..2fd8a2d 100644 --- a/src/popup/views/settings.js +++ b/src/popup/views/settings.js @@ -236,18 +236,12 @@ function init(ctx) { const json = await resp.json(); if (json.error) { log.errorf("RPC validation error:", json.error); - showFlash("Endpoint returned error: " + json.error.message); + showFlash("Endpoint returned an error."); return; } const net = currentNetwork(); if (json.result !== net.chainId) { - showFlash( - "Wrong network (expected " + - net.name + - ", got chain " + - json.result + - ").", - ); + showFlash("Wrong network: expected " + net.name + "."); return; } } catch (e) { diff --git a/src/popup/views/settingsAddToken.js b/src/popup/views/settingsAddToken.js index 205cb38..00e2c8c 100644 --- a/src/popup/views/settingsAddToken.js +++ b/src/popup/views/settingsAddToken.js @@ -115,9 +115,7 @@ function init(_ctx) { $("btn-settings-addtoken-manual").addEventListener("click", async () => { const addr = $("settings-addtoken-address").value.trim(); if (!addr || !addr.startsWith("0x")) { - showFlash( - "Please enter a valid contract address starting with 0x.", - ); + showFlash("Enter a valid contract address starting with 0x."); return; } if (isTracked(addr)) { diff --git a/src/shared/walletDefects.js b/src/shared/walletDefects.js index afa069b..8ed89e2 100644 --- a/src/shared/walletDefects.js +++ b/src/shared/walletDefects.js @@ -41,12 +41,10 @@ const DEFECTS = { "changed or removed, and this wallet stays until you delete " + "it yourself.", ], - // One sentence for the places that have room for one: the flash on a - // blocked Send, the inline error on the approval screens. - shortMessage: - "This wallet cannot sign, because it was imported from an " + - "extended private key that is not a master key. The wallet list " + - "explains what happened.", + // One line, for the flash on a blocked Send and the inline error on + // the approval screens. It must fit on the flash line; see showFlash() + // in src/popup/views/helpers.js. + shortMessage: "This wallet cannot sign. See the wallet list.", }, }; diff --git a/tests/e2e/run.js b/tests/e2e/run.js index 48eb11c..678858c 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -1400,6 +1400,63 @@ test("a rejected dust threshold shifts no layout (#233)", async (env) => { } }); +// ------------------------------------------------ the flash line (#252) + +// Every flash message must fit the one line #flash-msg reserves (see +// showFlash() in src/popup/views/helpers.js). This drives the longest one +// and measures the line's height with it. +// +// The line is measured in the monospace font the popup declares. Firefox +// draws the popup in it; Chromium draws it in the system font instead +// (https://git.eeqj.de/sneak/AutistMask/issues/418), which is narrower, so a +// message that wraps in Firefox would still fit here and the test would pass. +test("the longest flash message fits on one line (#252)", async (env) => { + const page = await openPopup(env.ctx, env.popupUrl); + try { + await page.setViewportSize(POPUP_VIEWPORT); + await openSettings(page); + await page.click("#btn-settings-add-token"); + await visible(page, "#view-settings-addtoken"); + + const font = await page.evaluate(() => { + const line = document.getElementById("flash-msg"); + line.style.fontFamily = "var(--font-mono)"; + return getComputedStyle(line).fontFamily; + }); + assert( + font.includes("monospace"), + "the flash line is not in the monospace font: " + font, + ); + + const before = await page.evaluate(measureFlashLine); + await page.fill("#settings-addtoken-address", "not an address"); + await page.click("#btn-settings-addtoken-manual"); + const after = await waitForFilledFlashLine(page); + + assert( + after.flashHeight === before.flashHeight, + "the flash line is " + + before.flashHeight + + "px empty and " + + after.flashHeight + + "px with " + + JSON.stringify(after.text) + + ", so the message wraps", + ); + assert( + after.text === "Enter a valid contract address starting with 0x.", + "the screen flashed " + + JSON.stringify(after.text) + + ", not the message this test measures", + ); + + await page.click("#btn-settings-addtoken-back"); + await visible(page, "#view-settings"); + } finally { + await page.close(); + } +}); + // --------------------------------------------- confirmation screen (#238) // // The screen that decides what gets signed. The arithmetic underneath it