fix: show every password error in its own fixed-height line (closes #493)
The add wallet screen reported a missing, short or mismatched password in the flash line at the top of the popup, and the private key export, recovery phrase and delete wallet screens each wrote to a line of their own above the password field. All four now use showError() and hideError() with a fixed-height error line below the field, as the send confirmation and approval screens do. On the add wallet screen the line sits beside the Import button, which keeps its place at 360x600. The line clears when the screen is shown again and when the password is tried again. Other add wallet messages stay in the flash line. Model: opus-5-5
This commit was merged in pull request #496.
This commit is contained in:
+109
-6
@@ -809,7 +809,7 @@ async function secretScreenState(page, view) {
|
||||
return page.evaluate(
|
||||
(v) => ({
|
||||
value: document.getElementById(v + "-value").textContent,
|
||||
error: document.getElementById(v + "-flash").textContent,
|
||||
error: document.getElementById(v + "-password-error").textContent,
|
||||
html: document.getElementById("view-" + v).innerHTML,
|
||||
resultHidden: document
|
||||
.getElementById(v + "-result")
|
||||
@@ -906,7 +906,8 @@ test("a wrong password reveals nothing (#161)", async (env) => {
|
||||
await env.page.click("#btn-show-phrase-reveal");
|
||||
await env.page.waitForFunction(
|
||||
() =>
|
||||
document.getElementById("show-phrase-flash").textContent.length > 0,
|
||||
document.getElementById("show-phrase-password-error").textContent
|
||||
.length > 0,
|
||||
null,
|
||||
{ timeout: 60000 },
|
||||
);
|
||||
@@ -1993,13 +1994,14 @@ test("an over-long flash message keeps to one line (#252)", async (env) => {
|
||||
|
||||
// Every screen that asks for a password reserves room for one line of error.
|
||||
// The two on the dApp approval screens also have a border and padding, which
|
||||
// that reserved height has to cover too.
|
||||
// that reserved height has to cover too. The add wallet screen's line sits
|
||||
// beside its button rather than above it, and has its own test below.
|
||||
const PASSWORD_ERROR_CONTAINERS = [
|
||||
"approve-tx-error",
|
||||
"approve-sign-error",
|
||||
"export-privkey-flash",
|
||||
"show-phrase-flash",
|
||||
"delete-wallet-flash",
|
||||
"export-privkey-password-error",
|
||||
"show-phrase-password-error",
|
||||
"delete-wallet-password-error",
|
||||
"confirm-tx-password-error",
|
||||
];
|
||||
|
||||
@@ -2064,6 +2066,107 @@ test("a password error moves nothing on any screen (#297)", async (env) => {
|
||||
}
|
||||
});
|
||||
|
||||
// ------------------------------------------ add wallet Import button (#493)
|
||||
|
||||
// At 360x600 the add wallet screen's Import button already starts near the
|
||||
// bottom of the popup, so its password error line sits beside the button
|
||||
// rather than above it. Measures the button and the line empty and again
|
||||
// filled with the longest error addWallet.js puts there. Runs in the page.
|
||||
function measureImportButton() {
|
||||
const button = document.getElementById("btn-add-wallet-confirm");
|
||||
const line = document.getElementById("add-wallet-password-error");
|
||||
const measure = () => {
|
||||
const b = button.getBoundingClientRect();
|
||||
const l = line.getBoundingClientRect();
|
||||
return {
|
||||
buttonTop: b.top + window.scrollY,
|
||||
buttonBottom: b.bottom + window.scrollY,
|
||||
lineTop: l.top + window.scrollY,
|
||||
lineBottom: l.bottom + window.scrollY,
|
||||
};
|
||||
};
|
||||
const empty = measure();
|
||||
line.textContent = "Password must be at least 12 characters.";
|
||||
line.style.visibility = "visible";
|
||||
const filled = measure();
|
||||
line.textContent = "";
|
||||
line.style.visibility = "hidden";
|
||||
return { empty, filled };
|
||||
}
|
||||
|
||||
test("the add wallet password error leaves Import where it was (#493)", async (env) => {
|
||||
const page = await openPopup(env.ctx, env.popupUrl);
|
||||
try {
|
||||
await page.setViewportSize(POPUP_VIEWPORT);
|
||||
// Brought up by toggling classes, as in the test above. The note
|
||||
// addWallet.js shows once a wallet exists is the only part of the
|
||||
// screen that differs between the first wallet and a later one.
|
||||
await page.evaluate(() => {
|
||||
const screen = document.getElementById("view-add-wallet");
|
||||
for (const view of document.querySelectorAll(".view")) {
|
||||
view.classList.toggle("hidden", view !== screen);
|
||||
}
|
||||
});
|
||||
for (const walletExists of [false, true]) {
|
||||
await page.evaluate(
|
||||
(shown) =>
|
||||
document
|
||||
.getElementById("add-wallet-separate-password-note")
|
||||
.classList.toggle("hidden", !shown),
|
||||
walletExists,
|
||||
);
|
||||
for (const tab of ["tab-mnemonic", "tab-privkey", "tab-xprv"]) {
|
||||
await page.click("#" + tab);
|
||||
const { empty, filled } =
|
||||
await page.evaluate(measureImportButton);
|
||||
const where =
|
||||
"#" +
|
||||
tab +
|
||||
(walletExists
|
||||
? " with a wallet already added"
|
||||
: " for the first wallet");
|
||||
// Printed pass or fail, as the dust threshold test does.
|
||||
console.log(
|
||||
"# add wallet Import top, " +
|
||||
where +
|
||||
": " +
|
||||
empty.buttonTop +
|
||||
"px",
|
||||
);
|
||||
for (const m of [empty, filled]) {
|
||||
assert(
|
||||
m.lineTop >= m.buttonTop &&
|
||||
m.lineBottom <= m.buttonBottom,
|
||||
"the error line on " +
|
||||
where +
|
||||
" does not fit beside Import, so it adds height: " +
|
||||
JSON.stringify(m),
|
||||
);
|
||||
}
|
||||
assert(
|
||||
filled.buttonTop === empty.buttonTop,
|
||||
"Import moved " +
|
||||
(filled.buttonTop - empty.buttonTop) +
|
||||
"px when the error appeared on " +
|
||||
where,
|
||||
);
|
||||
if (!walletExists) {
|
||||
assert(
|
||||
empty.buttonTop < POPUP_VIEWPORT.height,
|
||||
"Import starts at " +
|
||||
empty.buttonTop +
|
||||
"px on " +
|
||||
where +
|
||||
", below the fold",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
await page.close();
|
||||
}
|
||||
});
|
||||
|
||||
// --------------------------------------------- confirmation screen (#238)
|
||||
//
|
||||
// The screen that decides what gets signed. The arithmetic underneath it
|
||||
|
||||
Reference in New Issue
Block a user