fix: implement proper view navigation stack #146

Merged

sneak merged 1 commits from fix/issue-134-view-stack into main 2026-03-02 00:15:01 +01:00

Conversation 2 Commits 1 Files Changed 16 +158 -52

clawbot commented 2026-03-01 22:23:41 +01:00

Collaborator

Copy Link

Summary

Fixes the view stack pop bug where pressing Back in Settings (or any view) always returned to Main instead of the previous view.

Closes issue #134

Problem

The popup UI had no navigation stack. Every back button was hardcoded to a specific destination (usually Main). The reported path:

Main → Address → Transaction → Settings (gear icon) → Back

...would go to Main instead of returning to the Transaction view.

Solution

Implemented a proper view navigation stack (like iOS) as already described in the README:

• viewStack array added to persisted state — survives popup close/reopen
• pushCurrentView() — pushes the current view name onto the stack before any forward navigation
• goBack() — pops the stack and shows the previous view; falls back to Main if the stack is empty; re-renders the wallet list when returning to Main
• clearViewStack() — resets the stack for root transitions (e.g., after adding/deleting a wallet)

What Changed

1. helpers.js — Added navigation stack functions (pushCurrentView, goBack, clearViewStack, setRenderMain)
2. state.js — Added viewStack to persisted state
3. index.js — All ctx.show*() wrappers now push before navigating forward; gear button uses stack for toggle behavior
4. All view back buttons — Replaced hardcoded destinations with goBack() (settings, addressDetail, addressToken, transactionDetail, send, receive, addToken, confirmTx, addWallet, settingsAddToken, deleteWallet, export-privkey)
5. Direct showView() forward navigations — Added pushCurrentView() calls before showView("send") in addressDetail, addressToken, and home; before showView("export-privkey") in addressDetail; before deleteWallet.show() in settings
6. Reset-to-root transitions — clearViewStack() called after adding a wallet (all 3 import types), after deleting the last wallet, and after transaction completion (Done button)

Navigation Paths Verified

• Main → Settings → Back → returns to Main ✓
• Main → Address → Settings → Back → returns to Address ✓
• Main → Address → Transaction → Settings → Back → returns to Transaction ✓ (the reported bug)
• Main → Address → Token → Send → ConfirmTx → Back → Back → Back → Back → unwinds correctly through each view back to Main ✓
• Main → Address → Token → Transaction → Settings → Back → returns to Transaction ✓
• Settings → Add Wallet → (add) → Main → stack cleared, fresh root ✓
• Settings → Delete Wallet → Back → returns to Settings ✓
• Settings → Delete Wallet → (confirm) → stack reset to [main], settings shown ✓
• Address → Send → ConfirmTx → (broadcast) → SuccessTx → Done → stack reset, returns to address context ✓
• Popup close/reopen → viewStack persisted, back navigation still works ✓

