Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/goto/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ func main() {
utils.LogAndCloseApp(lgr, constant.AppExitCodeError, logMessage)
}

// Handle application shutdown
lgr.Debug("[MAIN] Save application state")
if err = st.Persist(); err != nil {
logMessage := fmt.Sprintf("[MAIN] Can't save application state before closing: %v", err)
Expand Down
80 changes: 53 additions & 27 deletions internal/ui/component/grouplist/grouplist.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func New(_ context.Context, repo storage.HostStorage, appState *state.State, log
delegate.SetSpacing(0)

model := list.New(listItems, delegate, 0, 0)
model.SetFilteringEnabled(false)
model.DisableQuitKeybindings() // We don't want to quit the app from this view.

// Setup model styles.
model.Styles = styles.styleList
Expand All @@ -65,7 +65,6 @@ func New(_ context.Context, repo storage.HostStorage, appState *state.State, log
}

m.Title = "select group"
m.SetShowStatusBar(false)

return &m
}
Expand All @@ -74,6 +73,7 @@ func (m *Model) Init() tea.Cmd { return nil }

func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
var cmd tea.Cmd
var cmds []tea.Cmd

switch msg := msg.(type) {
case tea.WindowSizeMsg:
Expand All @@ -83,49 +83,75 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
return m, nil
case tea.KeyMsg:
cmd = m.handleKeyboardEvent(msg)
return m, cmd
case message.OpenViewSelectGroup:
cmds = append(cmds, cmd)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Keyboard events processed twice in group list

The tea.KeyMsg case doesn't return early after handling keyboard events, causing the message to fall through to m.Model.Update(msg) at line 91. This results in keyboard events being processed twice: once by the custom handleKeyboardEvent and again by the underlying list model. The previous code returned immediately after handling keyboard events. This double-processing can cause unexpected behavior, especially for Enter and Escape keys which trigger view changes.

Fix in Cursor Fix in Web

case message.ViewGroupListOpen:
return m, m.loadItems()
}

m.Model, cmd = m.Model.Update(msg)
return m, cmd
// Only calculate status bar visibility AFTER the model is updated.
m.SetShowStatusBar(m.FilterState() != list.Unfiltered)

return m, tea.Batch(append(cmds, cmd)...)
}

func (m *Model) View() string {
return m.styles.styleComponentMargins.Render(m.Model.View())
}

func (m *Model) handleKeyboardEvent(msg tea.KeyMsg) tea.Cmd {
var cmd tea.Cmd

//exhaustive:ignore
//exhaustive:ignore // Handle only specific keys, other events are handled by the list model.
switch msg.Type {
case tea.KeyEscape:
m.logger.Debug("[UI] Escape key. Deselect group and exit from group list view.")
return tea.Sequence(
// If group view is shown and user presses ESC, we should
// deselect the group view and then show the full host list.
message.TeaCmd(message.GroupSelected{Name: ""}),
message.TeaCmd(message.CloseViewSelectGroup{}),
)
return m.handleEscapeKey()
case tea.KeyEnter:
selected := m.SelectedItem().(ListItemHostGroup).Title() //nolint:errcheck // SelectedItem always returns ListItemHostGroup
selected = strings.TrimSpace(selected)
return m.handleEnterKey()
}

if selected == noGroupSelected {
selected = ""
}
return nil
}

m.logger.Debug("[UI] Enter key. Select group '%s' and exit from group list view.", selected)
return tea.Sequence(
message.TeaCmd(message.GroupSelected{Name: selected}),
message.TeaCmd(message.CloseViewSelectGroup{}),
)
func (m *Model) handleEscapeKey() tea.Cmd {
// If model is in filter mode and press ESC, just disable filtering.
if m.FilterState() == list.Filtering || m.FilterState() == list.FilterApplied {
m.logger.Debug("[UI] Escape key. Deactivate filter in group list view.")
return nil
}

m.Model, cmd = m.Model.Update(msg)
return cmd
// If group view is shown and user presses ESC, we should
// deselect the group view and then show the full host list.
m.logger.Debug("[UI] Escape key. Deselect group and exit from group list view.")
return tea.Sequence(
message.TeaCmd(message.GroupSelect{Name: ""}),
message.TeaCmd(message.ViewGroupListClose{}),
)
}

