fix: Back from Settings never lands on Settings itself #486

Merged
clawbot merged 1 commits from issue-481-settings-back-self into next 2026-10-07 05:26:06 +02:00
6 changed files with 74 additions and 15 deletions
Showing only changes of commit 24b9cf4768 - Show all commits
+8 -5
View File
@@ -1283,11 +1283,14 @@ behind a "···" menu.
Navigation uses a stack model (like iOS): each forward action pushes the current Navigation uses a stack model (like iOS): each forward action pushes the current
screen onto `state.viewStack`, and "Back" pops it (`pushCurrentView()` and screen onto `state.viewStack`, and "Back" pops it (`pushCurrentView()` and
`goBack()` in `src/popup/views/helpers.js`). The root screen is either Welcome `goBack()` in `src/popup/views/helpers.js`). "Back" skips an entry for the
(no wallets) or Home (has wallets). Each screen below gives its view id in screen already showing: ShowRecoveryPhrase and the two delete screens take
parentheses; the registry of view ids is the `VIEWS` array in themselves off the stack when left, so the Settings gear on one of them leaves
`src/popup/views/helpers.js`, and the markup for a screen is the element with id Settings under Settings, and "Back" from there goes to the screen before
`view-` plus that view id in `src/popup/index.html`. Settings. The root screen is either Welcome (no wallets) or Home (has wallets).
Each screen below gives its view id in parentheses; the registry of view ids is
the `VIEWS` array in `src/popup/views/helpers.js`, and the markup for a screen
is the element with id `view-` plus that view id in `src/popup/index.html`.
Three elements sit outside the screens and are present on all of them: the title Three elements sit outside the screens and are present on all of them: the title
bar ("AutistMask by @sneak" plus the Settings gear), the flash message line bar ("AutistMask by @sneak" plus the Settings gear), the flash message line
+9
View File
@@ -45,6 +45,15 @@ but the review is broader than any of them.
# Completed Steps # Completed Steps
- 2026-10-07: Back from Settings no longer shows Settings again after the
settings gear was pressed on the recovery phrase or a delete wallet screen
opened from Settings, with or without a reopen in between
([#481](https://git.eeqj.de/sneak/AutistMask/issues/481)). Those screens take
themselves off the Back stack when left, which leaves Settings under Settings;
`goBack()` now skips an entry for the screen already showing.
`tests/showPhrase.test.js`, `tests/deleteWalletLostPassword.test.js` and
`tests/backNavigation.test.js` drive each path.
- 2026-10-07: `script/discard-dist-on-failure` returns the failed step's own - 2026-10-07: `script/discard-dist-on-failure` returns the failed step's own
exit status even when it cannot write its message, to a closed stderr or to a exit status even when it cannot write its message, to a closed stderr or to a
pipe nobody reads any more pipe nobody reads any more
+13 -2
View File
@@ -216,10 +216,21 @@ function pushCurrentView() {
// Pop the navigation stack and show the previous view. If the stack // Pop the navigation stack and show the previous view. If the stack
// is empty, fall back to the main (home) view. // is empty, fall back to the main (home) view.
//
// An entry for the view already showing is skipped: landing on it would
// make Back seem to do nothing. Settings, the recovery phrase or delete
// wallet screen, then the gear leaves Settings under Settings, because that
// screen takes itself off the stack when left, and a reopened popup cuts it
// off the restored stack the same way
// (https://git.eeqj.de/sneak/AutistMask/issues/481).
function goBack() { function goBack() {
const stack = state.viewStack;
while (stack.length > 0 && stack[stack.length - 1] === state.currentView) {
stack.pop();
}
let target; let target;
if (state.viewStack.length > 0) { if (stack.length > 0) {
target = state.viewStack.pop(); target = stack.pop();
} else { } else {
target = "main"; target = "main";
} }
+18
View File
@@ -52,6 +52,7 @@ const {
resetRenderedViews, resetRenderedViews,
} = require("../src/popup/viewRouter"); } = require("../src/popup/viewRouter");
const { state } = require("../src/shared/state"); const { state } = require("../src/shared/state");
const { restorableStack } = require("../src/shared/persistedState");
const ADDRESS = "0x1111111111111111111111111111111111111111"; const ADDRESS = "0x1111111111111111111111111111111111111111";
const TOKEN = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48"; const TOKEN = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
@@ -209,6 +210,23 @@ describe("Back onto a view the reopened popup never rendered", () => {
}); });
}); });
// https://git.eeqj.de/sneak/AutistMask/issues/481. Settings, the recovery
// phrase or delete wallet screen, the gear, then a reopen: the restored stack
// is cut at the screen the gear left, which leaves Settings under the Settings
// the popup reopens onto.
describe("Back from Settings reopened over its own entry", () => {
test.each(["show-phrase", "delete-wallet-confirm"])(
"goes to the screen under it after leaving %s",
(left) => {
const stored = ["main", "settings", left];
reopenedOn("settings", restorableStack(stored, "settings"));
goBack();
expect(calls).toEqual(["main"]);
expect(state.currentView).toBe("main");
},
);
});
// The guards are restoreView()'s, so a popped view whose backing data is // The guards are restoreView()'s, so a popped view whose backing data is
// gone lands on Home rather than on an empty template. // gone lands on Home rather than on an empty template.
describe("Back onto a view whose backing data is gone", () => { describe("Back onto a view whose backing data is gone", () => {
+7 -5
View File
@@ -618,9 +618,11 @@ describe("the password route's confirm button", () => {
// https://git.eeqj.de/sneak/AutistMask/issues/480: leaving either delete // https://git.eeqj.de/sneak/AutistMask/issues/480: leaving either delete
// screen drops its wallet selection, so Back onto one showed a screen whose // screen drops its wallet selection, so Back onto one showed a screen whose
// button could only answer "No wallet selected for deletion." // button could only answer "No wallet selected for deletion." Taking the
// screen off the stack leaves Settings under Settings, and Back must not land
// there either (https://git.eeqj.de/sneak/AutistMask/issues/481).
describe("Back from Settings after leaving by the settings gear", () => { describe("Back from Settings after leaving by the settings gear", () => {
test("does not land on the delete screen", () => { test("goes past the delete screen to the screen under Settings", () => {
const { helpers, deleteWallet, state } = load(); const { helpers, deleteWallet, state } = load();
deleteWallet.show(1); deleteWallet.show(1);
// The settings gear: push the current view, then show Settings. // The settings gear: push the current view, then show Settings.
@@ -629,10 +631,10 @@ describe("Back from Settings after leaving by the settings gear", () => {
expect(state.viewStack).toEqual(["main", "settings"]); expect(state.viewStack).toEqual(["main", "settings"]);
helpers.goBack(); helpers.goBack();
expect(state.currentView).not.toBe("delete-wallet-confirm"); expect(state.currentView).toBe("main");
}); });
test("does not land on the lost-password screen", async () => { test("goes past the lost-password screen to the screen under Settings", async () => {
const { helpers, deleteWallet, state } = load(); const { helpers, deleteWallet, state } = load();
await openLostPassword(deleteWallet, 1); await openLostPassword(deleteWallet, 1);
// The settings gear: push the current view, then show Settings. // The settings gear: push the current view, then show Settings.
@@ -641,7 +643,7 @@ describe("Back from Settings after leaving by the settings gear", () => {
expect(state.viewStack).toEqual(["main", "settings"]); expect(state.viewStack).toEqual(["main", "settings"]);
helpers.goBack(); helpers.goBack();
expect(state.currentView).not.toBe(VIEW); expect(state.currentView).toBe("main");
}); });
// The lost-password screen's own Back is "Back returns to the delete // The lost-password screen's own Back is "Back returns to the delete
+19 -3
View File
@@ -131,8 +131,10 @@ describe("Back from Settings after leaving by the settings gear", () => {
// https://git.eeqj.de/sneak/AutistMask/issues/461: leaving drops the // https://git.eeqj.de/sneak/AutistMask/issues/461: leaving drops the
// wallet selection, so Back onto this screen showed a password prompt // wallet selection, so Back onto this screen showed a password prompt
// that could only answer "No wallet is selected." // that could only answer "No wallet is selected." Taking the screen off
test("does not land on the recovery phrase screen", () => { // the stack leaves Settings under Settings, and Back must not land there
// either (https://git.eeqj.de/sneak/AutistMask/issues/481).
test("goes to the screen under Settings", () => {
const { helpers, state, showPhrase } = load(); const { helpers, state, showPhrase } = load();
// Opened from the wallet list in Settings, then left by the gear: // Opened from the wallet list in Settings, then left by the gear:
@@ -143,7 +145,21 @@ describe("Back from Settings after leaving by the settings gear", () => {
expect(state.viewStack).toEqual(["main", "settings"]); expect(state.viewStack).toEqual(["main", "settings"]);
helpers.goBack(); helpers.goBack();
expect(state.currentView).not.toBe(SHOW_PHRASE_VIEW); expect(state.currentView).toBe("main");
});
// Each round trip leaves one more Settings under Settings.
test("goes to the screen under Settings after two round trips", () => {
const { helpers, state, showPhrase } = load();
for (let i = 0; i < 2; i++) {
showPhrase.show(0);
helpers.pushCurrentView();
helpers.showView("settings");
}
expect(state.viewStack).toEqual(["main", "settings", "settings"]);
helpers.goBack();
expect(state.currentView).toBe("main");
}); });
// This Back takes Settings off the stack before the screen is left, so // This Back takes Settings off the stack before the screen is left, so