Skip to content

Commit 1fbf975

Browse files
authored
fix(messages): stop empty channel list after notification back navigation (#1994)
Tapping a message notification deep-links into the correct conversation, but tapping back left the channel list empty until you backed out again and re-entered Channels. Root cause: the Messages sidebar `List` bound its selection directly to `router.messagesState`, which a deep link sets to a payload-carrying value (e.g. `.channels(channelId: 2, messageId: 5)`). The sidebar only offers payload-free `.channels()` / `.directMessages()` rows, so that value matched no row and corrupted the collapsed NavigationSplitView back stack. Decouple the two concerns the single property was serving: - `messagesState` is now a transient deep-link command (carries the payload), consumed exactly once by the Messages view and then cleared. - new `messagesSection` is the durable, payload-free sidebar/section selection; it always matches a row, keeping the back stack intact, and survives tab switches. Router derives it from each deep link via `sidebarSection`. Consuming the payload clears `messagesState`, which re-fires onChange with nil; a guard makes that a no-op, so there is no re-entrancy and a later tab re-appear can't re-resolve the deep link and snap the user back into the notified conversation. Also fixes a sibling-selection bug: a DM notification arriving while a channel was open showed the channel thread (the detail pane prioritizes channelSelection); each branch now clears its sibling. Channel deep links that arrive before SwiftData loads the channels keep their payload to retry on the next appear instead of silently dropping. Adds Router-level coverage for messagesSection derivation, reset, and restoration, plus sidebarSection unit tests.
1 parent 8ecdc06 commit 1fbf975

5 files changed

Lines changed: 173 additions & 17 deletions

File tree

Meshtastic/Router/NavigationState.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,23 @@ enum MessagesNavigationState: Hashable {
1111
userNum: Int64? = nil,
1212
messageId: Int64? = nil
1313
)
14+
15+
/// The Messages sidebar section this state belongs to, with the deep-link payload
16+
/// (channelId/messageId, userNum) stripped.
17+
///
18+
/// The sidebar `List` only offers `.channels()` and `.directMessages()` rows, so its selection
19+
/// (`Router.messagesSection`) must stay payload-free — a payload-carrying value such as
20+
/// `.channels(channelId: 2, messageId: 5)` matches no row and corrupts the collapsed
21+
/// `NavigationSplitView` back stack (tap back → empty channel list). `Router` derives
22+
/// `messagesSection` from each deep-link `messagesState` via this property, keeping the correct
23+
/// row selected while the full payload stays on `messagesState` for the Messages view to
24+
/// consume once.
25+
var sidebarSection: MessagesNavigationState {
26+
switch self {
27+
case .channels: return .channels()
28+
case .directMessages: return .directMessages()
29+
}
30+
}
1431
}
1532

1633
// MARK: Map

