From 6a5991fdc601b55d636ab79169040d5d3cb17568 Mon Sep 17 00:00:00 2001 From: jalil Date: Fri, 28 Aug 2026 13:44:50 +0200 Subject: [PATCH 1/2] Give the thread view the HEY apps' More menu m over an open thread swaps the help bar for the More menu the HEY apps have: Forward, Label, Move and Trash, each behind the key the same action answers to on the list, q or esc to put the bar back. The thread stays on screen and keeps scrolling underneath. The actions aim at the posting the thread was opened from, so they work wherever the thread came from - a box, search results, a bundle or Previously Seen. Trashing (or moving) the open thread closes the thread view once the server has confirmed it, landing the reader back on the list they came from with the row already gone. --- internal/tui/mail.go | 266 ++++++++++++++++++++------- internal/tui/mail_test.go | 378 ++++++++++++++++++++++++++++++++++++++ internal/tui/more_menu.go | 69 +++++++ 3 files changed, 648 insertions(+), 65 deletions(-) create mode 100644 internal/tui/more_menu.go diff --git a/internal/tui/mail.go b/internal/tui/mail.go index ac9482a8..0cac10dc 100644 --- a/internal/tui/mail.go +++ b/internal/tui/mail.go @@ -249,20 +249,27 @@ type mailView struct { boxes []mail.Source boxIndex int - postingPaging listPaging - postingList contentList - topicViewport viewport.Model - topicContent string - topicID int64 - topicName string - entries []mail.Entry - attachments []messageAttachment - attachmentCursor int - imageContent string - entryOffsets []int // line where each message starts in the thread content - inThread bool - threadNotice string // what the open thread's read did not get; stays until the thread is left - contentHeight int // the rows the section has, which the thread's notices and viewport share + postingPaging listPaging + postingList contentList + topicViewport viewport.Model + topicContent string + topicID int64 + // topicPosting is a copy of the posting the open thread was opened from, nil for a + // topic opened directly. A copy, because the row itself can leave the list while + // the thread is open: opening marked it seen, and a live re-read's head page may + // no longer reach it. pendingTopicPosting is the same copy taken when the thread + // was asked for, so a re-read landing during the read itself cannot lose it. + topicPosting *mail.Posting + pendingTopicPosting *mail.Posting + topicName string + entries []mail.Entry + attachments []messageAttachment + attachmentCursor int + imageContent string + entryOffsets []int // line where each message starts in the thread content + inThread bool + threadNotice string // what the open thread's read did not get; stays until the thread is left + contentHeight int // the rows the section has, which the thread's notices and viewport share modal modal // the form or picker over the list, and the only one there can be cover coverPreset // the session's cover; HEY does not serve one to read @@ -515,6 +522,18 @@ func (v *mailView) Update(msg tea.Msg) (tea.Cmd, bool) { } v.inThread = true v.topicID = msg.topicID + // The list's row is the freshest picture of the posting when it is still + // there; the copy taken at request time covers a row a re-read carried away + // while the topic was loading. + v.topicPosting = nil + if pending := v.pendingTopicPosting; pending != nil && pending.ID == msg.postingID { + v.topicPosting = pending + } + if opened := v.openedPosting(msg.postingID); opened != nil { + posting := *opened + v.topicPosting = &posting + } + v.pendingTopicPosting = nil v.topicName = msg.title v.entries = msg.entries v.attachments = msg.attachments @@ -687,6 +706,31 @@ func (v *mailView) Update(msg tea.Msg) (tea.Cmd, bool) { case postingActionDoneMsg: v.finishMutation() + if msg.err == nil && msg.effect == postingActionRemove { + // A thread that left its box leaves the search and bundle lists with it, + // whether or not the reader is still inside it when the action lands — + // those lists never re-read themselves, and a row for a trashed thread + // would otherwise stay there to be opened. + if idx := postingIndexIn(v.searchList.postings, msg.postingID); idx >= 0 { + v.searchList.removeAt(idx) + } + if idx := postingIndexIn(v.bundleList.postings, msg.postingID); idx >= 0 { + v.bundleList.removeAt(idx) + } + // The open thread closes under an idle reader: what was on screen is in + // the Trash or another box now, and the list it came from is where they + // land. A reader who has moved on — a form or a picker up, typed text at + // stake — keeps the thread and leaves it themselves, as they always + // could. A reply or forward still loading for the closed thread is + // cancelled with it, so it cannot settle into a compose form over the + // list once the thread is gone. + if v.inThread && v.topicPosting != nil && msg.postingID == v.topicPosting.ID && v.modal == nil { + v.closeThread() + if v.requests.kind == mailRequestTopic || v.requests.kind == mailRequestReply || v.requests.kind == mailRequestForward { + v.requests.cancel() + } + } + } if msg.seen { return v.applySeenPostingAction(msg), true } @@ -816,12 +860,7 @@ func (v *mailView) View() string { return v.modal.draw(v) } if v.inThread { - v.fitThreadViewport() - var lines []string - for _, notice := range v.threadNotices() { - lines = append(lines, v.vc.styles.title.Render(notice)) - } - return strings.Join(append(lines, v.topicViewport.View()), "\n") + return v.threadView() } if v.searchActive { if v.notice != "" { @@ -864,6 +903,17 @@ func (v *mailView) listView() string { return v.listHeader() + v.postingList.view() } +// threadView is the open thread and its notices, which is what the More menu draws +// itself over. +func (v *mailView) threadView() string { + v.fitThreadViewport() + var lines []string + for _, notice := range v.threadNotices() { + lines = append(lines, v.vc.styles.title.Render(notice)) + } + return strings.Join(append(lines, v.topicViewport.View()), "\n") +} + // openModal puts a form or a picker over the list, sized to the screen it opens on. func (v *mailView) openModal(open modal) { v.modal = open @@ -929,6 +979,9 @@ func (v *mailView) HelpBindings() []helpBinding { } if v.inThread { bindings := []helpBinding{{"r", "reply"}, {"f", "forward"}} + if v.topicPosting != nil { + bindings = append(bindings, helpBinding{"m", "more"}) + } if len(v.entries) > 1 { bindings = append(bindings, helpBinding{"j/k", "next/previous message"}) } @@ -1221,39 +1274,7 @@ func (v *mailView) HandleContentKey(msg tea.KeyPressMsg) tea.Cmd { } if v.inThread { - switch msg.String() { - case "r", "R": - if v.topicID != 0 { - return v.loadReplyContext(v.topicID, v.topicName) - } - case "f", "F": - if v.topicID != 0 { - return v.loadForwardContext(v.topicID, v.topicName) - } - case "[": - v.moveAttachmentCursor(-1) - return nil - case "]": - v.moveAttachmentCursor(1) - return nil - case "s": - return v.saveSelectedAttachment() - case "o": - return v.openSelectedAttachment() - case "j": - if len(v.entryOffsets) > 1 { - v.jumpEntry(1) - return nil - } - case "k": - if len(v.entryOffsets) > 1 { - v.jumpEntry(-1) - return nil - } - } - var cmd tea.Cmd - v.topicViewport, cmd = v.topicViewport.Update(msg) - return cmd + return v.handleThreadKey(msg) } if v.searchActive { @@ -1384,6 +1405,50 @@ func (v *mailView) HandleContentKey(msg tea.KeyPressMsg) tea.Cmd { return nil } +// handleThreadKey routes one key over the open thread: the thread's own actions +// first, the viewport's scrolling for everything else. The More menu sends the keys +// it does not take through here too, so having the menu up never changes what a +// thread key means. Only lowercase m opens the menu — uppercase M is the Mail +// section's global shortcut and never reaches the thread. +func (v *mailView) handleThreadKey(msg tea.KeyPressMsg) tea.Cmd { + switch msg.String() { + case "r", "R": + if v.topicID != 0 { + return v.loadReplyContext(v.topicID, v.topicName) + } + case "f", "F": + if v.topicID != 0 { + return v.loadForwardContext(v.topicID, v.topicName) + } + case "m": + v.openMoreMenu() + return nil + case "[": + v.moveAttachmentCursor(-1) + return nil + case "]": + v.moveAttachmentCursor(1) + return nil + case "s": + return v.saveSelectedAttachment() + case "o": + return v.openSelectedAttachment() + case "j": + if len(v.entryOffsets) > 1 { + v.jumpEntry(1) + return nil + } + case "k": + if len(v.entryOffsets) > 1 { + v.jumpEntry(-1) + return nil + } + } + var cmd tea.Cmd + v.topicViewport, cmd = v.topicViewport.Update(msg) + return cmd +} + func (v *mailView) InThread() bool { return v.inThread || v.searchActive || v.bundleActive || v.seenActive } @@ -1405,9 +1470,7 @@ func (v *mailView) ExitThread() { return } if v.inThread { - v.inThread = false - v.threadNotice = "" - v.modal = nil + v.closeThread() v.requests.cancel() return } @@ -1425,6 +1488,17 @@ func (v *mailView) ExitThread() { v.requests.cancel() } +// closeThread leaves the thread view for the list it was opened from, taking whatever +// modal stood over the thread with it. Cancelling anything in flight stays with the +// caller: leaving by key abandons the pending read, leaving because an action removed +// the thread has nothing to abandon. +func (v *mailView) closeThread() { + v.inThread = false + v.threadNotice = "" + v.topicPosting = nil + v.modal = nil +} + func (v *mailView) clearSearch() { v.searchActive = false v.searchQuery = "" @@ -1875,6 +1949,13 @@ func (v *mailView) loadMoreSeenPostings() tea.Cmd { } func (v *mailView) requestTopic(boxID, topicID, postingID int64, title string) tea.Cmd { + // The posting is copied now, while the row is certainly still on screen: a live + // re-read can carry it out of the list before the topic arrives. + v.pendingTopicPosting = nil + if opened := v.openedPosting(postingID); opened != nil { + posting := *opened + v.pendingTopicPosting = &posting + } requestID, ctx := v.requests.begin(v.vc.ctx, mailRequestTopic) return v.fetchTopic(ctx, requestID, boxID, topicID, postingID, title) } @@ -2075,17 +2156,25 @@ func (v *mailView) openedPosting(postingID int64) *mail.Posting { // --- Posting actions --- func (v *mailView) startMove() { - selected := v.actionList().selectedPosting() + if selected := v.actionList().selectedPosting(); selected != nil { + v.openMovePicker(*selected) + } +} + +// openMovePicker puts the move picker up over posting and reports whether it opened: +// with nowhere to move to there is no picker, only the notice saying so. +func (v *mailView) openMovePicker(posting mail.Posting) bool { currentSource := v.actionSource() - if selected == nil || currentSource == nil { - return + if currentSource == nil { + return false } - picker := newMovePicker(*selected, v.boxes, *currentSource) + picker := newMovePicker(posting, v.boxes, *currentSource) if len(picker.destinations) == 0 { v.notice = "No other boxes available" - return + return false } v.openModal(picker) + return true } // startCoverPicker opens the cover picker. Only the Imbox can be covered, which @@ -2117,14 +2206,24 @@ func (v *mailView) startFolderPicker() tea.Cmd { v.notice = "Retrying labels…" return v.requestSources() } - selected := v.actionList().selectedPosting() - if selected == nil { - return nil + if selected := v.actionList().selectedPosting(); selected != nil { + cmd, _ := v.openFolderPicker(*selected) + return cmd } - v.openModal(newFolderPicker(*selected, v.boxes)) return nil } +// openFolderPicker puts the label picker up over posting, or retries the label +// discovery that failed, and reports whether a picker is now open. +func (v *mailView) openFolderPicker(posting mail.Posting) (tea.Cmd, bool) { + if v.folderDiscoveryErr != "" { + v.notice = "Retrying labels…" + return v.requestSources(), false + } + v.openModal(newFolderPicker(posting, v.boxes)) + return nil, true +} + func (v *mailView) filePosting(postingID, folderID int64, folderName string) tea.Cmd { return v.doFolderAction("Label "+terminal.SanitizeLine(folderName)+" added", false, func() error { return v.vc.sdk.Postings().File(v.vc.ctx, folderID, postingID) @@ -2267,6 +2366,43 @@ func (v *mailView) imboxSource() *mail.Source { return nil } +// openMoreMenu swaps the help bar for the thread's More menu — forward, label, move, +// trash, as in the HEY apps. The menu acts on the posting the thread was opened from; +// a topic opened directly has none, so there is nothing to offer. +func (v *mailView) openMoreMenu() { + if v.topicPosting == nil { + return + } + // The list's row is fresher than the snapshot for as long as it is still there: + // a label added since the thread was opened lives on the row, and a picker built + // off the snapshot would offer to add it again. + if live := v.openedPosting(v.topicPosting.ID); live != nil { + posting := *live + v.topicPosting = &posting + } + v.openModal(newMoreMenu(*v.topicPosting, v.menuOrganizes())) +} + +// menuOrganizes reports whether the More menu can offer label and move. A search +// match and a contact's threads say neither which box a thread lives in nor which +// labels it carries, so a picker built over them offers wrong choices — there the +// menu keeps to forward and trash, which need only the posting itself. +func (v *mailView) menuOrganizes() bool { + if v.searchActive { + return false + } + return !v.bundleActive || v.bundleContactID == 0 +} + +// trashOpenThread moves the thread on screen to the Trash. Closing the thread view is +// left to the action's done message, so the reader only lands back on the list once +// the thread has really gone. +func (v *mailView) trashOpenThread(postingID int64) tea.Cmd { + return v.doPostingAction("Thread moved to Trash", postingActionRemove, v.currentBoxID(), postingID, func() error { + return v.vc.sdk.Postings().MoveToTrash(v.vc.ctx, postingID) + }) +} + func (v *mailView) handlePostingAction(key string) tea.Cmd { selected := v.actionList().selectedPosting() if selected == nil { diff --git a/internal/tui/mail_test.go b/internal/tui/mail_test.go index 8d1ea5c4..b38a9ddd 100644 --- a/internal/tui/mail_test.go +++ b/internal/tui/mail_test.go @@ -519,6 +519,384 @@ func TestMailViewReportsFailureToMarkOpenedThreadSeen(t *testing.T) { } } +// --- Thread More menu --- + +func openTestThread(t *testing.T, v *mailView) { + t.Helper() + loaded, ok := runCmd(v.HandleContentKey(keyPress("enter"))).(topicLoadedMsg) + if !ok { + t.Fatal("opening the thread did not return topicLoadedMsg") + } + cmd, _ := v.Update(loaded) + if seen, ok := runCmd(cmd).(postingSeenMsg); ok { + v.Update(seen) + } + if !v.inThread || v.topicPosting == nil || v.topicPosting.ID != 100 { + t.Fatalf("thread state = open:%v posting:%v, want posting 100 open", v.inThread, v.topicPosting) + } +} + +// The thread view carries the HEY apps' More menu: m opens it over the thread, and t +// inside it trashes the thread — the server told first, the view closed and the row +// gone from the list only once the trash has landed. +func TestThreadMoreMenuTrashesTheOpenThread(t *testing.T) { + v, recorded := mailWithTestServer(t, http.StatusNoContent) + openTestThread(t, v) + + v.HandleContentKey(keyPress("m")) + if _, ok := v.modal.(*moreMenu); !ok { + t.Fatalf("m opened %T, want the More menu", v.modal) + } + cmd := v.HandleContentKey(keyPress("t")) + if v.modal != nil { + t.Error("committing an action should close the menu") + } + if !v.inThread { + t.Error("the thread should stay open until the trash lands") + } + done, ok := runCmd(cmd).(postingActionDoneMsg) + if !ok || done.err != nil { + t.Fatalf("trash returned %#v", done) + } + if recorded.method != http.MethodPost || recorded.path != "/postings/trash.json" { + t.Errorf("request = %s %s, want POST /postings/trash.json", recorded.method, recorded.path) + } + if len(recorded.body.PostingIDs) != 1 || recorded.body.PostingIDs[0] != 100 { + t.Errorf("posting_ids = %v, want [100]", recorded.body.PostingIDs) + } + + answer, _ := v.Update(done) + toast := deliverToView(v, answer) + if v.inThread { + t.Error("the trashed thread should close") + } + if v.postingIndex(100) >= 0 { + t.Error("the trashed thread should leave the list") + } + if toast != "Thread moved to Trash" { + t.Errorf("toast = %q, want the trash confirmation", toast) + } +} + +// The menu lives in the help bar, not in a box over the mail: m swaps the bar's +// bindings for the extra actions, the thread stays on screen and keeps scrolling, +// and q or esc swaps the bar back. +func TestThreadMoreMenuSwapsTheHelpBar(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + openTestThread(t, v) + before := v.View() + + v.HandleContentKey(keyPress("m")) + if !hasHelpBinding(v.HelpBindings(), "t") || !hasHelpBinding(v.HelpBindings(), "esc/q") { + t.Errorf("menu bindings = %v, want the extra actions and esc/q", v.HelpBindings()) + } + if v.View() != before { + t.Error("the menu should leave the thread on screen untouched") + } + + v.HandleContentKey(keyPress("down")) + if v.modal == nil { + t.Error("scrolling the thread should leave the menu up") + } + + v.HandleContentKey(keyPress("q")) + if v.modal != nil { + t.Error("q should put the help bar back") + } + if !v.inThread { + t.Error("leaving the menu should leave the thread on screen") + } + if !hasHelpBinding(v.HelpBindings(), "m") { + t.Errorf("thread bindings = %v, want m offered again", v.HelpBindings()) + } +} + +// The menu's items hand off to the same places their list keys go, and a picker that +// opens replaces the menu rather than being closed along with it. +func TestThreadMoreMenuOpensThePickers(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + openTestThread(t, v) + + v.HandleContentKey(keyPress("m")) + v.HandleContentKey(keyPress("b")) + picker, ok := v.modal.(*folderPicker) + if !ok { + t.Fatalf("b opened %T, want the label picker", v.modal) + } + if picker.posting.ID != 100 { + t.Errorf("label picker holds posting %d, want the open thread's 100", picker.posting.ID) + } + v.HandleContentKey(keyPress("esc")) + + v.HandleContentKey(keyPress("m")) + v.HandleContentKey(keyPress("v")) + if _, ok := v.modal.(*movePicker); !ok { + t.Fatalf("v opened %T, want the move picker", v.modal) + } + v.HandleContentKey(keyPress("esc")) + + v.HandleContentKey(keyPress("m")) + v.HandleContentKey(keyPress("esc")) + if v.modal != nil { + t.Errorf("esc left %T open, want the menu closed", v.modal) + } + if !v.inThread { + t.Error("leaving the menu should leave the thread on screen") + } +} + +// A live refresh can carry the opened row out of the list underneath the reader: +// opening the thread marked it seen, and the re-read head page no longer reaches it. +// The menu works off the posting captured when the thread was opened, so it neither +// leaves the help bar nor stops answering. +func TestThreadMoreMenuSurvivesTheRowLeavingTheList(t *testing.T) { + v, recorded := mailWithTestServer(t, http.StatusNoContent) + openTestThread(t, v) + + v.Update(postingsRefreshedMsg{ + requestID: v.liveRequestID, + boxID: 1, + sourceKind: v.currentSourceKind(), + postings: []mail.Posting{{ID: 300, TopicID: 300, Summary: "Newer thread", Creator: mail.Contact{Name: "Cara"}}}, + }) + if v.postingIndex(100) >= 0 { + t.Fatal("the refresh should have carried the opened row out of the list") + } + if !hasHelpBinding(v.HelpBindings(), "m") { + t.Error("the menu should still be offered after the row left") + } + + v.HandleContentKey(keyPress("m")) + if _, ok := v.modal.(*moreMenu); !ok { + t.Fatalf("m opened %T, want the More menu", v.modal) + } + done, ok := runCmd(v.HandleContentKey(keyPress("t"))).(postingActionDoneMsg) + if !ok || done.err != nil { + t.Fatalf("trash returned %#v", done) + } + if recorded.path != "/postings/trash.json" || len(recorded.body.PostingIDs) != 1 || recorded.body.PostingIDs[0] != 100 { + t.Errorf("request = %s %v, want POST /postings/trash.json [100]", recorded.path, recorded.body.PostingIDs) + } + answer, _ := v.Update(done) + deliverToView(v, answer) + if v.inThread { + t.Error("the trashed thread should close") + } +} + +// A reply asked for after the trash left dies with the thread: the trash's done +// message cancels the loading context, so a compose form cannot open over the list +// for a thread that is already in the Trash. +func TestThreadTrashCancelsThePendingReplyLoad(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + openTestThread(t, v) + + v.HandleContentKey(keyPress("m")) + trash := v.HandleContentKey(keyPress("t")) + v.HandleContentKey(keyPress("r")) // reply context now loading on the lane + if v.requests.kind != mailRequestReply { + t.Fatalf("request lane = %v, want a reply load in flight", v.requests.kind) + } + replyID := v.requests.id + + done := runCmd(trash).(postingActionDoneMsg) + answer, _ := v.Update(done) + deliverToView(v, answer) + if v.inThread { + t.Fatal("the trashed thread should close") + } + if v.requests.loading { + t.Error("closing the thread should cancel the reply load") + } + + v.Update(replyContextLoadedMsg{requestID: replyID, boxID: 1}) + if v.modal != nil { + t.Errorf("the cancelled reply still opened %T over the list", v.modal) + } +} + +// A reader who has already moved on when the trash lands keeps what they were doing: +// an open form or picker — typed text at stake — survives, and the thread stays for +// them to leave, as they always could. +func TestThreadTrashLeavesAnOccupiedReaderAlone(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + openTestThread(t, v) + + v.HandleContentKey(keyPress("m")) + trash := v.HandleContentKey(keyPress("t")) + v.HandleContentKey(keyPress("m")) // back in the menu while the trash is in flight + if _, ok := v.modal.(*moreMenu); !ok { + t.Fatalf("m reopened %T, want the More menu", v.modal) + } + + done := runCmd(trash).(postingActionDoneMsg) + answer, _ := v.Update(done) + deliverToView(v, answer) + if !v.inThread { + t.Error("the thread closed under a reader who had moved on") + } + if _, ok := v.modal.(*moreMenu); !ok { + t.Errorf("the modal became %T, want the menu kept", v.modal) + } + if v.postingIndex(100) >= 0 { + t.Error("the trashed row should still leave the list") + } +} + +// Leaving the thread while the trash is still in flight must not leave the trashed +// row behind on a search or bundle list — those lists never re-read themselves. +func TestThreadTrashRemovesTheRowAfterTheReaderLeft(t *testing.T) { + v, recorded := mailWithTestServer(t, http.StatusNoContent) + v.searchActive = true + v.searchList.setPostings([]mail.Posting{{ID: 100, TopicID: 100, Name: "Hello world", Creator: mail.Contact{Name: "Alice"}}}) + loaded := runCmd(v.HandleContentKey(keyPress("enter"))).(topicLoadedMsg) + v.Update(loaded) + if !v.inThread { + t.Fatal("the search result did not open") + } + + v.HandleContentKey(keyPress("m")) + trash := v.HandleContentKey(keyPress("t")) + v.ExitThread() // esc before the server answers + if v.inThread { + t.Fatal("esc should leave the thread") + } + + done := runCmd(trash).(postingActionDoneMsg) + if done.err != nil { + t.Fatalf("trash returned %#v", done) + } + if recorded.path != "/postings/trash.json" { + t.Fatalf("request = %s, want /postings/trash.json", recorded.path) + } + v.Update(done) + if postingIndexIn(v.searchList.postings, 100) >= 0 { + t.Error("the trashed row stayed on the search list") + } +} + +// A search match says neither which box its thread lives in nor which labels it +// carries, so from a search-opened thread the menu keeps to forward and trash rather +// than building label and move pickers over wrong choices. +func TestThreadMoreMenuKeepsToForwardAndTrashOverSearch(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + v.searchActive = true + v.searchList.setPostings([]mail.Posting{{ID: 100, TopicID: 100, Name: "Hello world", Creator: mail.Contact{Name: "Alice"}}}) + loaded := runCmd(v.HandleContentKey(keyPress("enter"))).(topicLoadedMsg) + v.Update(loaded) + + v.HandleContentKey(keyPress("m")) + if hasHelpBinding(v.HelpBindings(), "b") || hasHelpBinding(v.HelpBindings(), "v") { + t.Errorf("menu bindings = %v, want label and move withheld over search", v.HelpBindings()) + } + v.HandleContentKey(keyPress("v")) + if _, ok := v.modal.(*movePicker); ok { + t.Error("v opened a move picker built over the browsed box, not the thread's") + } + v.HandleContentKey(keyPress("b")) + if _, ok := v.modal.(*folderPicker); ok { + t.Error("b opened a label picker over a posting whose labels are unknown") + } +} + +// The menu takes only its own keys; the rest keep their thread meaning, so reading +// never degrades while the menu is up — r still starts a reply, not a scroll. +func TestThreadMoreMenuPassesThreadKeysThrough(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + openTestThread(t, v) + + v.HandleContentKey(keyPress("m")) + v.HandleContentKey(keyPress("r")) + if v.requests.kind != mailRequestReply { + t.Errorf("request lane = %v, want r under the menu to start the reply load", v.requests.kind) + } +} + +// A label added while the thread is open lives on the list's row, not the snapshot +// taken at opening — the menu re-reads the row so its label picker cannot offer to +// add the label again. +func TestThreadMoreMenuReadsTheFreshRow(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + openTestThread(t, v) + + idx := v.postingIndex(100) + v.postingList.postings[idx].Folders = []mail.Folder{{ID: 9, Name: "Receipts"}} + v.HandleContentKey(keyPress("m")) + if len(v.topicPosting.Folders) != 1 || v.topicPosting.Folders[0].ID != 9 { + t.Errorf("menu posting folders = %v, want the row's fresh labels", v.topicPosting.Folders) + } +} + +// A live refresh landing while the topic itself is still loading must not cost the +// thread its menu: the posting was in hand when the thread was asked for. +func TestThreadMoreMenuSurvivesARefreshDuringTheTopicLoad(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + + open := v.HandleContentKey(keyPress("enter")) + v.Update(postingsRefreshedMsg{ + requestID: v.liveRequestID, + boxID: 1, + sourceKind: v.currentSourceKind(), + postings: []mail.Posting{{ID: 300, TopicID: 300, Summary: "Newer thread", Creator: mail.Contact{Name: "Cara"}}}, + }) + loaded := runCmd(open).(topicLoadedMsg) + cmd, _ := v.Update(loaded) + if seen, ok := runCmd(cmd).(postingSeenMsg); ok { + v.Update(seen) + } + if !v.inThread || v.topicPosting == nil || v.topicPosting.ID != 100 { + t.Fatalf("thread state = open:%v posting:%v, want posting 100 held", v.inThread, v.topicPosting) + } + if !hasHelpBinding(v.HelpBindings(), "m") { + t.Error("the menu should still be offered") + } +} + +// A trash the server refuses leaves the reader where they were: in the thread, the +// row still on the list, the failure reported. +func TestThreadMoreMenuKeepsTheThreadWhenTrashFails(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusInternalServerError) + openTestThread(t, v) + + v.HandleContentKey(keyPress("m")) + done, ok := runCmd(v.HandleContentKey(keyPress("t"))).(postingActionDoneMsg) + if !ok || done.err == nil { + t.Fatalf("a refused trash returned %#v, want its error", done) + } + cmd, _ := v.Update(done) + if _, ok := runCmd(cmd).(errMsg); !ok { + t.Error("a refused trash should be reported") + } + if !v.inThread { + t.Error("a refused trash should leave the thread open") + } + if v.postingIndex(100) < 0 { + t.Error("a refused trash should leave the row on the list") + } +} + +// A topic opened directly was not opened from any list, so there is no posting for +// the menu to act on: m does nothing and the help bar does not offer it. +func TestThreadMoreMenuNeedsAPosting(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + loaded, ok := runCmd(v.requestTopic(0, 100, 0, "")).(topicLoadedMsg) + if !ok { + t.Fatal("direct topic did not return topicLoadedMsg") + } + v.Update(loaded) + if !v.inThread { + t.Fatal("direct topic did not open") + } + + v.HandleContentKey(keyPress("m")) + if v.modal != nil { + t.Errorf("m opened %T over a topic with no posting", v.modal) + } + if hasHelpBinding(v.HelpBindings(), "m") { + t.Error("the help bar offers the menu with nothing for it to act on") + } +} + // --- Posting actions --- func TestMailViewPostingKeysCallExpectedEndpoints(t *testing.T) { diff --git a/internal/tui/more_menu.go b/internal/tui/more_menu.go new file mode 100644 index 00000000..f07009c5 --- /dev/null +++ b/internal/tui/more_menu.go @@ -0,0 +1,69 @@ +package tui + +import ( + "strings" + + tea "charm.land/bubbletea/v2" + + "github.com/basecamp/hey-cli/internal/mail" +) + +// moreMenu is the thread view's More menu, as in the HEY apps: m swaps the help bar +// for the actions a thread offers beyond reading it, each behind the key the same +// action answers to on the posting list. The thread stays on screen and every thread +// key keeps its meaning underneath — the menu is a mode of the help bar, not a box +// over the mail. It holds the posting the thread was opened from, so the actions keep +// their aim even while the list underneath refreshes. +type moreMenu struct { + plainModal + + posting mail.Posting + // organizes is whether label and move are offered: only a list that knows the + // thread's box and labels can back their pickers. + organizes bool +} + +func newMoreMenu(posting mail.Posting, organizes bool) *moreMenu { + return &moreMenu{posting: posting, organizes: organizes} +} + +// handleKey commits an action by its key and sends every other key through the +// thread's own routing, so having the menu up never gets in the way of reading. +func (m *moreMenu) handleKey(view *mailView, msg tea.KeyPressMsg) (tea.Cmd, bool) { + if msg.Key().Code == tea.KeyEscape { + return nil, false + } + switch key := strings.ToLower(msg.String()); key { + case "f": + return view.loadForwardContext(view.topicID, view.topicName), false + case "b": + if m.organizes { + cmd, open := view.openFolderPicker(m.posting) + // A failed label discovery leaves its "press b to retry" notice over the + // thread, where bare b means nothing — the retry keeps the menu up so + // the advertised key stays bound to retrying. + return cmd, open || view.folderDiscoveryErr != "" + } + case "v": + if m.organizes { + return nil, view.openMovePicker(m.posting) + } + case "t": + return view.trashOpenThread(m.posting.ID), false + case "q", "m": + return nil, false + } + return view.handleThreadKey(msg), true +} + +func (m *moreMenu) draw(view *mailView) string { + return view.threadView() +} + +func (m *moreMenu) helpBindings() []helpBinding { + bindings := []helpBinding{{"f", "forward"}} + if m.organizes { + bindings = append(bindings, helpBinding{"b", "label"}, helpBinding{"v", "move"}) + } + return append(bindings, helpBinding{"t", "trash"}, helpBinding{"esc/q", "back"}) +} From 6694a96c539563f05304f2c461b5c4cd6dcf03e3 Mon Sep 17 00:00:00 2001 From: jalil Date: Fri, 28 Aug 2026 15:57:46 +0200 Subject: [PATCH 2/2] Keep the thread's retained posting in step with its labels The More menu builds its label picker off the posting the thread was opened from, and that snapshot is the only copy left once a live re-read has carried the row out of the list - which is the whole reason it is kept. A label filed or removed from the menu never reached it: the completion re-read the box list and nothing else. Reopening the picker then showed a label the thread already carries as unchecked, and enter filed it a second time instead of removing it. The completion now carries the posting and the label it moved, the way the collection one already does, and lays that change onto every copy still in hand - the thread's retained posting and the row on whichever list holds it, which also covers the bundle and Previously Seen lists that are never re-read. A created label is known only by its name until the source reload hands back its ID, so a label without one matches on its name. Tests: labelling from the menu with the row gone leaves the reopened picker offering to remove the label rather than file it again, for both a filed and a freshly created one. Two more cover the menu over bundles, which nothing reached before - label and move offered inside an unseen bundle, withheld over a contact's threads. --- internal/tui/folders.go | 8 +- internal/tui/mail.go | 68 ++++++++++++++- internal/tui/mail_test.go | 171 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 239 insertions(+), 8 deletions(-) diff --git a/internal/tui/folders.go b/internal/tui/folders.go index 8ea9d4c3..972cd6b9 100644 --- a/internal/tui/folders.go +++ b/internal/tui/folders.go @@ -78,9 +78,9 @@ func (p *folderPicker) selected() *folderPickerSelection { return &choices[p.cursor] } -func (p *folderPicker) postingHasFolder(folderID int64) bool { +func (p *folderPicker) postingHasFolder(choice mail.Source) bool { for _, folder := range p.posting.Folders { - if folder.ID == folderID { + if sameFolder(folder, mail.Folder{ID: choice.ID, Name: choice.Name}) { return true } } @@ -134,7 +134,7 @@ func (p *folderPicker) handleKey(view *mailView, msg tea.KeyPressMsg) (tea.Cmd, if selection := p.selected(); selection != nil { switch selection.kind { case folderPickerExisting: - if p.postingHasFolder(selection.folder.ID) { + if p.postingHasFolder(selection.folder) { return view.unfilePosting(p.posting.ID, selection.folder.ID, selection.folder.Name), false } return view.filePosting(p.posting.ID, selection.folder.ID, selection.folder.Name), false @@ -237,7 +237,7 @@ func (p *folderPicker) view(styles styles, width int) string { switch choice.kind { case folderPickerExisting: mark := "[ ]" - if p.postingHasFolder(choice.folder.ID) { + if p.postingHasFolder(choice.folder) { mark = "[x]" } label = mark + " " + terminal.SanitizeLine(choice.folder.Name) diff --git a/internal/tui/mail.go b/internal/tui/mail.go index 0cac10dc..959fa919 100644 --- a/internal/tui/mail.go +++ b/internal/tui/mail.go @@ -226,6 +226,9 @@ type folderActionDoneMsg struct { action string sourceID int64 sourceKind mail.Kind + postingID int64 + folder mail.Folder + added bool created bool err error } @@ -787,6 +790,7 @@ func (v *mailView) Update(msg tea.Msg) (tea.Cmd, bool) { } return nil, true } + v.notePostingLabelled(msg) var done tea.Cmd if msg.sourceID == v.currentBoxID() && msg.sourceKind == v.currentSourceKind() { done = notify(msg.action) @@ -2225,13 +2229,18 @@ func (v *mailView) openFolderPicker(posting mail.Posting) (tea.Cmd, bool) { } func (v *mailView) filePosting(postingID, folderID int64, folderName string) tea.Cmd { - return v.doFolderAction("Label "+terminal.SanitizeLine(folderName)+" added", false, func() error { + folder := mail.Folder{ID: folderID, Name: folderName} + return v.doFolderAction("Label "+terminal.SanitizeLine(folderName)+" added", postingID, folder, true, false, func() error { return v.vc.sdk.Postings().File(v.vc.ctx, folderID, postingID) }) } +// createFolderForPosting labels the thread with a label that does not exist yet. The +// server answers the creation with nothing but its blessing, so the new label is known +// by name until the source reload that follows hands back its ID. func (v *mailView) createFolderForPosting(postingID int64, folderName string) tea.Cmd { - return v.doFolderAction("Label "+terminal.SanitizeLine(folderName)+" created", true, func() error { + folder := mail.Folder{Name: folderName} + return v.doFolderAction("Label "+terminal.SanitizeLine(folderName)+" created", postingID, folder, true, true, func() error { return v.vc.sdk.Postings().CreateFolder(v.vc.ctx, folderName, postingID) }) } @@ -2241,12 +2250,13 @@ func (v *mailView) unfilePosting(postingID, folderID int64, folderName string) t if folderID != 0 { label = "Label " + terminal.SanitizeLine(folderName) + " removed" } - return v.doFolderAction(label, false, func() error { + folder := mail.Folder{ID: folderID, Name: folderName} + return v.doFolderAction(label, postingID, folder, false, false, func() error { return v.vc.sdk.Postings().Unfile(v.vc.ctx, folderID, postingID) }) } -func (v *mailView) doFolderAction(label string, created bool, fn func() error) tea.Cmd { +func (v *mailView) doFolderAction(label string, postingID int64, folder mail.Folder, added, created bool, fn func() error) tea.Cmd { sourceID, sourceKind := v.currentSourceIdentity() v.pendingMutations++ return func() tea.Msg { @@ -2254,12 +2264,62 @@ func (v *mailView) doFolderAction(label string, created bool, fn func() error) t action: label, sourceID: sourceID, sourceKind: sourceKind, + postingID: postingID, + folder: folder, + added: added, created: created, err: fn(), } } } +// notePostingLabelled carries a landed label change onto every copy of the posting the +// reader still holds. The list's row is the obvious one, but the thread's retained +// posting matters more: once a live re-read has carried the row out of the list that +// snapshot is the only copy left, and it is what the More menu builds its label picker +// off. Left stale, the picker shows a label the thread already carries as unchecked and +// files it a second time instead of removing it. +func (v *mailView) notePostingLabelled(msg folderActionDoneMsg) { + if row := v.openedPosting(msg.postingID); row != nil { + updatePostingFolders(row, msg.folder, msg.added) + } + if v.topicPosting != nil && v.topicPosting.ID == msg.postingID { + updatePostingFolders(v.topicPosting, msg.folder, msg.added) + } +} + +// updatePostingFolders answers a label change on a posting in hand; an unnamed label +// removed is the picker's "remove all labels". It builds a fresh slice rather than +// editing in place, because the retained posting and the list's row are separate copies +// that still share one backing array, and rewriting that array under both is how one of +// them ends up wrong. +func updatePostingFolders(posting *mail.Posting, folder mail.Folder, added bool) { + if !added && folder.ID == 0 { + posting.Folders = nil + return + } + kept := make([]mail.Folder, 0, len(posting.Folders)+1) + for _, carried := range posting.Folders { + if !sameFolder(carried, folder) { + kept = append(kept, carried) + } + } + if added { + kept = append(kept, folder) + } + posting.Folders = kept +} + +// sameFolder matches two labels. A label just created for a thread is known only by the +// name it was created under until the source reload that follows hands back its ID, so a +// label without one matches on its name. +func sameFolder(a, b mail.Folder) bool { + if a.ID != 0 && b.ID != 0 { + return a.ID == b.ID + } + return a.Name == b.Name +} + func (v *mailView) startCollectionPicker() tea.Cmd { if v.collectionDiscoveryErr != "" { v.notice = "Retrying collections…" diff --git a/internal/tui/mail_test.go b/internal/tui/mail_test.go index b38a9ddd..c03f9513 100644 --- a/internal/tui/mail_test.go +++ b/internal/tui/mail_test.go @@ -799,6 +799,177 @@ func TestThreadMoreMenuKeepsToForwardAndTrashOverSearch(t *testing.T) { } } +// openTestBundleThread opens the bundle on the list and then the first thread inside +// it, which is how the menu is reached over a bundle rather than over a box list. +func openTestBundleThread(t *testing.T, v *mailView, postingID int64) { + t.Helper() + loaded, ok := runCmd(v.HandleContentKey(keyPress("enter"))).(bundleLoadedMsg) + if !ok || loaded.err != nil { + t.Fatalf("opening the bundle returned %#v", loaded) + } + v.Update(loaded) + topic, ok := runCmd(v.HandleContentKey(keyPress("enter"))).(topicLoadedMsg) + if !ok || topic.err != nil { + t.Fatalf("opening the bundle's thread returned %#v", topic) + } + cmd, _ := v.Update(topic) + if seen, ok := runCmd(cmd).(postingSeenMsg); ok { + v.Update(seen) + } + if !v.inThread || v.topicPosting == nil || v.topicPosting.ID != postingID { + t.Fatalf("thread state = open:%v posting:%v, want posting %d open", v.inThread, v.topicPosting, postingID) + } +} + +// An unseen bundle holds the box's own postings — the box they live in is the box on +// screen, and each row carries its labels — so the menu offers label and move there +// just as it does over the list. +func TestThreadMoreMenuOrganizesFromAnUnseenBundle(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + v.boxes = append(v.boxes, mail.Source{ID: 12, Kind: mail.KindFolder, Name: "Receipts"}) + v.postingList.postings[0] = bundleRow() + openTestBundleThread(t, v, 511) + + v.HandleContentKey(keyPress("m")) + if !hasHelpBinding(v.HelpBindings(), "b") || !hasHelpBinding(v.HelpBindings(), "v") { + t.Errorf("menu bindings = %v, want label and move offered inside an unseen bundle", v.HelpBindings()) + } + v.HandleContentKey(keyPress("b")) + picker, ok := v.modal.(*folderPicker) + if !ok { + t.Fatalf("b opened %T, want the label picker", v.modal) + } + if picker.posting.ID != 511 { + t.Errorf("label picker holds posting %d, want the bundle member's 511", picker.posting.ID) + } + + v.HandleContentKey(keyPress("esc")) + v.HandleContentKey(keyPress("m")) + v.HandleContentKey(keyPress("v")) + if _, ok := v.modal.(*movePicker); !ok { + t.Fatalf("v opened %T, want the move picker", v.modal) + } +} + +// A read bundle opens as every thread with its contact, gathered from across the boxes: +// a row there says neither which box its thread sits in nor which labels it carries, so +// the menu keeps to forward and trash the way it does over search. +func TestThreadMoreMenuKeepsToForwardAndTrashOverContactThreads(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + v.boxes = append(v.boxes, mail.Source{ID: 12, Kind: mail.KindFolder, Name: "Receipts"}) + row := bundleRow() + row.Seen = true + v.postingList.postings[0] = row + openTestBundleThread(t, v, 513) + if v.bundleContactID != 88 { + t.Fatalf("bundle contact = %d, want the contact's threads", v.bundleContactID) + } + + v.HandleContentKey(keyPress("m")) + if hasHelpBinding(v.HelpBindings(), "b") || hasHelpBinding(v.HelpBindings(), "v") { + t.Errorf("menu bindings = %v, want label and move withheld over a contact's threads", v.HelpBindings()) + } + v.HandleContentKey(keyPress("v")) + if _, ok := v.modal.(*movePicker); ok { + t.Error("v opened a move picker built over the browsed box, not the thread's") + } + v.HandleContentKey(keyPress("b")) + if _, ok := v.modal.(*folderPicker); ok { + t.Error("b opened a label picker over a posting whose labels are unknown") + } + if !hasHelpBinding(v.HelpBindings(), "f") || !hasHelpBinding(v.HelpBindings(), "t") { + t.Errorf("menu bindings = %v, want forward and trash still offered", v.HelpBindings()) + } +} + +// A label filed from the menu once a live re-read has carried the row out of the list +// has only the retained posting to land on — there is no row left to re-read it from. +// Kept in step, reopening the picker shows the label checked and enter removes it; +// stale, the picker would offer to file the same label a second time. +func TestThreadMoreMenuKeepsItsLabelsWhenTheRowIsGone(t *testing.T) { + v, recorded := mailWithTestServer(t, http.StatusNoContent) + v.boxes = append(v.boxes, mail.Source{ID: 12, Kind: mail.KindFolder, Name: "Receipts"}) + openTestThread(t, v) + + v.Update(postingsRefreshedMsg{ + requestID: v.liveRequestID, + boxID: 1, + sourceKind: v.currentSourceKind(), + postings: []mail.Posting{{ID: 300, TopicID: 300, Summary: "Newer thread", Creator: mail.Contact{Name: "Cara"}}}, + }) + if v.postingIndex(100) >= 0 { + t.Fatal("the refresh should have carried the opened row out of the list") + } + + v.HandleContentKey(keyPress("m")) + v.HandleContentKey(keyPress("b")) + filed, ok := runCmd(v.HandleContentKey(keyPress("enter"))).(folderActionDoneMsg) + if !ok || filed.err != nil || !filed.added { + t.Fatalf("labelling returned %#v", filed) + } + if recorded.path != "/postings/filings.json" { + t.Fatalf("request = %s, want the label filed", recorded.path) + } + v.Update(filed) + if len(v.topicPosting.Folders) != 1 || v.topicPosting.Folders[0].ID != 12 { + t.Fatalf("retained posting folders = %v, want the filed label", v.topicPosting.Folders) + } + + v.HandleContentKey(keyPress("m")) + v.HandleContentKey(keyPress("b")) + if _, ok := v.modal.(*folderPicker); !ok { + t.Fatalf("b reopened %T, want the label picker", v.modal) + } + if view := v.View(); !strings.Contains(view, "[x] Receipts") || !strings.Contains(view, "Remove all labels") { + t.Errorf("reopened picker view = %q, want Receipts checked", view) + } + removed, ok := runCmd(v.HandleContentKey(keyPress("enter"))).(folderActionDoneMsg) + if !ok || removed.added { + t.Fatalf("enter over the carried label returned %#v, want it removed rather than filed again", removed) + } + v.Update(removed) + if len(v.topicPosting.Folders) != 0 { + t.Errorf("retained posting folders = %v, want the label gone", v.topicPosting.Folders) + } +} + +// A label created from the menu is known only by its name until the sources reload +// hands back its ID, and that is enough to keep the retained posting honest: reopening +// the picker shows the new label checked rather than offering to file it again. +func TestThreadMoreMenuKeepsALabelItJustCreated(t *testing.T) { + v, _ := mailWithTestServer(t, http.StatusNoContent) + openTestThread(t, v) + + v.Update(postingsRefreshedMsg{ + requestID: v.liveRequestID, + boxID: 1, + sourceKind: v.currentSourceKind(), + postings: []mail.Posting{{ID: 300, TopicID: 300, Summary: "Newer thread", Creator: mail.Contact{Name: "Cara"}}}, + }) + + v.HandleContentKey(keyPress("m")) + v.HandleContentKey(keyPress("b")) + v.HandleContentKey(keyPress("enter")) // + Create a new label… + folderModal(v).input.SetValue("Receipts") + created, ok := runCmd(v.HandleContentKey(keyPress("enter"))).(folderActionDoneMsg) + if !ok || created.err != nil || !created.created { + t.Fatalf("creating the label returned %#v", created) + } + v.Update(created) + + // The reload that follows brings the label back with the ID the server gave it. + v.boxes = append(v.boxes, mail.Source{ID: 12, Kind: mail.KindFolder, Name: "Receipts"}) + v.HandleContentKey(keyPress("m")) + v.HandleContentKey(keyPress("b")) + if view := v.View(); !strings.Contains(view, "[x] Receipts") { + t.Errorf("reopened picker view = %q, want the created label checked", view) + } + removed, ok := runCmd(v.HandleContentKey(keyPress("enter"))).(folderActionDoneMsg) + if !ok || removed.added { + t.Fatalf("enter over the created label returned %#v, want it removed rather than filed again", removed) + } +} + // The menu takes only its own keys; the rest keep their thread meaning, so reading // never degrades while the menu is up — r still starts a reply, not a scroll. func TestThreadMoreMenuPassesThreadKeysThrough(t *testing.T) {