## Summary Fixes the view stack pop bug where pressing Back in Settings (or any view) always returned to Main instead of the previous view. Closes [issue #134](https://git.eeqj.de/sneak/AutistMask/issues/134) ## Problem The popup UI had no navigation stack. Every back button was hardcoded to a specific destination (usually Main). The reported path: > Main → Address → Transaction → Settings (gear icon) → Back ...would go to Main instead of returning to the Transaction view. ## Solution Implemented a proper view navigation stack (like iOS) as already described in the README: - **`viewStack`** array added to persisted state — survives popup close/reopen - **`pushCurrentView()`** — pushes the current view name onto the stack before any forward navigation - **`goBack()`** — pops the stack and shows the previous view; falls back to Main if the stack is empty; re-renders the wallet list when returning to Main - **`clearViewStack()`** — resets the stack for root transitions (e.g., after adding/deleting a wallet) ### What Changed 1. **helpers.js** — Added navigation stack functions (`pushCurrentView`, `goBack`, `clearViewStack`, `setRenderMain`) 2. **state.js** — Added `viewStack` to persisted state 3. **index.js** — All `ctx.show*()` wrappers now push before navigating forward; gear button uses stack for toggle behavior 4. **All view back buttons** — Replaced hardcoded destinations with `goBack()` (settings, addressDetail, addressToken, transactionDetail, send, receive, addToken, confirmTx, addWallet, settingsAddToken, deleteWallet, export-privkey) 5. **Direct `showView()` forward navigations** — Added `pushCurrentView()` calls before `showView("send")` in addressDetail, addressToken, and home; before `showView("export-privkey")` in addressDetail; before `deleteWallet.show()` in settings 6. **Reset-to-root transitions** — `clearViewStack()` called after adding a wallet (all 3 import types), after deleting the last wallet, and after transaction completion (Done button) ### Navigation Paths Verified - **Main → Settings → Back** → returns to Main ✓ - **Main → Address → Settings → Back** → returns to Address ✓ - **Main → Address → Transaction → Settings → Back** → returns to Transaction ✓ (the reported bug) - **Main → Address → Token → Send → ConfirmTx → Back → Back → Back → Back** → unwinds correctly through each view back to Main ✓ - **Main → Address → Token → Transaction → Settings → Back** → returns to Transaction ✓ - **Settings → Add Wallet → (add) → Main** → stack cleared, fresh root ✓ - **Settings → Delete Wallet → Back** → returns to Settings ✓ - **Settings → Delete Wallet → (confirm)** → stack reset to [main], settings shown ✓ - **Address → Send → ConfirmTx → (broadcast) → SuccessTx → Done** → stack reset, returns to address context ✓ - **Popup close/reopen** → viewStack persisted, back navigation still works ✓

clawbot added the

needs-review

bot

labels 2026-03-01 22:23:41 +01:00

clawbot added 1 commit 2026-03-01 22:23:41 +01:00

fix: implement proper view navigation stack ... 0ab202e998

The popup UI had no navigation stack — all back buttons were hardcoded
to specific destinations. This meant navigating Main → Address →
Transaction → Settings → Back would go to Main instead of Transaction.

Changes:
- Add viewStack array to persisted state (survives popup close/reopen)
- Add pushCurrentView(), goBack(), clearViewStack() helpers in helpers.js
- goBack() pops the stack and shows the previous view; falls back to
  Main if the stack is empty; re-renders the wallet list when returning
  to the Main view
- All ctx.show*() wrappers in index.js now push the current view before
  navigating forward
- All back buttons across every view now use goBack() instead of
  hardcoded destinations
- Direct showView() calls for forward navigation (Send, Export Private
  Key) push before switching
- Stack is cleared on reset-to-root actions: adding a wallet, deleting
  the last wallet
- After wallet deletion with remaining wallets, stack is reset to
  [main] and settings is re-shown
- After transaction completion (Done button), stack is reset to the
  appropriate address context

Closes #134

clawbot commented 2026-03-01 22:27:15 +01:00

Author

Collaborator

Copy Link

✅ Review: PASS

Summary

Clean, well-structured implementation of a proper push/pop navigation stack that fixes the view stack pop bug described in #134.

What was reviewed

• 16 files changed (+158 / -52) — all navigation-related, no unrelated changes
• No modifications to Dockerfile, Makefile, package.json, prettier config, test config, or CI workflows
• Single clean commit: fix: implement proper view navigation stack
• docker build . passes (including make check)

Architecture

The implementation adds a viewStack array to persisted state with three core functions in helpers.js:

1. pushCurrentView() — pushes state.currentView before any forward navigation
2. goBack() — pops the stack and shows the previous view; falls back to main if empty; calls _renderMain() when returning to main to refresh the wallet list
3. clearViewStack() — resets the stack for root transitions (after wallet add/delete, transaction completion)

Bug fix verification

Traced the exact bug scenario from issue #134:

Step	Action	Stack After	currentView
1	Start	[]	main
2	Click address	["main"]	address
3	Click transaction	["main", "address"]	transaction
4	Click gear	["main", "address", "transaction"]	settings
5	Click back	["main", "address"]	transaction ✅

Previously step 5 would go to main. Now it correctly returns to transaction.

Design choices verified