Meshtastic/Router/Router.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,21 @@ class Router: ObservableObject {
99
@Published
1010
var selectedTab: NavigationState.Tab
1111

12+
/// Transient deep-link *command* for the Messages tab (carries the channelId/userNum payload).
13+
/// Set by `route(url:)` / restoration and consumed exactly once by the Messages view, which
14+
/// resolves it into a selected conversation and then clears it. Kept separate from
15+
/// `messagesSection` so the one-shot payload never reaches the sidebar selection and so a later
16+
/// tab re-appear can't re-resolve it and snap the user back into the notified conversation.
1217
@Published
1318
var messagesState: MessagesNavigationState?
1419

20+
/// Durable, payload-free Messages sidebar/section selection (`.channels()` / `.directMessages()`
21+
/// / nil). Drives the sidebar `List` selection and the content column. Because it only ever
22+
/// holds payload-free values it always matches a sidebar row, keeping the collapsed
23+
/// `NavigationSplitView` back stack intact. Survives across tab switches; reset by `popToRoot`.
24+
@Published
25+
var messagesSection: MessagesNavigationState?
26+
1527
@Published
1628
var mapState: MapNavigationState?
1729

@@ -36,6 +48,7 @@ class Router: ObservableObject {
3648
set {
3749
selectedTab = newValue.selectedTab
3850
messagesState = newValue.messages
51+
messagesSection = newValue.messages?.sidebarSection
3952
selectedNodeNum = newValue.nodeListSelectedNodeNum
4053
mapState = newValue.map
4154
if let setting = newValue.settings {
@@ -89,6 +102,7 @@ class Router: ObservableObject {
89102
) {
90103
self.selectedTab = navigationState.selectedTab
91104
self.messagesState = navigationState.messages
105+
self.messagesSection = navigationState.messages?.sidebarSection
92106
if let num = navigationState.nodeListSelectedNodeNum {
93107
self.selectedNodeNum = num
94108
}
@@ -154,6 +168,7 @@ class Router: ObservableObject {
154168
}
155169
selectedTab = .messages
156170
messagesState = state
171+
messagesSection = state?.sidebarSection
157172
}
158173

159174
private func routeNodes(_ components: URLComponents) {
@@ -180,6 +195,7 @@ class Router: ObservableObject {
180195
switch tab {
181196
case .messages:
182197
messagesState = nil
198+
messagesSection = nil
183199
case .nodes:
184200
selectedNodeNum = nil
185201
case .map:

Meshtastic/Views/Messages/Messages.swift

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,25 @@ struct Messages: View {
4242
Binding(get: { self.node }, set: { self.nodeNum = $0?.num })
4343
}
4444

45+
/// Sidebar selection binding. Reads/writes the durable, payload-free `router.messagesSection`,
46+
/// so the bound value always matches a `.channels()` / `.directMessages()` row and the
47+
/// collapsed `NavigationSplitView` back stack stays intact. The setter only fires on a user tap
48+
/// (selecting a different section), where we also reset the detail pane so the new section
49+
/// starts with nothing selected — matching the behavior of a fresh sidebar navigation.
50+
private var sidebarSelection: Binding<MessagesNavigationState?> {
51+
Binding(
52+
get: { self.router.messagesSection },
53+
set: { newValue in
54+
self.router.messagesSection = newValue
55+
self.channelSelection = nil
56+
self.userSelection = nil
57+
}
58+
)
59+
}
60+
4561
var body: some View {
4662
NavigationSplitView(columnVisibility: $columnVisibility) {
47-
List(selection: $router.messagesState) {
63+
List(selection: sidebarSelection) {
4864
NavigationLink(value: MessagesNavigationState.channels()) {
4965
Spacer()
5066
Label {
@@ -97,7 +113,7 @@ struct Messages: View {
97113
}
98114
}
99115
} content: {
100-
switch router.messagesState {
116+
switch router.messagesSection {
101117
case .channels:
102118
ChannelList(node: nodeBinding, channelSelection: $channelSelection)
103119
// Removed navigationTitle and navigationBarTitleDisplayMode here.
@@ -115,9 +131,9 @@ struct Messages: View {
115131
ChannelMessageList(myInfo: myInfo, channel: channelSelection)
116132
} else if let userSelection {
117133
UserMessageList(user: userSelection)
118-
} else if case .channels = router.messagesState {
134+
} else if case .channels = router.messagesSection {
119135
Text("Select a channel")
120-
} else if case .directMessages = router.messagesState {
136+
} else if case .directMessages = router.messagesSection {
121137
Text("Select a conversation")
122138
}
123139
}
@@ -128,39 +144,66 @@ struct Messages: View {
128144
}
129145
}
130146
}.onAppear {
131-
setupNavigationState()
132-
}.onChange(of: router.messagesState) {
133-
setupNavigationState()
147+
// Handles the deep link set by `route(url:)` before this view began observing.
148+
consumeDeepLink(router.messagesState)
149+
}.onChange(of: router.messagesState) { _, newValue in
150+
consumeDeepLink(newValue)
151+
}.onChange(of: router.messagesSection) { _, newValue in
152+
// A reset (e.g. `popToRoot` on disconnect) nils the section; clear the detail pane to
153+
// match. Section *changes* between .channels/.directMessages are reset by the sidebar
154+
// setter and by `consumeDeepLink`, so only the nil case needs handling here.
155+
if newValue == nil {
156+
channelSelection = nil
157+
userSelection = nil
158+
}
134159
}
135160
}
136161

137-
private func setupNavigationState() {
162+
private func bootstrapNodeNum() {
138163
let nodeId = Int64(UserDefaults.preferredPeripheralNum)
139164
if nodeId > 0 && nodeNum == nil {
140165
nodeNum = nodeId
141166
}
167+
}
142168

143-
guard let state = router.messagesState else {
144-
channelSelection = nil
145-
userSelection = nil
146-
return
147-
}
169+
/// Resolves a deep-link `messagesState` into the selected conversation, then clears the payload
170+
/// so it's consumed exactly once. Clearing `router.messagesState` re-fires the `onChange` above
171+
/// with `nil`, which this guard turns into a no-op — selections are never clobbered. If the
172+
/// target can't be resolved yet (e.g. channels not loaded on a cold launch) the payload is left
173+
/// in place so a later `onAppear` can retry instead of silently dropping the deep link.
174+
private func consumeDeepLink(_ state: MessagesNavigationState?) {
175+
bootstrapNodeNum()
176+
guard let state else { return }
148177

149178
switch state {
150179
case .channels(channelId: let channelId, messageId: _):
151-
if let channelId {
152-
channelSelection = node?.myInfo?.channels.first { $0.id == channelId }
153-
} else {
180+
router.messagesSection = .channels()
181+
guard let channelId else {
154182
channelSelection = nil
155183
userSelection = nil
184+
break
185+
}
186+
guard let channel = node?.myInfo?.channels.first(where: { $0.id == channelId }) else {
187+
return // Not resolvable yet — keep the payload and retry on the next appear.
156188
}
189+
channelSelection = channel
190+
// Clear the sibling DM selection so the detail pane (which prioritizes
191+
// channelSelection) can't surface a stale conversation under the Channels section.
192+
userSelection = nil
157193
case .directMessages(userNum: let userNum, messageId: _):
194+
router.messagesSection = .directMessages()
158195
if let userNum {
196+
// getUser always resolves (creating a placeholder if needed), so a DM deep link
197+
// never needs the cold-launch retry that the channel branch does.
159198
userSelection = getUser(id: userNum, context: context)
160199
} else {
161-
channelSelection = nil
162200
userSelection = nil
163201
}
202+
// Clear the sibling channel selection: the detail pane prioritizes channelSelection,
203+
// so a previously-open channel would otherwise mask this DM and show the wrong thread.
204+
channelSelection = nil
164205
}
206+
207+
router.messagesState = nil // Consumed — prevents a re-appear from re-resolving it.
165208
}
166209
}

MeshtasticTests/NavigationStateTests.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,32 @@ struct MessagesNavigationStateTests {
9494
let b = MessagesNavigationState.channels(channelId: 1)
9595
#expect(a.hashValue == b.hashValue)
9696
}
97+
98+
// MARK: sidebarSection
99+
100+
@Test func sidebarSection_channels_stripsPayload() {
101+
// A notification deep link carries channelId/messageId; the sidebar only has a
102+
// payload-free `.channels()` row, so the section must normalize to match it.
103+
#expect(MessagesNavigationState.channels(channelId: 2, messageId: 5).sidebarSection == .channels())
104+
#expect(MessagesNavigationState.channels(channelId: 7).sidebarSection == .channels())
105+
}
106+
107+
@Test func sidebarSection_directMessages_stripsPayload() {
108+
#expect(MessagesNavigationState.directMessages(userNum: 42, messageId: 9).sidebarSection == .directMessages())
109+
#expect(MessagesNavigationState.directMessages(userNum: 13).sidebarSection == .directMessages())
110+
}
111+
112+
@Test func sidebarSection_payloadFree_isIdentity() {
113+
// A manual sidebar tap already produces a payload-free value; normalizing is a no-op.
114+
#expect(MessagesNavigationState.channels().sidebarSection == .channels())
115+
#expect(MessagesNavigationState.directMessages().sidebarSection == .directMessages())
116+
}
117+
118+
@Test func sidebarSection_preservesSection() {
119+
// Normalization must not cross sections (channels must never become directMessages).
120+
#expect(MessagesNavigationState.channels(channelId: 1).sidebarSection != .directMessages())
121+
#expect(MessagesNavigationState.directMessages(userNum: 1).sidebarSection != .channels())
122+
}
97123
}
98124

99125
// MARK: - MapNavigationState

MeshtasticTests/RouterDeepLinkTests.swift

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,60 @@ struct RouterURLRoutingTests {
5252
#expect(router.messagesState == nil)
5353
}
5454

55+
// MARK: - messagesSection (durable, payload-free sidebar selection)
56+
57+
@Test @MainActor func route_messages_channelId_setsPayloadFreeSection() {
58+
let router = Router()
59+
router.route(url: URL(string: "meshtastic:///messages?channelId=5&messageId=99")!)
60+
// The deep-link payload lives on messagesState; the sidebar section is the payload-free
61+
// sibling so it matches a real sidebar row.
62+
#expect(router.messagesState == .channels(channelId: 5, messageId: 99))
63+
#expect(router.messagesSection == .channels())
64+
}
65+
66+
@Test @MainActor func route_messages_userNum_setsPayloadFreeSection() {
67+
let router = Router()
68+
router.route(url: URL(string: "meshtastic:///messages?userNum=42&messageId=7")!)
69+
#expect(router.messagesState == .directMessages(userNum: 42, messageId: 7))
70+
#expect(router.messagesSection == .directMessages())
71+
}
72+
73+
@Test @MainActor func route_messages_noParams_clearsSection() {
74+
let router = Router()
75+
router.route(url: URL(string: "meshtastic:///messages?channelId=5")!)
76+
router.route(url: URL(string: "meshtastic:///messages")!)
77+
#expect(router.messagesSection == nil)
78+
}
79+
80+
@Test @MainActor func popToRoot_messages_clearsStateAndSection() {
81+
let router = Router()
82+
router.route(url: URL(string: "meshtastic:///messages?channelId=5")!)
83+
#expect(router.messagesSection == .channels())
84+
router.popToRoot(tab: .messages)
85+
#expect(router.messagesState == nil)
86+
#expect(router.messagesSection == nil)
87+
}
88+
89+
@Test @MainActor func init_derivesSectionFromMessagesState() {
90+
// Restoration path: a Router built from a persisted NavigationState should expose the
91+
// payload-free section immediately, before any view consumes the deep link.
92+
let router = Router(navigationState: NavigationState(
93+
selectedTab: .messages,
94+
messages: .channels(channelId: 3, messageId: 1)
95+
))
96+
#expect(router.messagesState == .channels(channelId: 3, messageId: 1))
97+
#expect(router.messagesSection == .channels())
98+
}
99+
100+
@Test @MainActor func navigationState_setter_derivesSection() {
101+
let router = Router()
102+
router.navigationState = NavigationState(
103+
selectedTab: .messages,
104+
messages: .directMessages(userNum: 8)
105+
)
106+
#expect(router.messagesSection == .directMessages())
107+
}
108+
55109
@Test @MainActor func route_connect() {
56110
let router = Router()
57111
let url = URL(string: "meshtastic:///connect")!

0 commit comments

Comments
 (0)