From 59bcfdab7b1cc60f9bca430ff0ea649fed22bb2d Mon Sep 17 00:00:00 2001 From: alvroble <50918598+alvroble@users.noreply.github.com> Date: Sun, 30 Jun 2024 13:48:35 +0200 Subject: [PATCH 1/5] Add exit dialog when entering passphrase Fixes / required changes: * Issue #510 'Behavior of back button confusing when entering password' * Updated test suite accordingly --- src/seedsigner/gui/screens/seed_screens.py | 4 +- src/seedsigner/views/seed_views.py | 44 +++++++++++++++++++--- tests/screenshot_generator/generator.py | 1 + tests/test_flows_psbt.py | 2 +- tests/test_flows_seed.py | 9 ++++- tests/test_flows_tools.py | 2 +- 6 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/seedsigner/gui/screens/seed_screens.py b/src/seedsigner/gui/screens/seed_screens.py index 9fe35c1c..b20e8ba1 100644 --- a/src/seedsigner/gui/screens/seed_screens.py +++ b/src/seedsigner/gui/screens/seed_screens.py @@ -850,11 +850,11 @@ class SeedAddPassphraseScreen(BaseTopNavScreen): self.hw_button3.is_selected = True self.hw_button3.render() self.renderer.show_image() - return self.passphrase + return self.passphrase, None elif input == HardwareButtonsConstants.KEY_PRESS and self.top_nav.is_selected: # Back button clicked - return self.top_nav.selected_button + return self.passphrase, self.top_nav.selected_button # Check for keyboard swaps if input == HardwareButtonsConstants.KEY1: diff --git a/src/seedsigner/views/seed_views.py b/src/seedsigner/views/seed_views.py index 9ec874da..98c2f387 100644 --- a/src/seedsigner/views/seed_views.py +++ b/src/seedsigner/views/seed_views.py @@ -323,19 +323,51 @@ class SeedAddPassphraseView(View): def run(self): - ret = self.run_screen(seed_screens.SeedAddPassphraseScreen, passphrase=self.seed.passphrase) + ret_passphrase, ret_btn = self.run_screen(seed_screens.SeedAddPassphraseScreen, passphrase=self.seed.passphrase) - if ret == RET_CODE__BACK_BUTTON: - return Destination(BackStackView) - # The new passphrase will be the return value; it might be empty. - self.seed.set_passphrase(ret) - if len(self.seed.passphrase) > 0: + self.seed.set_passphrase(ret_passphrase) + if ret_btn == RET_CODE__BACK_BUTTON: + if len(self.seed.passphrase) > 0: + return Destination(SeedAddPassphraseExitDialogView) + else: + return Destination(BackStackView) + elif len(self.seed.passphrase) > 0: return Destination(SeedReviewPassphraseView) else: return Destination(SeedFinalizeView) +class SeedAddPassphraseExitDialogView(View): + EXIT = ("Exit", None, None, "red") + CONTINUE = "Continue editing" + + def __init__(self): + super().__init__() + self.seed = self.controller.storage.get_pending_seed() + + + def run(self): + passphrase = self.seed.passphrase + + button_data = [self.EXIT, self.CONTINUE] + + selected_menu_num = self.run_screen( + WarningScreen, + title="Exit", + status_headline=None, + text=f"Please confirm that you want to exit", + show_back_button=False, + button_data=button_data, + ) + + if button_data[selected_menu_num] == self.EXIT: + self.seed.set_passphrase("") + return Destination(SeedFinalizeView) + + elif button_data[selected_menu_num] == self.CONTINUE: + return Destination(SeedAddPassphraseView) + class SeedReviewPassphraseView(View): """ diff --git a/tests/screenshot_generator/generator.py b/tests/screenshot_generator/generator.py index 278df338..6b7effe1 100644 --- a/tests/screenshot_generator/generator.py +++ b/tests/screenshot_generator/generator.py @@ -134,6 +134,7 @@ def test_generate_screenshots(target_locale): seed_views.SeedMnemonicInvalidView, seed_views.SeedFinalizeView, seed_views.SeedAddPassphraseView, + seed_views.SeedAddPassphraseExitDialogView, seed_views.SeedReviewPassphraseView, (seed_views.SeedOptionsView, dict(seed_num=0)), diff --git a/tests/test_flows_psbt.py b/tests/test_flows_psbt.py index 3f487380..384ca760 100644 --- a/tests/test_flows_psbt.py +++ b/tests/test_flows_psbt.py @@ -78,7 +78,7 @@ class TestPSBTFlows(FlowTest): FlowStep(psbt_views.PSBTSelectSeedView, button_data_selection=psbt_views.PSBTSelectSeedView.SCAN_SEED), FlowStep(scan_views.ScanSeedQRView, before_run=load_seed_into_decoder), FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value="abc"), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("abc", None)), FlowStep(seed_views.SeedReviewPassphraseView, button_data_selection=seed_views.SeedReviewPassphraseView.DONE), FlowStep(seed_views.SeedOptionsView, is_redirect=True), FlowStep(psbt_views.PSBTOverviewView), diff --git a/tests/test_flows_seed.py b/tests/test_flows_seed.py index e71ad0b5..2191f563 100644 --- a/tests/test_flows_seed.py +++ b/tests/test_flows_seed.py @@ -41,9 +41,14 @@ class TestSeedFlows(FlowTest): FlowStep(MainMenuView, button_data_selection=MainMenuView.SCAN), FlowStep(scan_views.ScanView, before_run=load_seed_into_decoder), # simulate read SeedQR; ret val is ignored FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value="muhpassphrase"), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase",RET_CODE__BACK_BUTTON)), + FlowStep(seed_views.SeedAddPassphraseExitDialogView, button_data_selection=seed_views.SeedAddPassphraseExitDialogView.EXIT), + FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase",RET_CODE__BACK_BUTTON)), + FlowStep(seed_views.SeedAddPassphraseExitDialogView, button_data_selection=seed_views.SeedAddPassphraseExitDialogView.CONTINUE), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase",None)), FlowStep(seed_views.SeedReviewPassphraseView, button_data_selection=seed_views.SeedReviewPassphraseView.EDIT), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value="muhpassphrase2"), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase2",None)), FlowStep(seed_views.SeedReviewPassphraseView, button_data_selection=seed_views.SeedReviewPassphraseView.DONE), FlowStep(seed_views.SeedOptionsView), ]) diff --git a/tests/test_flows_tools.py b/tests/test_flows_tools.py index f24f5a23..3563715a 100644 --- a/tests/test_flows_tools.py +++ b/tests/test_flows_tools.py @@ -65,7 +65,7 @@ class TestToolsFlows(FlowTest): self.run_sequence( sequence=[ FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value="mypassphrase"), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("mypassphrase",None)), FlowStep(seed_views.SeedReviewPassphraseView, button_data_selection=seed_views.SeedReviewPassphraseView.DONE), FlowStep(seed_views.SeedOptionsView, is_redirect=True), FlowStep(seed_views.SeedExportXpubScriptTypeView), From 1670bf678ef70bba4d9ef9d90e10c831e82161df Mon Sep 17 00:00:00 2001 From: alvroble <50918598+alvroble@users.noreply.github.com> Date: Sun, 7 Jul 2024 12:06:37 +0200 Subject: [PATCH 2/5] Modified dialog text and button order --- src/seedsigner/views/seed_views.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/seedsigner/views/seed_views.py b/src/seedsigner/views/seed_views.py index 98c2f387..6e7af186 100644 --- a/src/seedsigner/views/seed_views.py +++ b/src/seedsigner/views/seed_views.py @@ -338,9 +338,10 @@ class SeedAddPassphraseView(View): return Destination(SeedFinalizeView) + class SeedAddPassphraseExitDialogView(View): - EXIT = ("Exit", None, None, "red") CONTINUE = "Continue editing" + EXIT = ("Exit", None, None, "red") def __init__(self): super().__init__() @@ -348,25 +349,24 @@ class SeedAddPassphraseExitDialogView(View): def run(self): - passphrase = self.seed.passphrase - - button_data = [self.EXIT, self.CONTINUE] + button_data = [self.CONTINUE, self.EXIT] selected_menu_num = self.run_screen( WarningScreen, - title="Exit", + title="Abandon passphrase?", status_headline=None, - text=f"Please confirm that you want to exit", + text=f"Exiting will abandon the BIP-39 Passphrase", show_back_button=False, button_data=button_data, ) - if button_data[selected_menu_num] == self.EXIT: + if button_data[selected_menu_num] == self.CONTINUE: + return Destination(SeedAddPassphraseView) + + elif button_data[selected_menu_num] == self.EXIT: self.seed.set_passphrase("") return Destination(SeedFinalizeView) - elif button_data[selected_menu_num] == self.CONTINUE: - return Destination(SeedAddPassphraseView) class SeedReviewPassphraseView(View): From 530e334c6b1e8a328d3bd7d3178b8bfdba096b6a Mon Sep 17 00:00:00 2001 From: alvroble <50918598+alvroble@users.noreply.github.com> Date: Sun, 7 Jul 2024 20:31:08 +0200 Subject: [PATCH 3/5] Updated dialog text and accordingly better variable naming --- src/seedsigner/views/seed_views.py | 14 +++++++------- tests/test_flows_seed.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/seedsigner/views/seed_views.py b/src/seedsigner/views/seed_views.py index 6e7af186..51ff9185 100644 --- a/src/seedsigner/views/seed_views.py +++ b/src/seedsigner/views/seed_views.py @@ -340,8 +340,8 @@ class SeedAddPassphraseView(View): class SeedAddPassphraseExitDialogView(View): - CONTINUE = "Continue editing" - EXIT = ("Exit", None, None, "red") + EDIT = "Edit passphrase" + DISCARD = ("Discard passphrase", None, None, "red") def __init__(self): super().__init__() @@ -349,21 +349,21 @@ class SeedAddPassphraseExitDialogView(View): def run(self): - button_data = [self.CONTINUE, self.EXIT] + button_data = [self.EDIT, self.DISCARD] selected_menu_num = self.run_screen( WarningScreen, - title="Abandon passphrase?", + title="Discard passphrase?", status_headline=None, - text=f"Exiting will abandon the BIP-39 Passphrase", + text=f"Discard passphrase and go back?", show_back_button=False, button_data=button_data, ) - if button_data[selected_menu_num] == self.CONTINUE: + if button_data[selected_menu_num] == self.EDIT: return Destination(SeedAddPassphraseView) - elif button_data[selected_menu_num] == self.EXIT: + elif button_data[selected_menu_num] == self.DISCARD: self.seed.set_passphrase("") return Destination(SeedFinalizeView) diff --git a/tests/test_flows_seed.py b/tests/test_flows_seed.py index 2191f563..7d56950f 100644 --- a/tests/test_flows_seed.py +++ b/tests/test_flows_seed.py @@ -42,10 +42,10 @@ class TestSeedFlows(FlowTest): FlowStep(scan_views.ScanView, before_run=load_seed_into_decoder), # simulate read SeedQR; ret val is ignored FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase",RET_CODE__BACK_BUTTON)), - FlowStep(seed_views.SeedAddPassphraseExitDialogView, button_data_selection=seed_views.SeedAddPassphraseExitDialogView.EXIT), + FlowStep(seed_views.SeedAddPassphraseExitDialogView, button_data_selection=seed_views.SeedAddPassphraseExitDialogView.DISCARD), FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase",RET_CODE__BACK_BUTTON)), - FlowStep(seed_views.SeedAddPassphraseExitDialogView, button_data_selection=seed_views.SeedAddPassphraseExitDialogView.CONTINUE), + FlowStep(seed_views.SeedAddPassphraseExitDialogView, button_data_selection=seed_views.SeedAddPassphraseExitDialogView.EDIT), FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase",None)), FlowStep(seed_views.SeedReviewPassphraseView, button_data_selection=seed_views.SeedReviewPassphraseView.EDIT), FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase2",None)), From d8b7672cf48b93eb22abdf5796b420a1000e1af4 Mon Sep 17 00:00:00 2001 From: alvroble <50918598+alvroble@users.noreply.github.com> Date: Tue, 9 Jul 2024 17:53:26 +0200 Subject: [PATCH 4/5] Modified return value from tuple to dict * Screen return values were updated to a more structured dict format * Test flows were updated accordingly * TODO comment was added suggesting a future refactor to return a Response object in order to support more complex use cases --- src/seedsigner/gui/screens/seed_screens.py | 5 +++-- src/seedsigner/views/seed_views.py | 9 ++++++--- tests/test_flows_psbt.py | 2 +- tests/test_flows_seed.py | 8 ++++---- tests/test_flows_tools.py | 2 +- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/seedsigner/gui/screens/seed_screens.py b/src/seedsigner/gui/screens/seed_screens.py index b20e8ba1..f657716d 100644 --- a/src/seedsigner/gui/screens/seed_screens.py +++ b/src/seedsigner/gui/screens/seed_screens.py @@ -844,17 +844,18 @@ class SeedAddPassphraseScreen(BaseTopNavScreen): keyboard_swap = False # Check our two possible exit conditions + # TODO: note the unusual return value, consider refactoring to a Response object in the future if input == HardwareButtonsConstants.KEY3: # Save! # First light up key3 self.hw_button3.is_selected = True self.hw_button3.render() self.renderer.show_image() - return self.passphrase, None + return dict(passphrase=self.passphrase) elif input == HardwareButtonsConstants.KEY_PRESS and self.top_nav.is_selected: # Back button clicked - return self.passphrase, self.top_nav.selected_button + return dict(passphrase=self.passphrase, is_back_button=True) # Check for keyboard swaps if input == HardwareButtonsConstants.KEY1: diff --git a/src/seedsigner/views/seed_views.py b/src/seedsigner/views/seed_views.py index 51ff9185..a89a2008 100644 --- a/src/seedsigner/views/seed_views.py +++ b/src/seedsigner/views/seed_views.py @@ -323,17 +323,20 @@ class SeedAddPassphraseView(View): def run(self): - ret_passphrase, ret_btn = self.run_screen(seed_screens.SeedAddPassphraseScreen, passphrase=self.seed.passphrase) + ret_dict = self.run_screen(seed_screens.SeedAddPassphraseScreen, passphrase=self.seed.passphrase) # The new passphrase will be the return value; it might be empty. - self.seed.set_passphrase(ret_passphrase) - if ret_btn == RET_CODE__BACK_BUTTON: + self.seed.set_passphrase(ret_dict["passphrase"]) + + if "is_back_button" in ret_dict: if len(self.seed.passphrase) > 0: return Destination(SeedAddPassphraseExitDialogView) else: return Destination(BackStackView) + elif len(self.seed.passphrase) > 0: return Destination(SeedReviewPassphraseView) + else: return Destination(SeedFinalizeView) diff --git a/tests/test_flows_psbt.py b/tests/test_flows_psbt.py index 384ca760..67c5149e 100644 --- a/tests/test_flows_psbt.py +++ b/tests/test_flows_psbt.py @@ -78,7 +78,7 @@ class TestPSBTFlows(FlowTest): FlowStep(psbt_views.PSBTSelectSeedView, button_data_selection=psbt_views.PSBTSelectSeedView.SCAN_SEED), FlowStep(scan_views.ScanSeedQRView, before_run=load_seed_into_decoder), FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("abc", None)), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=dict(passphrase="abc")), FlowStep(seed_views.SeedReviewPassphraseView, button_data_selection=seed_views.SeedReviewPassphraseView.DONE), FlowStep(seed_views.SeedOptionsView, is_redirect=True), FlowStep(psbt_views.PSBTOverviewView), diff --git a/tests/test_flows_seed.py b/tests/test_flows_seed.py index 7d56950f..4210543d 100644 --- a/tests/test_flows_seed.py +++ b/tests/test_flows_seed.py @@ -41,14 +41,14 @@ class TestSeedFlows(FlowTest): FlowStep(MainMenuView, button_data_selection=MainMenuView.SCAN), FlowStep(scan_views.ScanView, before_run=load_seed_into_decoder), # simulate read SeedQR; ret val is ignored FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase",RET_CODE__BACK_BUTTON)), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=dict(passphrase="muhpassphrase", is_back_button=True)), FlowStep(seed_views.SeedAddPassphraseExitDialogView, button_data_selection=seed_views.SeedAddPassphraseExitDialogView.DISCARD), FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase",RET_CODE__BACK_BUTTON)), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=dict(passphrase="muhpassphrase", is_back_button=True)), FlowStep(seed_views.SeedAddPassphraseExitDialogView, button_data_selection=seed_views.SeedAddPassphraseExitDialogView.EDIT), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase",None)), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=dict(passphrase="muhpassphrase")), FlowStep(seed_views.SeedReviewPassphraseView, button_data_selection=seed_views.SeedReviewPassphraseView.EDIT), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("muhpassphrase2",None)), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=dict(passphrase="muhpassphrase")), FlowStep(seed_views.SeedReviewPassphraseView, button_data_selection=seed_views.SeedReviewPassphraseView.DONE), FlowStep(seed_views.SeedOptionsView), ]) diff --git a/tests/test_flows_tools.py b/tests/test_flows_tools.py index 3563715a..20ed8238 100644 --- a/tests/test_flows_tools.py +++ b/tests/test_flows_tools.py @@ -65,7 +65,7 @@ class TestToolsFlows(FlowTest): self.run_sequence( sequence=[ FlowStep(seed_views.SeedFinalizeView, button_data_selection=seed_views.SeedFinalizeView.PASSPHRASE), - FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=("mypassphrase",None)), + FlowStep(seed_views.SeedAddPassphraseView, screen_return_value=dict(passphrase="mypassphrase")), FlowStep(seed_views.SeedReviewPassphraseView, button_data_selection=seed_views.SeedReviewPassphraseView.DONE), FlowStep(seed_views.SeedOptionsView, is_redirect=True), FlowStep(seed_views.SeedExportXpubScriptTypeView), From d7efbb3b3ed9cae8fbfecb461f3bbf873aea4cc6 Mon Sep 17 00:00:00 2001 From: alvroble <50918598+alvroble@users.noreply.github.com> Date: Tue, 9 Jul 2024 17:59:11 +0200 Subject: [PATCH 5/5] Updated dialog text --- src/seedsigner/views/seed_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/seedsigner/views/seed_views.py b/src/seedsigner/views/seed_views.py index a89a2008..a9bea1a8 100644 --- a/src/seedsigner/views/seed_views.py +++ b/src/seedsigner/views/seed_views.py @@ -358,7 +358,7 @@ class SeedAddPassphraseExitDialogView(View): WarningScreen, title="Discard passphrase?", status_headline=None, - text=f"Discard passphrase and go back?", + text=f"Your current passphrase entry will be erased", show_back_button=False, button_data=button_data, )