From 09a24ee97dfa7fd38706e73e0ff7eaef3457c4e7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 22 Dec 2023 17:31:45 +0100 Subject: [PATCH] Use ErrorToast instead of error panel when invoking a disabled command --- pkg/gui/context/menu_context.go | 3 +- pkg/gui/keybindings.go | 3 +- pkg/integration/components/menu_driver.go | 9 ++++- pkg/integration/tests/branch/delete.go | 11 +++---- .../tests/branch/rebase_to_upstream.go | 10 +++--- .../tests/branch/reset_to_upstream.go | 10 +++--- pkg/integration/tests/file/copy_menu.go | 33 +++++++++---------- .../amend_non_head_commit_during_rebase.go | 5 +-- .../edit_non_todo_commit_during_rebase.go | 5 +-- .../edit_the_confl_commit.go | 5 +-- .../interactive_rebase/fixup_first_commit.go | 5 +-- .../tests/interactive_rebase/quick_start.go | 21 ++++-------- .../squash_down_first_commit.go | 5 +-- 13 files changed, 53 insertions(+), 72 deletions(-) diff --git a/pkg/gui/context/menu_context.go b/pkg/gui/context/menu_context.go index f972f2fbb..1ea0e64c1 100644 --- a/pkg/gui/context/menu_context.go +++ b/pkg/gui/context/menu_context.go @@ -173,7 +173,8 @@ func (self *MenuContext) GetKeybindings(opts types.KeybindingsOpts) []*types.Bin func (self *MenuContext) OnMenuPress(selectedItem *types.MenuItem) error { if selectedItem != nil && selectedItem.DisabledReason != "" { - return self.c.ErrorMsg(selectedItem.DisabledReason) + self.c.ErrorToast(self.c.Tr.DisabledMenuItemPrefix + selectedItem.DisabledReason) + return nil } if err := self.c.PopContext(); err != nil { diff --git a/pkg/gui/keybindings.go b/pkg/gui/keybindings.go index afb12ce85..a2e9fafbd 100644 --- a/pkg/gui/keybindings.go +++ b/pkg/gui/keybindings.go @@ -416,7 +416,8 @@ func (gui *Gui) callKeybindingHandler(binding *types.Binding) error { disabledReason = binding.GetDisabledReason() } if disabledReason != "" { - return gui.c.ErrorMsg(disabledReason) + gui.c.ErrorToast(gui.Tr.DisabledMenuItemPrefix + disabledReason) + return nil } return binding.Handler() } diff --git a/pkg/integration/components/menu_driver.go b/pkg/integration/components/menu_driver.go index 7ab42a3e1..2f93df82d 100644 --- a/pkg/integration/components/menu_driver.go +++ b/pkg/integration/components/menu_driver.go @@ -18,10 +18,12 @@ func (self *MenuDriver) Title(expected *TextMatcher) *MenuDriver { return self } -func (self *MenuDriver) Confirm() { +func (self *MenuDriver) Confirm() *MenuDriver { self.checkNecessaryChecksCompleted() self.getViewDriver().PressEnter() + + return self } func (self *MenuDriver) Cancel() { @@ -72,6 +74,11 @@ func (self *MenuDriver) Tooltip(option *TextMatcher) *MenuDriver { return self } +func (self *MenuDriver) Tap(f func()) *MenuDriver { + self.getViewDriver().Tap(f) + return self +} + func (self *MenuDriver) checkNecessaryChecksCompleted() { if !self.hasCheckedTitle { self.t.Fail("You must check the title of a menu popup by calling Title() before calling Confirm()/Cancel().") diff --git a/pkg/integration/tests/branch/delete.go b/pkg/integration/tests/branch/delete.go index cdda78ec6..0b6adfac4 100644 --- a/pkg/integration/tests/branch/delete.go +++ b/pkg/integration/tests/branch/delete.go @@ -37,12 +37,11 @@ var Delete = NewIntegrationTest(NewIntegrationTestArgs{ Tooltip(Contains("You cannot delete the checked out branch!")). Title(Equals("Delete branch 'branch-three'?")). Select(Contains("Delete local branch")). - Confirm() - t.ExpectPopup(). - Alert(). - Title(Equals("Error")). - Content(Contains("You cannot delete the checked out branch!")). - Confirm() + Confirm(). + Tap(func() { + t.ExpectToast(Contains("You cannot delete the checked out branch!")) + }). + Cancel() }). SelectNextItem(). Press(keys.Universal.Remove). diff --git a/pkg/integration/tests/branch/rebase_to_upstream.go b/pkg/integration/tests/branch/rebase_to_upstream.go index 1fc51350d..5c9e0f388 100644 --- a/pkg/integration/tests/branch/rebase_to_upstream.go +++ b/pkg/integration/tests/branch/rebase_to_upstream.go @@ -48,11 +48,11 @@ var RebaseToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ Title(Equals("Upstream options")). Select(Contains("Rebase checked-out branch onto upstream of selected branch")). Tooltip(Contains("Disabled: The selected branch has no upstream (or the upstream is not stored locally)")). - Confirm() - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Equals("The selected branch has no upstream (or the upstream is not stored locally)")). - Confirm() + Confirm(). + Tap(func() { + t.ExpectToast(Equals("Disabled: The selected branch has no upstream (or the upstream is not stored locally)")) + }). + Cancel() }). SelectNextItem(). Lines( diff --git a/pkg/integration/tests/branch/reset_to_upstream.go b/pkg/integration/tests/branch/reset_to_upstream.go index 1eecd689b..c933787e4 100644 --- a/pkg/integration/tests/branch/reset_to_upstream.go +++ b/pkg/integration/tests/branch/reset_to_upstream.go @@ -42,11 +42,11 @@ var ResetToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ Title(Equals("Upstream options")). Select(Contains("Reset checked-out branch onto upstream of selected branch")). Tooltip(Contains("Disabled: The selected branch has no upstream (or the upstream is not stored locally)")). - Confirm() - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Equals("The selected branch has no upstream (or the upstream is not stored locally)")). - Confirm() + Confirm(). + Tap(func() { + t.ExpectToast(Equals("Disabled: The selected branch has no upstream (or the upstream is not stored locally)")) + }). + Cancel() }). SelectNextItem(). Lines( diff --git a/pkg/integration/tests/file/copy_menu.go b/pkg/integration/tests/file/copy_menu.go index 5f7f00623..a1af13d7b 100644 --- a/pkg/integration/tests/file/copy_menu.go +++ b/pkg/integration/tests/file/copy_menu.go @@ -30,12 +30,11 @@ var CopyMenu = NewIntegrationTest(NewIntegrationTestArgs{ Title(Equals("Copy to clipboard")). Select(Contains("File name")). Tooltip(Equals("Disabled: Nothing to copy")). - Confirm() - - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Equals("Nothing to copy")). - Confirm() + Confirm(). + Tap(func() { + t.ExpectToast(Equals("Disabled: Nothing to copy")) + }). + Cancel() }) t.Shell(). @@ -56,12 +55,11 @@ var CopyMenu = NewIntegrationTest(NewIntegrationTestArgs{ Title(Equals("Copy to clipboard")). Select(Contains("Diff of selected file")). Tooltip(Contains("Disabled: Nothing to copy")). - Confirm() - - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Equals("Nothing to copy")). - Confirm() + Confirm(). + Tap(func() { + t.ExpectToast(Equals("Disabled: Nothing to copy")) + }). + Cancel() }). Press(keys.Files.CopyFileInfoToClipboard). Tap(func() { @@ -69,12 +67,11 @@ var CopyMenu = NewIntegrationTest(NewIntegrationTestArgs{ Title(Equals("Copy to clipboard")). Select(Contains("Diff of all files")). Tooltip(Contains("Disabled: Nothing to copy")). - Confirm() - - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Equals("Nothing to copy")). - Confirm() + Confirm(). + Tap(func() { + t.ExpectToast(Equals("Disabled: Nothing to copy")) + }). + Cancel() }) t.Shell(). diff --git a/pkg/integration/tests/interactive_rebase/amend_non_head_commit_during_rebase.go b/pkg/integration/tests/interactive_rebase/amend_non_head_commit_during_rebase.go index e9c421297..a0d0f066e 100644 --- a/pkg/integration/tests/interactive_rebase/amend_non_head_commit_during_rebase.go +++ b/pkg/integration/tests/interactive_rebase/amend_non_head_commit_during_rebase.go @@ -34,10 +34,7 @@ var AmendNonHeadCommitDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{ NavigateToLine(Contains(commit)). Press(keys.Commits.AmendToCommit) - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("Can't perform this action during a rebase")). - Confirm() + t.ExpectToast(Contains("Can't perform this action during a rebase")) } }, }) diff --git a/pkg/integration/tests/interactive_rebase/edit_non_todo_commit_during_rebase.go b/pkg/integration/tests/interactive_rebase/edit_non_todo_commit_during_rebase.go index 57f219227..78cb875ac 100644 --- a/pkg/integration/tests/interactive_rebase/edit_non_todo_commit_during_rebase.go +++ b/pkg/integration/tests/interactive_rebase/edit_non_todo_commit_during_rebase.go @@ -29,9 +29,6 @@ var EditNonTodoCommitDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{ NavigateToLine(Contains("commit 01")). Press(keys.Universal.Edit) - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("Can't perform this action during a rebase")). - Confirm() + t.ExpectToast(Contains("Can't perform this action during a rebase")) }, }) diff --git a/pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go b/pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go index 96ec81c74..85a3df27c 100644 --- a/pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go +++ b/pkg/integration/tests/interactive_rebase/edit_the_confl_commit.go @@ -39,9 +39,6 @@ var EditTheConflCommit = NewIntegrationTest(NewIntegrationTestArgs{ NavigateToLine(Contains("<-- YOU ARE HERE --- commit three")). Press(keys.Commits.RenameCommit) - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("Changing this kind of rebase todo entry is not allowed")). - Confirm() + t.ExpectToast(Contains("Changing this kind of rebase todo entry is not allowed")) }, }) diff --git a/pkg/integration/tests/interactive_rebase/fixup_first_commit.go b/pkg/integration/tests/interactive_rebase/fixup_first_commit.go index a45c2f050..ff099d760 100644 --- a/pkg/integration/tests/interactive_rebase/fixup_first_commit.go +++ b/pkg/integration/tests/interactive_rebase/fixup_first_commit.go @@ -24,10 +24,7 @@ var FixupFirstCommit = NewIntegrationTest(NewIntegrationTestArgs{ NavigateToLine(Contains("commit 01")). Press(keys.Commits.MarkCommitAsFixup). Tap(func() { - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Equals("There's no commit below to squash into")). - Confirm() + t.ExpectToast(Equals("Disabled: There's no commit below to squash into")) }). Lines( Contains("commit 02"), diff --git a/pkg/integration/tests/interactive_rebase/quick_start.go b/pkg/integration/tests/interactive_rebase/quick_start.go index 713e00cdb..9e95f961e 100644 --- a/pkg/integration/tests/interactive_rebase/quick_start.go +++ b/pkg/integration/tests/interactive_rebase/quick_start.go @@ -50,13 +50,9 @@ var QuickStart = NewIntegrationTest(NewIntegrationTestArgs{ Contains("initial commit"), ). // Verify we can't quick start from main - Press(keys.Commits.StartInteractiveRebase). - Tap(func() { - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("Cannot start interactive rebase: the HEAD commit is a merge commit or is present on the main branch, so there is no appropriate base commit to start the rebase from. You can start an interactive rebase from a specific commit by selecting the commit and pressing `e`.")). - Confirm() - }) + Press(keys.Commits.StartInteractiveRebase) + + t.ExpectToast(Equals("Disabled: Cannot start interactive rebase: the HEAD commit is a merge commit or is present on the main branch, so there is no appropriate base commit to start the rebase from. You can start an interactive rebase from a specific commit by selecting the commit and pressing `e`.")) t.Views().Branches(). Focus(). @@ -80,15 +76,10 @@ var QuickStart = NewIntegrationTest(NewIntegrationTestArgs{ Contains("initial commit"), ). // Try again, verify we fail because we're already rebasing - Press(keys.Commits.StartInteractiveRebase). - Tap(func() { - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("Can't perform this action during a rebase")). - Confirm() + Press(keys.Commits.StartInteractiveRebase) - t.Common().AbortRebase() - }) + t.ExpectToast(Equals("Disabled: Can't perform this action during a rebase")) + t.Common().AbortRebase() // Verify if a merge commit is present on the branch we start from there t.Views().Branches(). diff --git a/pkg/integration/tests/interactive_rebase/squash_down_first_commit.go b/pkg/integration/tests/interactive_rebase/squash_down_first_commit.go index 0f58419f9..65d6bfaa7 100644 --- a/pkg/integration/tests/interactive_rebase/squash_down_first_commit.go +++ b/pkg/integration/tests/interactive_rebase/squash_down_first_commit.go @@ -24,10 +24,7 @@ var SquashDownFirstCommit = NewIntegrationTest(NewIntegrationTestArgs{ NavigateToLine(Contains("commit 01")). Press(keys.Commits.SquashDown). Tap(func() { - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Equals("There's no commit below to squash into")). - Confirm() + t.ExpectToast(Equals("Disabled: There's no commit below to squash into")) }). Lines( Contains("commit 02"),