fix: QoL change unread / read behaviour

README.md

@@ -337,8 +337,9 @@ Thread categories are:
 New threads retain a `NEW THREAD` marker. When an existing thread receives new
 comments, diple places a `NEW MESSAGES` divider before the first unread comment
 and emphasizes the unread comment rail. Moving the thread-list cursor does not
-clear this state. It is cleared after the last unread comment becomes visible
+clear this state. It is cleared when the thread detail pane receives focus, when
-while scrolling the focused detail pane, or manually with
+the thread is resolved, after the last unread comment becomes visible while
+scrolling the focused detail pane, or manually with
 `keybindings.threads.mark_read` (`m` by default).
 
 Within a category, `"file"` keeps paths together and `"timestamp"` sorts by

tui.go

@@ -415,7 +415,7 @@ func (m *App) startReply() {
 	m.writeMode, m.writeThreadID, m.replyDraft, m.err = writeReply, thread.ID, "", nil
 	m.restoreReplyDraft(thread.ID)
 	m.folded[thread.ID] = false
-	m.focus = threadDetailPane
+	m.focusThreadDetail()
 	m.scroll = m.detailMaxScroll()
 }
 
@@ -960,6 +960,9 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 			m.folded[thread.ID] = thread.IsResolved && m.foldResolved
 			break
 		}
+		if msg.thread.IsResolved {
+			m.markThreadRead(msg.threadID)
+		}
 		sortReviewThreads(m.details.Threads, m.threadStatusOrder, m.threadWithinStatus)
 		m.threadIndex = indexThread(m.details.Threads, selected)
 		m.writeThreadID = ""