• Gear button toggle: If settings is already visible, clicking gear calls goBack() (toggle off). If hidden, it pushes current view and opens settings. Correct.
• Root resets: clearViewStack() is called after all 3 wallet import paths, after deleting the last wallet, and after transaction completion (Done button). Prevents stale stack entries.
• addToken completion: Manually pops the stack and calls addressDetail.show() directly so the user sees the newly-added token. Stack integrity is maintained.
• deleteWallet (non-last): Resets stack to ["main"] and shows settings. Back from settings → main. Correct.
• txStatus navigateBack: Resets stack to ["main"] (or ["main", "address"]) and shows the address view. Back navigates sensibly after tx completion.
• Lazy require(): Used in deleteWallet.js and txStatus.js to avoid circular dependencies. Standard Node.js pattern, works correctly since modules are fully loaded by the time click handlers fire.
• Persistence: viewStack is persisted via saveState() (triggered by showView()), survives popup close/reopen.
• Guard against null: pushCurrentView() checks state.currentView is truthy before pushing.
• loadState defense: Array.isArray(saved.viewStack) ? saved.viewStack : [] handles missing/corrupt data.

No issues found

No test weakening, no config changes, no unrelated modifications. The implementation is minimal and focused.

## ✅ Review: PASS ### Summary Clean, well-structured implementation of a proper push/pop navigation stack that fixes the view stack pop bug described in #134. ### What was reviewed - **16 files changed** (+158 / -52) — all navigation-related, no unrelated changes - No modifications to Dockerfile, Makefile, package.json, prettier config, test config, or CI workflows - Single clean commit: `fix: implement proper view navigation stack` - `docker build .` passes (including `make check`) ### Architecture The implementation adds a `viewStack` array to persisted state with three core functions in `helpers.js`: 1. **`pushCurrentView()`** — pushes `state.currentView` before any forward navigation 2. **`goBack()`** — pops the stack and shows the previous view; falls back to main if empty; calls `_renderMain()` when returning to main to refresh the wallet list 3. **`clearViewStack()`** — resets the stack for root transitions (after wallet add/delete, transaction completion) ### Bug fix verification Traced the exact bug scenario from issue #134: | Step | Action | Stack After | currentView | |------|--------|-------------|-------------| | 1 | Start | `[]` | main | | 2 | Click address | `["main"]` | address | | 3 | Click transaction | `["main", "address"]` | transaction | | 4 | Click gear | `["main", "address", "transaction"]` | settings | | 5 | Click back | `["main", "address"]` | **transaction** ✅ | Previously step 5 would go to main. Now it correctly returns to transaction. ### Design choices verified - **Gear button toggle**: If settings is already visible, clicking gear calls `goBack()` (toggle off). If hidden, it pushes current view and opens settings. Correct. - **Root resets**: `clearViewStack()` is called after all 3 wallet import paths, after deleting the last wallet, and after transaction completion (Done button). Prevents stale stack entries. - **addToken completion**: Manually pops the stack and calls `addressDetail.show()` directly so the user sees the newly-added token. Stack integrity is maintained. - **deleteWallet (non-last)**: Resets stack to `["main"]` and shows settings. Back from settings → main. Correct. - **txStatus navigateBack**: Resets stack to `["main"]` (or `["main", "address"]`) and shows the address view. Back navigates sensibly after tx completion. - **Lazy `require()`**: Used in deleteWallet.js and txStatus.js to avoid circular dependencies. Standard Node.js pattern, works correctly since modules are fully loaded by the time click handlers fire. - **Persistence**: `viewStack` is persisted via `saveState()` (triggered by `showView()`), survives popup close/reopen. - **Guard against null**: `pushCurrentView()` checks `state.currentView` is truthy before pushing. - **loadState defense**: `Array.isArray(saved.viewStack) ? saved.viewStack : []` handles missing/corrupt data. ### No issues found No test weakening, no config changes, no unrelated modifications. The implementation is minimal and focused.

clawbot added

merge-ready

and removed

needs-review

bot

labels 2026-03-01 22:27:38 +01:00

sneak was assigned by clawbot 2026-03-01 22:27:51 +01:00

clawbot commented 2026-03-01 22:29:34 +01:00

