diff --git a/CHANGELOG.md b/CHANGELOG.md index 52ec758..2f45469 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## Unreleased + +- **Help over the Config view:** after moving through the fields with the arrow keys, F9 left the + field in use focused behind the help: its cursor showed through the window, the help did not + scroll (the form took the keys), and closing it returned to the wrong field. Arrow-key moves in + forms now keep the application's focus in step. + ## v0.2.0 New: diff --git a/internal/ui/app.go b/internal/ui/app.go index a0de7da..dd9d9a4 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -200,6 +200,7 @@ func New(cfg *config.Config) *App { return false }) a.app.SetRoot(&toastRoot{Flex: layout, app: a}, true).EnableMouse(false) + syncFocus = func(p tview.Primitive) { a.app.SetFocus(p) } a.app.SetInputCapture(a.handleKey) a.switchTo(0) a.renderServices() diff --git a/internal/ui/form.go b/internal/ui/form.go index 3f5c398..2822ccd 100644 --- a/internal/ui/form.go +++ b/internal/ui/form.go @@ -18,6 +18,18 @@ import ( // cancel is called on Esc, except when Esc only closes an open dropdown list // (tview would otherwise treat that as cancelling the whole form). func arrowNavigation(f *tview.Form, cancel func(), hooks *browseHooks) { + // Form.SetFocus moves the focus between its items without telling the + // application, which then still holds the item that was left. Opening a + // dialog would only blur that one: the item actually in use kept its + // cursor on screen and, because the form still had the focus, took the + // dialog's keys, and closing the dialog returned to the wrong item. So the + // application is told after every move. + move := func(index int) { + f.SetFocus(index) + if syncFocus != nil { + syncFocus(f) // the form hands the focus on to the item at index + } + } var b browser var closingList bool f.SetCancelFunc(func() { @@ -65,7 +77,7 @@ func arrowNavigation(f *tview.Form, cancel func(), hooks *browseHooks) { } return nil } - f.SetFocus(next) + move(next) case tcell.KeyTab, tcell.KeyBacktab: dir := 1 if ev.Key() == tcell.KeyBacktab { @@ -86,7 +98,7 @@ func arrowNavigation(f *tview.Form, cancel func(), hooks *browseHooks) { cur, _ := dd.GetCurrentOption() dd.SetCurrentOption((cur + step + n) % n) case button >= 0: - f.SetFocus(min(max(pos+step, f.GetFormItemCount()), total-1)) + move(min(max(pos+step, f.GetFormItemCount()), total-1)) default: return ev } @@ -97,6 +109,9 @@ func arrowNavigation(f *tview.Form, cancel func(), hooks *browseHooks) { }) } +// syncFocus gives a primitive the application's focus; New sets it. +var syncFocus func(p tview.Primitive) + func focusedDropDown(f *tview.Form, item int) (*tview.DropDown, bool) { if item < 0 { return nil, false diff --git a/internal/ui/settings_test.go b/internal/ui/settings_test.go index 923ff73..d4b1738 100644 --- a/internal/ui/settings_test.go +++ b/internal/ui/settings_test.go @@ -313,3 +313,30 @@ func TestTestConnectionsButtonSitsBelowTheServices(t *testing.T) { } } } + +func TestHelpOverConfigTakesTheFocus(t *testing.T) { + a := newTestApp(t) + a.handleKey(tcell.NewEventKey(tcell.KeyF6, 0, 0)) + moveDown := a.settings.form.GetInputCapture() + moveDown(tcell.NewEventKey(tcell.KeyDown, 0, 0)) + moveDown(tcell.NewEventKey(tcell.KeyDown, 0, 0)) + field := a.settings.inputs["radarr.profile"] + if a.app.GetFocus() != field { + t.Fatalf("after ↓↓ the application's focus is %T, not the third field", a.app.GetFocus()) + } + + a.handleKey(tcell.NewEventKey(tcell.KeyF9, 0, 0)) + // The field must not keep the focus behind the help: it would draw its + // cursor through the window, and the form would take the help's keys. + if field.HasFocus() || a.settings.form.HasFocus() { + t.Errorf("behind the help: field focused %v, form focused %v", field.HasFocus(), a.settings.form.HasFocus()) + } + if _, ok := a.app.GetFocus().(*tview.TextView); !ok { + t.Errorf("help has no focus: %T", a.app.GetFocus()) + } + + a.closeModal() + if a.app.GetFocus() != field || !field.HasFocus() { + t.Errorf("closing the help returns to %T, not the field that was in use", a.app.GetFocus()) + } +}