Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The fix is localized to the click handler, removes the confirmed faulty substring/duplicate-word behavior, and is backed by focused regression tests.
Pull request overview
Fixes mnemonic-word deselection in VerifyMnemonicPage by tracking selected button instances (not text) and rebuilding the phrase from the selected words, which correctly handles whole-word removal and repeated-word buttons.
Changes:
- Replace text-based removal logic with instance-based selection tracking and phrase rebuild.
- Add a dedicated regression test project (with lightweight MAUI stubs) covering deselection, middle-word removal, duplicates, and full selection order.
- Register the new test project in the solution.
File summaries
| File | Description |
|---|---|
| OneGateApp/Pages/VerifyMnemonicPage.xaml.cs | Switch selection state to an ordered List<Button> and rebuild the mnemonic phrase from selected instances. |
| tests/P2-01/MnemonicSelectionTests.cs | Adds regression tests that reproduce the prior bug and validate correct behavior (including duplicate word instances). |
| tests/P2-01/PageStubs.cs | Provides minimal MAUI/page/service stubs so the production click handler can be compiled and exercised in tests. |
| tests/P2-01/MnemonicSelection.Tests.csproj | New xUnit test project wiring in the production VerifyMnemonicPage.xaml.cs plus test dependencies. |
| tests/P2-01/README.md | Documents how to run the regression tests and what they cover. |
| OneGateApp.slnx | Adds the new regression test project to the solution. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Deselecting a mnemonic word removed one character instead of the selected word. Matching by text also could not distinguish repeated-word buttons.
Change
Track selected button instances in order and rebuild the phrase from the selected words. Removing or reselecting one occurrence no longer damages another occurrence or a substring.
This independent PR addresses audit P2-01 only, based on master
623603d.Validation
VerifyMnemonicPagecontrols and handlers. Both passed six checks for whole-word removal, middle-word removal, reselection, independent duplicate occurrences, the complete phrase, and the production validator.Screenshots, native result JSON and simulator-only fixture code remain outside the repository.