Author

Collaborator

Copy Link

Code Review: PASS ✅

Reviewing PR #146 which closes issue #134 (view stack pop bug).

Issue Requirements Checklist

• Navigation from Main → Address → Transaction → Settings → Back returns to Transaction (not Main) — The gear button now calls pushCurrentView() before showing settings, and settings back button calls goBack() which pops the stack correctly.
• Proper view stack is maintained (not hardcoded back destinations) — All back buttons replaced with goBack(), all forward navigations push via pushCurrentView().
• Stack answers sneak's question: "are we actually keeping a stack?" — Yes. viewStack array in persisted state implements a proper push/pop navigation stack, as already described in the README's screen map ("Navigation uses a stack model (like iOS)").

Implementation Review

Core stack functions (helpers.js):

• pushCurrentView() — correctly captures state.currentView before any forward navigation. No duplicate push risk since it's called once per transition.
• goBack() — pops stack, falls back to "main" on empty stack, re-renders wallet list when returning to main via _renderMain callback. Sound design.
• clearViewStack() — properly resets stack for root transitions.
• setRenderMain() — clean callback pattern to avoid circular dependencies.

Forward navigation (index.js ctx wrappers): All 9 ctx.show*() wrappers consistently call pushCurrentView() before the target view's show(). Direct pushCurrentView() + showView() calls in addressDetail.js, addressToken.js, home.js, and settings.js for inline navigations (Send, Export Private Key, Delete Wallet) are also correct.

Back navigation: Every view's back button now uses goBack() instead of hardcoded destinations — settings, addressDetail, addressToken, transactionDetail, send, receive, addToken, confirmTx, addWallet, settingsAddToken, deleteWallet, export-privkey.

Edge cases handled correctly:

• Empty stack: goBack() falls back to "main" — safe default.
• Popup close/reopen: viewStack persisted to extension storage via saveState() (called by showView()), restored with Array.isArray() guard in loadState(). Stack survives popup lifecycle.
• Wallet add (all 3 import types): clearViewStack() before showView("main") — clean root reset.
• Wallet delete (last): clearViewStack() then showView("welcome") — correct.
• Wallet delete (non-last): clearViewStack(), push "main", show settings — back from settings goes to main. Correct.
• Transaction completion (txStatus.js): clearViewStack(), push "main" (and optionally "address"), show address context — clean navigation path home after tx. Correct.
• Add token success (addToken.js): Manual stack pop + direct addressDetail.show() — correctly returns to address detail with re-render so new token is visible immediately.
• Gear button toggle: Correctly pushes before showing settings, calls goBack() when settings already visible. Works from any context (main, address, transaction, etc.).
• No unbounded stack growth: Stack clears on major transitions (wallet add/delete, tx completion). Max depth bounded by navigation depth (~4-5 in deepest paths).

Persistence correctness: pushCurrentView() modifies state.viewStack in memory; the subsequent showView() call always triggers saveState() which persists the updated stack. clearViewStack() is always followed by either showView() or explicit saveState(). No window for data loss.

Cheating Check

• No Makefile changes ✅
• No CI/workflow changes ✅
• No linter config changes ✅
• No test assertion modifications ✅
• Only popup source files and state.js modified (16 files, all application code) ✅

Build Verification

docker build . passes (includes make check — formatting, linting, tests).

Files Changed

16 files — state.js, helpers.js, index.js, and all view modules. Changes are systematic and consistent. No stray modifications.

Branch is up-to-date with main (1 commit ahead, 0 behind). No rebase needed.