func (m *Model) handleEnterKey() tea.Cmd {
// If filter is active, by default pressing Enter just selects
// the first item from the list of filtered items. Prevent that.
if m.FilterState() == list.Filtering {
m.logger.Debug("[UI] Enter key. Select item in group list view.")
return nil
}

if m.SelectedItem() == nil {
m.logger.Debug("[UI] Enter key. No group selected, nothing to do.")
return nil
}

selected := m.SelectedItem().(ListItemHostGroup).Title() //nolint:errcheck // SelectedItem always returns ListItemHostGroup
selected = strings.TrimSpace(selected)

if selected == noGroupSelected {
selected = ""
}

m.logger.Debug("[UI] Enter key. Select group '%s' and exit from group list view.", selected)
return tea.Sequence(
message.TeaCmd(message.GroupSelect{Name: selected}),
message.TeaCmd(message.ViewGroupListClose{}),
)
}

func (m *Model) loadItems() tea.Cmd {
Expand Down
147 changes: 112 additions & 35 deletions internal/ui/component/grouplist/grouplist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import (

func TestNew(t *testing.T) {
model := NewMockGroupModel(false)
require.False(t, model.FilteringEnabled())
require.False(t, model.ShowStatusBar())
require.False(t, model.FilteringEnabled())
require.True(t, model.FilteringEnabled())
// Quit app keys is disabled
require.False(t, model.KeyMap.Quit.Enabled())
require.False(t, model.KeyMap.ForceQuit.Enabled())
require.Equal(t, "select group", model.Title)
}

Expand All @@ -40,58 +41,134 @@ func TestUpdate(t *testing.T) {

// Loads hosts when the form is shown
listModel = NewMockGroupModel(false)
listModel.Update(message.OpenViewSelectGroup{})
listModel.Update(message.ViewGroupListOpen{})

// Selected group is "no group"
require.Equal(t, noGroupSelected, listModel.SelectedItem().(ListItemHostGroup).Title())
}

func TestHandleKeyboardEvent_Enter(t *testing.T) {
// Can Select group
func Test_handleKeyboardEvent(t *testing.T) {
listModel := NewMockGroupModel(false)
listModel.loadItems()
require.Equal(t, noGroupSelected, listModel.SelectedItem().(ListItemHostGroup).Title())

// Select Group 1
listModel.Update(tea.KeyMsg{Type: tea.KeyDown})
_, cmd := listModel.Update(tea.KeyMsg{Type: tea.KeyEnter})
tests := []struct {
name string
keyMsg tea.KeyMsg
expectCmd bool
}{
{
name: "Can handle Enter key",
keyMsg: tea.KeyMsg{Type: tea.KeyEnter},
expectCmd: true,
},
{
name: "Can handle Esc key",
keyMsg: tea.KeyMsg{Type: tea.KeyEsc},
expectCmd: true,
},
{
name: "Unhandled key returns nil cmd",
keyMsg: tea.KeyMsg{Type: tea.KeyDown},
expectCmd: false,
},
}

var actualMsgs []tea.Msg
testutils.CmdToMessage(cmd, &actualMsgs)
expectedMsgs := []tea.Msg{
message.GroupSelected{Name: "Group 1"},
message.CloseViewSelectGroup{},
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd := listModel.handleKeyboardEvent(tt.keyMsg)
if tt.expectCmd {
require.NotNil(t, cmd)
} else {
require.Nil(t, cmd)
}
})
}
}

func Test_handleEnterKey(t *testing.T) {
tests := []struct {
name string
group string
itemIndex int
expectedMsgs []tea.Msg
}{
{
name: "Can handle 'no group' selection",
group: noGroupSelected,
itemIndex: 0,
expectedMsgs: []tea.Msg{
message.GroupSelect{Name: ""},
message.ViewGroupListClose{},
},
},
{
name: "Can handle Group 1 selection ",
group: "Group 1",
itemIndex: 1,
expectedMsgs: []tea.Msg{
message.GroupSelect{Name: "Group 1"},
message.ViewGroupListClose{},
},
},
{
name: "Can handle empty group list ",
group: "",
itemIndex: 55, // Out of range index, selected item will be nil
expectedMsgs: nil,
},
}

require.ElementsMatch(t, expectedMsgs, actualMsgs)
require.Equal(t, "Group 1", listModel.SelectedItem().(ListItemHostGroup).Title())
listModel := NewMockGroupModel(false)
listModel.loadItems()

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
listModel.Select(tt.itemIndex)
cmd := listModel.handleEnterKey()

var actualMsgs []tea.Msg
testutils.CmdToMessage(cmd, &actualMsgs)
require.ElementsMatch(t, tt.expectedMsgs, actualMsgs)
if listModel.SelectedItem() != nil {
require.Equal(t, tt.group, listModel.SelectedItem().(ListItemHostGroup).Title())
}
})
}
}

