diff --git a/docs/tui.md b/docs/tui.md index 711d57ec..4cf87355 100644 --- a/docs/tui.md +++ b/docs/tui.md @@ -173,11 +173,11 @@ the iOS and Android apps each keep their own local choice too, and this is the s ## Attachments -Thread attachments always appear with their filename, media type, and size. Use `[` and `]` to select an attachment, `s` to save it without replacing an existing file, and `o` to download and open it in an external application. Attachments never open automatically. Kitty and Ghostty can show inline images. In the message itself, a file is marked by its kind: 📷 an image, 🎬 a video, 🎵 audio, 📄 a PDF or document, 📎 anything else. An image shows by name rather than by its URL, unless its name looks like a URL, when the real destination is shown beside it; an image on the web is a link you can select with Tab, and the row above the shortcut bar shows its whole destination. +Thread attachments always appear with their filename, media type, and size. Use `[` and `]` to select an attachment, `s` to save it without replacing an existing file, and Enter to download and open it in an external application — once Tab has selected a link, Enter opens the link instead, and Escape clears it. Attachments never open automatically. Kitty and Ghostty can show inline images. In the message itself, a file is marked by its kind: 📷 an image, 🎬 a video, 🎵 audio, 📄 a PDF or document, 📎 anything else. An image shows by name rather than by its URL, unless its name looks like a URL, when the real destination is shown beside it; an image on the web is a link you can select with Tab, and the row above the shortcut bar shows its whole destination. ## Contacts -Press Shift+O to open Contacts. Use Enter to view a contact, `a` to add, `e` to edit, `n` to edit the private note, `x` twice to delete a note, `h` to hide, and `u` to show the most recently hidden contact again. Escape or `q` goes back. +Press `o` (or Shift+O) to open Contacts — the letter underlined in its tab. Use Enter to view a contact, `a` to add, `e` to edit, `n` to edit the private note, `x` twice to delete a note, `h` to hide, and `u` to show the most recently hidden contact again. Escape or `q` goes back. The private note is shown formatted and edited as Markdown, the way `hey contact note set` takes it, so a note written in HEY keeps its bold, italics, lists, links and headings when you edit it here. Press ctrl+s to save it. A note holding an attachment or other formatting Markdown cannot preserve is shown but refused by the editor rather than losing that content; edit it in HEY or use `hey contact note set --note-html`. diff --git a/internal/tui/mail.go b/internal/tui/mail.go index 4609c5b7..8c8263b4 100644 --- a/internal/tui/mail.go +++ b/internal/tui/mail.go @@ -1058,8 +1058,11 @@ func (v *mailView) HelpBindings() []helpBinding { helpBinding{"[", "previous attachment"}, helpBinding{"]", "next attachment"}, helpBinding{"s", "save attachment"}, - helpBinding{"o", "open attachment"}, ) + // A selected link has Enter, and the footer above says so. + if !v.LinkSelectionActive() { + bindings = append(bindings, helpBinding{"enter", "open attachment"}) + } } return bindings } @@ -1356,6 +1359,11 @@ func (v *mailView) HandleContentKey(msg tea.KeyPressMsg) tea.Cmd { if cmd, handled := v.handleLinkKey(msg); handled { return cmd } + // Enter opens what is selected: a link once Tab has picked one, which + // handleLinkKey has already answered, and otherwise the attachment. + if msg.Key().Code == tea.KeyEnter && len(v.attachments) > 0 { + return v.openSelectedAttachment() + } switch msg.String() { case "r", "R": @@ -1378,8 +1386,6 @@ func (v *mailView) HandleContentKey(msg tea.KeyPressMsg) tea.Cmd { return nil case "s": return v.saveSelectedAttachment() - case "o": - return v.openSelectedAttachment() case "j": if len(v.entryOffsets) > 1 { v.jumpEntry(1) diff --git a/internal/tui/mail_test.go b/internal/tui/mail_test.go index 7fc79b25..8f90f3f7 100644 --- a/internal/tui/mail_test.go +++ b/internal/tui/mail_test.go @@ -3030,9 +3030,9 @@ func TestMailViewDownloadsBeforeExplicitExternalOpen(t *testing.T) { t.Fatalf("loading a thread opened an attachment: %v", events) } - open := v.HandleContentKey(keyPress("o")) + open := v.HandleContentKey(keyPress("enter")) if open == nil { - t.Fatal("o should open the selected attachment") + t.Fatal("Enter should open the selected attachment") } if len(events) != 0 { t.Fatalf("handling the key opened an attachment before the command ran: %v", events) @@ -3071,7 +3071,7 @@ func TestMailViewDoesNotOpenAttachmentWhenDownloadFails(t *testing.T) { attachments: []messageAttachment{{ID: "501:1", MessageID: 501, Filename: "chart.png", URL: "/rails/blobs/chart.png"}}, }) - open := v.HandleContentKey(keyPress("o")) + open := v.HandleContentKey(keyPress("enter")) v.Update(open()) if openCalls != 0 { t.Errorf("opener was called %d times after download failure", openCalls) diff --git a/internal/tui/nav.go b/internal/tui/nav.go index ccd6fea6..4ba8c48f 100644 --- a/internal/tui/nav.go +++ b/internal/tui/nav.go @@ -30,7 +30,7 @@ const ( // navItem is a single item in a navigation row. type navItem struct { - shortcut string // Shift+letter shortcut, underlined inside the label + shortcut string // the key that jumps here, underlined inside the label label string } @@ -38,17 +38,20 @@ type navItem struct { var sectionItems = []navItem{ {"M", "Mail"}, - {"O", "Contacts"}, + {"o", "Contacts"}, {"C", "Calendar"}, {"J", "Journal"}, } -// sectionForShortcut returns the section for a Shift+letter shortcut, or -1. +// sectionForShortcut returns the section for a shortcut key, or -1. Contacts is the +// one on a lowercase letter: the C its label starts with is Calendar's, so its letter +// is the o underlined inside it, which is what a reader presses. It answers O too, +// the way HEY's letter shortcuts answer either case. func sectionForShortcut(key string) section { switch key { case "M": return sectionMail - case "O": + case "o", "O": return sectionContacts case "C": return sectionCalendar diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 24edd16a..11486f75 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -754,7 +754,8 @@ func (m *model) updateHelpBindings() { bindings = []helpBinding{ {"←→", "section"}, {"tab", "next row"}, - {"shift+M/O/C/J", "jump"}, + {"shift+M/C/J", "jump"}, + {"o", "contacts"}, quitHint, } case rowSubnav: diff --git a/internal/tui/tui_test.go b/internal/tui/tui_test.go index 1e7a56ac..b4a44dde 100644 --- a/internal/tui/tui_test.go +++ b/internal/tui/tui_test.go @@ -1,6 +1,7 @@ package tui import ( + "context" "errors" "fmt" "net/http" @@ -649,14 +650,29 @@ func TestLoadingViewKeepsItsSectionUntilResponse(t *testing.T) { } func TestContactsSectionShortcut(t *testing.T) { - m := modelWithBoxes() - updated, cmd := m.Update(keyPress("O")) - result := updated.(model) - if result.section != sectionContacts || result.activeView != result.contactsView { - t.Errorf("O shortcut selected section %d and view %T", result.section, result.activeView) + for _, key := range []string{"o", "O"} { + m := modelWithBoxes() + updated, cmd := m.Update(keyPress(key)) + result := updated.(model) + if result.section != sectionContacts || result.activeView != result.contactsView { + t.Errorf("%s shortcut selected section %d and view %T", key, result.section, result.activeView) + } + if cmd == nil || !result.loading { + t.Errorf("opening Contacts with %s should start its initial list request", key) + } } - if cmd == nil || !result.loading { - t.Error("opening Contacts should start its initial list request") +} + +func TestContactsTabUnderlinesTheKeyThatOpensIt(t *testing.T) { + for _, item := range sectionItems { + index := strings.Index(item.label, item.shortcut) + if index < 0 { + t.Errorf("%s tab underlines %q, which is not a letter of its label as written", item.label, item.shortcut) + continue + } + if got := sectionForShortcut(item.shortcut); got < 0 || sectionItems[got].label != item.label { + t.Errorf("pressing the %q underlined in %s does not open it", item.shortcut, item.label) + } } } @@ -1698,6 +1714,87 @@ func TestEnterWithoutASelectedLinkDoesNotOpen(t *testing.T) { } } +func openAttachmentLinkThreadThroughModel(t *testing.T) (model, *[]string) { + t.Helper() + m := openLinkThreadThroughModel(t) + var events []string + m.mailView.attachments = []messageAttachment{{ID: "501:1", MessageID: 501, Filename: "quarterly-report.pdf", URL: "/rails/blobs/quarterly-report.pdf"}} + m.mailView.attachmentCursor = 0 + m.mailView.vc.newAttachmentTempDir = func() (string, error) { return t.TempDir(), nil } + m.mailView.vc.saveAttachment = func(context.Context, string, string, bool) (int64, error) { + events = append(events, "save") + return 2048, nil + } + m.mailView.vc.openAttachment = func(string) error { + events = append(events, "open attachment") + return nil + } + m.mailView.vc.openURL = func(destination string) error { + events = append(events, "visit "+destination) + return nil + } + m.updateHelpBindings() + return m, &events +} + +func TestEnterOpensTheAttachmentUntilALinkIsSelected(t *testing.T) { + m, events := openAttachmentLinkThreadThroughModel(t) + if !hasHelpBinding(m.mailView.HelpBindings(), "enter") { + t.Errorf("a thread with an attachment should offer Enter to open it: %v", m.mailView.HelpBindings()) + } + updated, cmd := m.Update(keyPress("enter")) + m = updated.(model) + if cmd == nil { + t.Fatal("Enter without a selected link should open the selected attachment") + } + runCmd(cmd) + if fmt.Sprint(*events) != "[save open attachment]" { + t.Errorf("Enter without a selected link = %v, want the attachment downloaded and opened", *events) + } + + *events = nil + updated, _ = m.Update(keyPress("tab")) + m = updated.(model) + if hasHelpBinding(m.mailView.HelpBindings(), "enter") { + t.Errorf("with a link selected Enter belongs to the link, whose footer says so: %v", m.mailView.HelpBindings()) + } + updated, cmd = m.Update(keyPress("enter")) + m = updated.(model) + if cmd == nil { + t.Fatal("Enter on a selected link returned no command") + } + runCmd(cmd) + if len(*events) != 1 || !strings.HasPrefix((*events)[0], "visit ") { + t.Errorf("Enter on a selected link = %v, want only the link visited", *events) + } + + *events = nil + updated, _ = m.Update(keyPress("esc")) + m = updated.(model) + if m.mailView.selectedLink != -1 || !m.mailView.inThread { + t.Fatalf("Escape should clear the link and stay in the thread: selected=%d inThread=%v", m.mailView.selectedLink, m.mailView.inThread) + } + _, cmd = m.Update(keyPress("enter")) + runCmd(cmd) + if fmt.Sprint(*events) != "[save open attachment]" { + t.Errorf("Enter after clearing the link = %v, want the attachment again", *events) + } +} + +func TestOOpensContactsFromAThreadWithAttachments(t *testing.T) { + m, events := openAttachmentLinkThreadThroughModel(t) + // Opening the thread marked it seen; a section waits for that write, so let it land. + m.mailView.pendingMutations = 0 + updated, _ := m.Update(keyPress("o")) + m = updated.(model) + if m.section != sectionContacts || m.activeView != m.contactsView { + t.Errorf("o in a thread selected section %d and view %T, want Contacts", m.section, m.activeView) + } + if len(*events) != 0 { + t.Errorf("o should no longer open an attachment: %v", *events) + } +} + func TestThreadLinkNavigationYieldsToAModal(t *testing.T) { m := openLinkThreadThroughModel(t) updated, _ := m.Update(keyPress("tab"))