diff --git a/README.md b/README.md index 95ef211..79505b3 100644 --- a/README.md +++ b/README.md @@ -1283,11 +1283,14 @@ behind a "ยทยทยท" menu. Navigation uses a stack model (like iOS): each forward action pushes the current screen onto `state.viewStack`, and "Back" pops it (`pushCurrentView()` and -`goBack()` in `src/popup/views/helpers.js`). 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`. +`goBack()` in `src/popup/views/helpers.js`). "Back" skips an entry for the +screen already showing: ShowRecoveryPhrase and the two delete screens take +themselves off the stack when left, so the Settings gear on one of them leaves +Settings under Settings, and "Back" from there goes to the screen before +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 bar ("AutistMask by @sneak" plus the Settings gear), the flash message line diff --git a/TODO.md b/TODO.md index a51de03..cb85906 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,15 @@ but the review is broader than any of them. # 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 exit status even when it cannot write its message, to a closed stderr or to a pipe nobody reads any more diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index 04bffc6..a9687a2 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -216,10 +216,21 @@ function pushCurrentView() { // Pop the navigation stack and show the previous view. If the stack // 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() { + const stack = state.viewStack; + while (stack.length > 0 && stack[stack.length - 1] === state.currentView) { + stack.pop(); + } let target; - if (state.viewStack.length > 0) { - target = state.viewStack.pop(); + if (stack.length > 0) { + target = stack.pop(); } else { target = "main"; } diff --git a/tests/backNavigation.test.js b/tests/backNavigation.test.js index 1b4ff48..1ed246a 100644 --- a/tests/backNavigation.test.js +++ b/tests/backNavigation.test.js @@ -52,6 +52,7 @@ const { resetRenderedViews, } = require("../src/popup/viewRouter"); const { state } = require("../src/shared/state"); +const { restorableStack } = require("../src/shared/persistedState"); const ADDRESS = "0x1111111111111111111111111111111111111111"; 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 // gone lands on Home rather than on an empty template. describe("Back onto a view whose backing data is gone", () => { diff --git a/tests/deleteWalletLostPassword.test.js b/tests/deleteWalletLostPassword.test.js index 8e23c2f..4cc8877 100644 --- a/tests/deleteWalletLostPassword.test.js +++ b/tests/deleteWalletLostPassword.test.js @@ -618,9 +618,11 @@ describe("the password route's confirm button", () => { // https://git.eeqj.de/sneak/AutistMask/issues/480: leaving either delete // 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", () => { - test("does not land on the delete screen", () => { + test("goes past the delete screen to the screen under Settings", () => { const { helpers, deleteWallet, state } = load(); deleteWallet.show(1); // 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"]); 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(); await openLostPassword(deleteWallet, 1); // 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"]); 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 diff --git a/tests/showPhrase.test.js b/tests/showPhrase.test.js index 54a0b3d..686f32f 100644 --- a/tests/showPhrase.test.js +++ b/tests/showPhrase.test.js @@ -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 // wallet selection, so Back onto this screen showed a password prompt - // that could only answer "No wallet is selected." - test("does not land on the recovery phrase screen", () => { + // that could only answer "No wallet is selected." 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). + test("goes to the screen under Settings", () => { const { helpers, state, showPhrase } = load(); // 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"]); 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