func TestHandleKeyboardEvent_Esc(t *testing.T) {
// Can handle Esc key
func Test_handleEnterKey_WhenFiltering(t *testing.T) {
// When pressing Enter key while filtering, it should return nil cmd, as we do not want to select
// the first item in the list, but instead let user to select an item using Up/Down keys.
listModel := NewMockGroupModel(false)
listModel.loadItems()
require.Equal(t, noGroupSelected, listModel.SelectedItem().(ListItemHostGroup).Title())
// Put the list in filter mode
listModel.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("/")})

// Select Group 1
listModel.Update(tea.KeyMsg{Type: tea.KeyDown})
listModel.Update(tea.KeyMsg{Type: tea.KeyEnter})
require.Equal(t, "Group 1", listModel.SelectedItem().(ListItemHostGroup).Title())
require.True(t, listModel.SettingFilter()) // Activate filter mode
_, cmd := listModel.Update(tea.KeyMsg{Type: tea.KeyEnter})
require.Nil(t, cmd)
}

// Now press Escape key and ensure that Model will send group unselect message and closed the form
func Test_handleEscapeKey(t *testing.T) {
// Test case 1: Press escape in filter mode
// Can handle Esc key
listModel := NewMockGroupModel(false)
listModel.loadItems()
// Put the list in filter mode
listModel.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("/")})
require.True(t, listModel.SettingFilter()) // Verify that filter mode is activate
_, cmd := listModel.Update(tea.KeyMsg{Type: tea.KeyEsc})
require.Nil(t, cmd)

// Test case 2: Press escape when not in filter mode - it must deselect the group and close the form
require.False(t, listModel.SettingFilter()) // Verify that we're not in filter mode after the first test case
_, cmd = listModel.Update(tea.KeyMsg{Type: tea.KeyEsc})
var actualMsgs []tea.Msg
testutils.CmdToMessage(cmd, &actualMsgs)

expectedMsgs := []tea.Msg{
message.GroupSelected{Name: ""},
message.CloseViewSelectGroup{},
}

// Note that though, Escape key was pressed, the group remains selected inside the group list component.
// This is by design - if user opens group list dialog again, previously selected group will be focused.
require.Equal(t, "Group 1", listModel.SelectedItem().(ListItemHostGroup).Title())
require.ElementsMatch(t, expectedMsgs, actualMsgs)
require.ElementsMatch(t, actualMsgs, []tea.Msg{
message.GroupSelect{Name: ""},
message.ViewGroupListClose{},
})
}