## Code Review: PASS ✅ Reviewing [PR #146](https://git.eeqj.de/sneak/AutistMask/pulls/146) which closes [issue #134](https://git.eeqj.de/sneak/AutistMask/issues/134) (view stack pop bug). ### Issue Requirements Checklist - [x] **Navigation from Main → Address → Transaction → Settings → Back returns to Transaction (not Main)** — The gear button now calls `pushCurrentView()` before showing settings, and settings back button calls `goBack()` which pops the stack correctly. - [x] **Proper view stack is maintained (not hardcoded back destinations)** — All back buttons replaced with `goBack()`, all forward navigations push via `pushCurrentView()`. - [x] **Stack answers sneak's question: "are we actually keeping a stack?"** — Yes. `viewStack` array in persisted state implements a proper push/pop navigation stack, as already described in the README's screen map ("Navigation uses a stack model (like iOS)"). ### Implementation Review **Core stack functions** (`helpers.js`): - `pushCurrentView()` — correctly captures `state.currentView` before any forward navigation. No duplicate push risk since it's called once per transition. - `goBack()` — pops stack, falls back to "main" on empty stack, re-renders wallet list when returning to main via `_renderMain` callback. Sound design. - `clearViewStack()` — properly resets stack for root transitions. - `setRenderMain()` — clean callback pattern to avoid circular dependencies. **Forward navigation** (`index.js` ctx wrappers): All 9 `ctx.show*()` wrappers consistently call `pushCurrentView()` before the target view's `show()`. Direct `pushCurrentView()` + `showView()` calls in `addressDetail.js`, `addressToken.js`, `home.js`, and `settings.js` for inline navigations (Send, Export Private Key, Delete Wallet) are also correct. **Back navigation**: Every view's back button now uses `goBack()` instead of hardcoded destinations — settings, addressDetail, addressToken, transactionDetail, send, receive, addToken, confirmTx, addWallet, settingsAddToken, deleteWallet, export-privkey. **Edge cases handled correctly:** - **Empty stack**: `goBack()` falls back to "main" — safe default. - **Popup close/reopen**: `viewStack` persisted to extension storage via `saveState()` (called by `showView()`), restored with `Array.isArray()` guard in `loadState()`. Stack survives popup lifecycle. - **Wallet add** (all 3 import types): `clearViewStack()` before `showView("main")` — clean root reset. - **Wallet delete (last)**: `clearViewStack()` then `showView("welcome")` — correct. - **Wallet delete (non-last)**: `clearViewStack()`, push "main", show settings — back from settings goes to main. Correct. - **Transaction completion** (`txStatus.js`): `clearViewStack()`, push "main" (and optionally "address"), show address context — clean navigation path home after tx. Correct. - **Add token success** (`addToken.js`): Manual stack pop + direct `addressDetail.show()` — correctly returns to address detail with re-render so new token is visible immediately. - **Gear button toggle**: Correctly pushes before showing settings, calls `goBack()` when settings already visible. Works from any context (main, address, transaction, etc.). - **No unbounded stack growth**: Stack clears on major transitions (wallet add/delete, tx completion). Max depth bounded by navigation depth (~4-5 in deepest paths). **Persistence correctness**: `pushCurrentView()` modifies `state.viewStack` in memory; the subsequent `showView()` call always triggers `saveState()` which persists the updated stack. `clearViewStack()` is always followed by either `showView()` or explicit `saveState()`. No window for data loss. ### Cheating Check - No Makefile changes ✅ - No CI/workflow changes ✅ - No linter config changes ✅ - No test assertion modifications ✅ - Only popup source files and state.js modified (16 files, all application code) ✅ ### Build Verification `docker build .` passes (includes `make check` — formatting, linting, tests). ### Files Changed 16 files — `state.js`, `helpers.js`, `index.js`, and all view modules. Changes are systematic and consistent. No stray modifications. Branch is up-to-date with `main` (1 commit ahead, 0 behind). No rebase needed.

sneak merged commit a22f33d511 into main 2026-03-02 00:15:01 +01:00

sneak deleted branch fix/issue-134-view-stack 2026-03-02 00:15:02 +01:00

sneak referenced this issue from a commit 2026-03-02 00:15:02 +01:00

fix: implement proper view navigation stack (#146)

clawbot referenced this pull request 2026-03-02 00:16:48 +01:00

feat: add About well to settings with build info and debug easter egg #145

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bot

Agent's turn to work

  

merge-ready

  

needs-checks

  

needs-rebase

  

needs-review

  

needs-rework

No Label

bot

merge-ready

needs-checks

needs-rebase

needs-review

needs-rework

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

sneak

1 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#146

No description provided.