Cluster 1 — code quality + dialog XamlRoot fix + CSV importer hardening [v2.0]#42
Merged
Conversation
…t fix Code quality / modernization (Cluster 1 of PassKey 2.0): - Extract BaseDetailViewModel<TEntry> with template-method pattern; the four Detail VMs (Password, CreditCard, Identity, SecureNote) now inherit shared add/edit/save/delete plumbing (~200 LOC of duplication removed) - Introduce IVaultEntry interface as the generic constraint for BaseDetailViewModel - Replace Task.Run(async () => await …) anti-pattern in LoginVM/SetupVM with Task.Run(Func<Task>) so the KDF/decrypt work stays off the UI thread without the nested async-over-async wrapper - Implement IDisposable on MainVM and the four List VMs; named handlers replace closures so VaultLocked / NavigationStack.CurrentChanged can be unsubscribed, and Entries collections are cleared on vault lock to avoid retaining decrypted data in memory after lock - Extract Maestro/Visa/Amex/Discover/Diners BIN prefixes in CardTypeDetector to a dedicated BinPrefixes static class with explicit named constants Dialog / UX fixes: - Pre-existing bug: ContentDialog.ShowAsync threw "This element does not have a XamlRoot" on every delete confirmation triggered from a ViewModel (Detail and List). Introduce IDialogQueueService.ConfirmAsync + a lazy XamlRootAccessor delegate that resolves Window.Content.XamlRoot just-in-time on the UI thread, removing the duplicated factory pattern at five call sites and fixing the crash - Pre-existing UX bug: importing an empty / unrecognised file silently advanced to the merge dialog and then showed "Import completed" with zero changes. SettingsViewModel.ImportAsync now surfaces an explicit "no recognisable entries" error when the parsed vault is empty CSV importer (NordPass support): - Add "note" (singular) as alias for the Notes column header - Detect a "type" column and skip non-password rows so card / identity / note labels do not pollute the imported password list (proper cross-type import is tracked for a future cluster) - Two new unit tests cover the NordPass-shape header and the singular-note alias Test suite: 178 passed (was 176). Verified locally: build-installer.ps1 produces both PassKey-Setup-x64.exe and PassKey-Portable-x64.zip; user-facing smoke test (delete on all four entry types, import of a real NordPass CSV with 238 password rows, lock/unlock cycle) confirms no regressions and that the previously broken delete + import flows now work.
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.
First cluster of the PassKey 2.0 refactor described in
piano-revisione-correzione-miglioramento.md(private notes file).Summary
Code quality / modernization
BaseDetailViewModel<TEntry>with a template-method pattern; the four Detail VMs (Password, CreditCard, Identity, SecureNote) inherit shared add/edit/save/delete plumbing — ~200 LOC of duplication removed.IVaultEntryinterface as the generic constraint.Task.Run(async () => await …)in LoginVM/SetupVM withTask.Run(Func<Task>)to keep KDF/decrypt off the UI thread without the nested async-over-async wrapper.IDisposableon MainVM and the four List VMs; named handlers replace closures soVaultLocked/NavigationStack.CurrentChangedcan be unsubscribed, andEntriescollections are cleared on vault lock to avoid retaining decrypted data after lock.CardTypeDetectorto a dedicatedBinPrefixesstatic class.Dialog / UX fixes
ContentDialog.ShowAsyncthrew"This element does not have a XamlRoot"on every delete confirmation triggered from a ViewModel. NewIDialogQueueService.ConfirmAsync+ lazyXamlRootAccessordelegate resolvesWindow.Content.XamlRootjust-in-time on the UI thread; the duplicated factory pattern is removed from 5 call sites and the crash is fixed.SettingsViewModel.ImportAsyncnow surfaces an explicit error when the parsed vault is empty.CSV importer — NordPass support
note(singular) as alias for theNotescolumn header.typecolumn and skip non-password rows so card / identity / note labels do not pollute the imported password list (richer cross-type import will land in a later cluster).Test plan
dotnet test→ 178 / 178 (was 176; +2 NordPass cases)scripts/build-installer.ps1produces bothPassKey-Setup-x64.exeandPassKey-Portable-x64.zip