func TestLoadItems(t *testing.T) {
Expand Down
12 changes: 6 additions & 6 deletions internal/ui/component/hostedit/hostedit.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func (m *EditModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.viewport.SetContent(m.inputsView())
case debouncedMessage:
cmd = m.handleDebouncedMessage(msg)
case message.HostSSHConfigLoaded:
case message.HostSSHConfigLoadComplete:
m.host.SSHHostConfig = &msg.Config
m.updateInputFields()
m.viewport.SetContent(m.inputsView())
Expand Down Expand Up @@ -262,7 +262,7 @@ func (m *EditModel) handleKeyboardEvent(msg tea.KeyMsg) tea.Cmd {
switch {
case key.Matches(msg, m.keyMap.Discard):
m.logger.Info("[UI] Discard changes for host id: %v", m.host.ID)
return message.TeaCmd(message.CloseViewHostEdit{})
return message.TeaCmd(message.ViewHostEditClose{})
case m.host.IsReadOnly():
m.logger.Debug("[UI] Received a key event. Cannot modify a readonly host.")
return m.displayNotificationMsg("host loaded from SSH config is readonly")
Expand Down Expand Up @@ -340,14 +340,14 @@ func (m *EditModel) save(_ tea.Msg) tea.Cmd {
cmd = message.TeaCmd(message.ErrorOccurred{Err: err})
} else {
cmd = lo.Ternary(m.isNewHost,
message.TeaCmd(message.HostCreated{Host: host}),
message.TeaCmd(message.HostUpdated{Host: host}))
message.TeaCmd(message.HostCreate{Host: host}),
message.TeaCmd(message.HostUpdate{Host: host}))
}

return tea.Sequence(
message.TeaCmd(message.CloseViewHostEdit{}),
message.TeaCmd(message.ViewHostEditClose{}),
// Order matters here! That's why we use tea.Sequence instead of tea.Batch.
message.TeaCmd(message.HostSelected{HostID: host.ID}),
message.TeaCmd(message.HostSelect{HostID: host.ID}),
cmd,
)
}
Expand Down
14 changes: 7 additions & 7 deletions internal/ui/component/hostedit/hostedit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ func TestSave(t *testing.T) {

var dst []tea.Msg
testutils.CmdToMessage(messageSequence, &dst)
require.Contains(t, dst, message.CloseViewHostEdit{})
require.Contains(t, dst, message.HostSelected{HostID: 0})
require.Contains(t, dst, message.ViewHostEditClose{})
require.Contains(t, dst, message.HostSelect{HostID: 0})
}

func TestCopyInputValueFromTo(t *testing.T) {
Expand Down Expand Up @@ -230,7 +230,7 @@ func TestUpdate_HostSSHConfigLoaded(t *testing.T) {
require.NotEqual(t, "default: Mock User", model.inputs[inputLogin].Placeholder)
require.NotEqual(t, "default: Mock Port", model.inputs[inputNetworkPort].Placeholder)

model.Update(message.HostSSHConfigLoaded{
model.Update(message.HostSSHConfigLoadComplete{
HostID: 0,
Config: sshconfig.Config{
IdentityFile: "Mock Identity File",
Expand Down Expand Up @@ -380,7 +380,7 @@ func TestUpdate_KeyDiscard(t *testing.T) {
Type: tea.KeyEscape,
})

require.Equal(t, message.CloseViewHostEdit{}, cmd())
require.Equal(t, message.ViewHostEditClose{}, cmd())
}

func TestUpdate_KeySave(t *testing.T) {
Expand All @@ -394,13 +394,13 @@ func TestUpdate_KeySave(t *testing.T) {
testutils.CmdToMessage(cmd, &msgs)

for _, msg := range msgs {
if _, ok := msg.(message.HostSelected); ok {
if _, ok := msg.(message.HostSelect); ok {
continue
}
if _, ok := msg.(message.CloseViewHostEdit); ok {
if _, ok := msg.(message.ViewHostEditClose); ok {
continue
}
if _, ok := msg.(message.HostUpdated); ok {
if _, ok := msg.(message.HostUpdate); ok {
continue
}

Expand Down
Loading
Loading