@@ -1247,7 +1250,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		if m.screen == threadScreen {
 			m.listHidden = !m.listHidden
 			if m.listHidden {
-				m.focus = threadDetailPane
+				m.focusThreadDetail()
 			} else {
 				m.focus = threadListPane
 			}
@@ -1290,7 +1293,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 			return m, nil
 		}
 		if m.screen == threadScreen {
-			m.focus = threadDetailPane
+			m.focusThreadDetail()
 		}
 	case "h":
 		if m.screen == threadScreen {
@@ -1480,29 +1483,53 @@ func (m *App) trackThreadUpdates(details PRDetails) {
 }
 
 func (m *App) markCurrentThreadRead() {
-	if m.threadIndex >= 0 && m.threadIndex < len(m.details.Threads) {
+	if m.threadIndex < 0 || m.threadIndex >= len(m.details.Threads) {
-		thread := m.details.Threads[m.threadIndex]
+		return
-		delete(m.unreadThreads, thread.ID)
+	}
-		delete(m.newThreads, thread.ID)
+	m.markThreadRead(m.details.Threads[m.threadIndex].ID)
-		prID := m.currentPRKey()
+}
-		state := m.readState.Data[prID]
+
-		if state.Threads == nil {
+func (m *App) markThreadRead(threadID string) {
-			state.Threads = make(map[string]bool)
+	thread := m.threadByID(threadID)
-		}
+	if thread == nil {
-		if state.Comments == nil {
+		return
-			state.Comments = make(map[string]bool)
+	}
-		}
+	prID := m.currentPRKey()
-		state.Initialized = true
+	state := m.readState.Data[prID]
-		state.Threads[thread.ID] = true
+	changed := m.unreadThreads[thread.ID] || m.newThreads[thread.ID] ||
-		for _, comment := range thread.Comments {
+		!state.Initialized || !state.Threads[thread.ID]
-			state.Comments[comment.ID] = true
+	for _, comment := range thread.Comments {
-			delete(m.unreadComments, comment.ID)
+		if m.unreadComments[comment.ID] || !state.Comments[comment.ID] {
-		}
+			changed = true
-		m.readState.Data[prID] = state
+			break
-		if err := m.readState.save(); err != nil {
-			m.recordHealth("read state", healthWarning, err.Error())
 		}
 	}
+	if !changed {
+		return
+	}
+	delete(m.unreadThreads, thread.ID)
+	delete(m.newThreads, thread.ID)
+	if state.Threads == nil {
+		state.Threads = make(map[string]bool)
+	}
+	if state.Comments == nil {
+		state.Comments = make(map[string]bool)
+	}
+	state.Initialized = true
+	state.Threads[thread.ID] = true
+	for _, comment := range thread.Comments {
+		state.Comments[comment.ID] = true
+		delete(m.unreadComments, comment.ID)
+	}
+	m.readState.Data[prID] = state
+	if err := m.readState.save(); err != nil {
+		m.recordHealth("read state", healthWarning, err.Error())
+	}
+}
+
+func (m *App) focusThreadDetail() {
+	m.focus = threadDetailPane
+	m.markCurrentThreadRead()
 }
 
 func (m *App) markCommentRead(commentID string) {
@@ -1545,10 +1572,9 @@ func (m *App) scrollToFirstUnread() {
 		return
 	}
 	width, _ := m.detailPaneSize()
-	dividerAnchor := "unread:" + firstUnread
 	commentAnchor := "comment:" + firstUnread + ":header"
 	for index, line := range m.renderedDetailLines(width) {
-		if line.anchor == dividerAnchor || line.anchor == commentAnchor {
+		if line.anchor == commentAnchor {
 			m.scroll = min(index, m.detailMaxScroll())
 			return
 		}
@@ -1595,8 +1621,10 @@ func (m *App) moveToUnread(direction int) {
 		}
 		if m.unreadThreads[m.details.Threads[index].ID] {
 			m.threadIndex, m.scroll = index, 0
-			m.focus = threadDetailPane
 			m.scrollToFirstUnread()
+			anchor := m.detailScrollAnchor()
+			m.focusThreadDetail()
+			m.restoreDetailAnchor(anchor)
 			return
 		}
 	}

tui_test.go

@@ -1301,19 +1301,47 @@ func TestPollingMarksNewThreadCommentsUnread(t *testing.T) {
 	}
 	updated, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("l")})
 	m = updated.(App)
-	if !m.unreadThreads["thread"] {
+	if m.unreadThreads["thread"] || m.unreadComments["comment-2"] {
-		t.Fatal("focusing the thread detail marked the thread read")
+		t.Fatal("focusing the thread detail did not mark the thread read")
 	}
+
+	m.unreadThreads["thread"] = true
+	m.unreadComments["comment-2"] = true
 	updated, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("n")})
 	m = updated.(App)
-	if !m.unreadThreads["thread"] || m.focus != threadDetailPane ||
+	if m.unreadThreads["thread"] || m.unreadComments["comment-2"] ||
-		m.detailScrollAnchor() != "unread:comment-2" {
+		m.focus != threadDetailPane {
-		t.Fatal("next-unread did not preserve and position the unread update")
+		t.Fatal("next-unread did not open and mark the thread discussion read")
 	}
-	updated, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("j")})
+}
+
+func TestResolvingThreadMarksItRead(t *testing.T) {
+	service := &recordingService{}
+	m := NewApp(service, "o", "r", false, 50, time.Second)
+	m.screen, m.loading = threadScreen, false
+	m.details = PRDetails{
+		PullRequest: PullRequest{ID: "pr", Owner: "o", Repository: "r", Number: 1},
+		Threads: []ReviewThread{{
+			ID: "thread", ViewerCanResolve: true,
+			Comments: []ReviewComment{{ID: "comment"}},
+		}},
+	}
+	m.unreadThreads["thread"] = true
+	m.unreadComments["comment"] = true
+
+	updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("R")})
 	m = updated.(App)
-	if m.unreadThreads["thread"] || m.unreadComments["comment-2"] {
+	updated, command := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("y")})
-		t.Fatal("scrolling the visible final unread comment did not mark the thread read")
+	m = updated.(App)
+	updated, _ = m.Update(command())
+	m = updated.(App)
+
+	if m.unreadThreads["thread"] || m.unreadComments["comment"] {
+		t.Fatal("resolved thread remained unread")
+	}
+	state := m.readState.Data["pr"]
+	if !state.Threads["thread"] || !state.Comments["comment"] {
+		t.Fatalf("resolved thread read state was not persisted: %#v", state)
 	}
 }
 

version.go

@@ -1,3 +1,3 @@
 package main
 
-const dipleVersion = "0.1.0"
+const dipleVersion = "0.1.1"