From 620e329521a6b9059a8b0f603cc7d4a5c835f26a Mon Sep 17 00:00:00 2001 From: Blake McAnally Date: Wed, 28 Aug 2024 07:02:55 -0500 Subject: [PATCH] Refactor router state to more closely match tab bar behavior --- Meshtastic/Router/NavigationState.swift | 47 +------ Meshtastic/Router/Router.swift | 30 +++-- Meshtastic/Views/ContentView.swift | 9 +- Meshtastic/Views/Messages/Messages.swift | 34 ++--- Meshtastic/Views/Nodes/MeshMap.swift | 2 +- Meshtastic/Views/Nodes/NodeList.swift | 7 +- Meshtastic/Views/Settings/Settings.swift | 7 +- MeshtasticTests/RouterTests.swift | 151 ++++++++++++++++++----- 8 files changed, 158 insertions(+), 129 deletions(-) diff --git a/Meshtastic/Router/NavigationState.swift b/Meshtastic/Router/NavigationState.swift index 0ff61108..0a85f4ab 100644 --- a/Meshtastic/Router/NavigationState.swift +++ b/Meshtastic/Router/NavigationState.swift @@ -55,17 +55,7 @@ enum SettingsNavigationState: String { case firmwareUpdates } -enum NavigationState: Hashable { - case messages(MessagesNavigationState? = nil) - case bluetooth - case nodes(selectedNodeNum: Int64? = nil) - case map(MapNavigationState? = nil) - case settings(SettingsNavigationState? = nil) -} - -// MARK: Tab Bar - -extension NavigationState { +struct NavigationState: Hashable { enum Tab: String, Hashable { case messages case bluetooth @@ -74,34 +64,9 @@ extension NavigationState { case settings } - var tab: Tab { - get { - switch self { - case .messages: - .messages - case .bluetooth: - .bluetooth - case .nodes: - .nodes - case .map: - .map - case .settings: - .settings - } - } - set { - self = switch newValue { - case .messages: - .messages() - case .bluetooth: - .bluetooth - case .nodes: - .nodes() - case .map: - .map() - case .settings: - .settings() - } - } - } + var selectedTab: Tab = .bluetooth + var messages: MessagesNavigationState? + var nodeListSelectedNodeNum: Int64? + var map: MapNavigationState? + var settings: SettingsNavigationState? } diff --git a/Meshtastic/Router/Router.swift b/Meshtastic/Router/Router.swift index 51803ed4..718c71b1 100644 --- a/Meshtastic/Router/Router.swift +++ b/Meshtastic/Router/Router.swift @@ -12,7 +12,9 @@ class Router: ObservableObject { private var cancellables: Set = [] init( - navigationState: NavigationState = .bluetooth + navigationState: NavigationState = NavigationState( + selectedTab: .bluetooth + ) ) { self.navigationState = navigationState @@ -21,10 +23,6 @@ class Router: ObservableObject { }.store(in: &cancellables) } - func route(to destination: NavigationState) { - navigationState = destination - } - func route(url: URL) { guard url.scheme == "meshtastic" else { Logger.services.error("🛣 Received routing URL \(url, privacy: .public) with invalid scheme. Ignoring route.") @@ -38,7 +36,7 @@ class Router: ObservableObject { if components.path == "/messages" { routeMessages(components) } else if components.path == "/bluetooth" { - route(to: .bluetooth) + navigationState.selectedTab = .bluetooth } else if components.path == "/nodes" { routeNodes(components) } else if components.path == "/map" { @@ -75,7 +73,8 @@ class Router: ObservableObject { } else { nil } - route(to: .messages(state)) + navigationState.selectedTab = .messages + navigationState.messages = state } private func routeNodes(_ components: URLComponents) { @@ -83,7 +82,9 @@ class Router: ObservableObject { .first(where: { $0.name == "nodenum" })? .value .flatMap(Int64.init) - route(to: .nodes(selectedNodeNum: nodeId)) + + navigationState.selectedTab = .nodes + navigationState.nodeListSelectedNodeNum = nodeId } private func routeMap(_ components: URLComponents) { @@ -95,12 +96,14 @@ class Router: ObservableObject { .first(where: { $0.name == "waypointId" })? .value .flatMap(Int64.init) - if let nodeId { - route(to: .map(.selectedNode(nodeId))) + + navigationState.selectedTab = .map + navigationState.map = if let nodeId { + .selectedNode(nodeId) } else if let waypointId { - route(to: .map(.waypoint(waypointId))) + .waypoint(waypointId) } else { - route(to: .map()) + nil } } @@ -112,6 +115,7 @@ class Router: ObservableObject { .flatMap(String.init) .flatMap(SettingsNavigationState.init(rawValue:)) - route(to: .settings(settingFromPath)) + navigationState.selectedTab = .settings + navigationState.settings = settingFromPath } } diff --git a/Meshtastic/Views/ContentView.swift b/Meshtastic/Views/ContentView.swift index e8384c35..23b25f0c 100644 --- a/Meshtastic/Views/ContentView.swift +++ b/Meshtastic/Views/ContentView.swift @@ -13,14 +13,7 @@ struct ContentView: View { var router: Router var body: some View { - TabView(selection: Binding( - get: { - appState.router.navigationState.tab - }, - set: { newValue in - appState.router.navigationState.tab = newValue - } - )) { + TabView(selection: $appState.router.navigationState.selectedTab) { Messages( router: appState.router, unreadChannelMessages: $appState.unreadChannelMessages, diff --git a/Meshtastic/Views/Messages/Messages.swift b/Meshtastic/Views/Messages/Messages.swift index 88a6f2a2..09b8d1af 100644 --- a/Meshtastic/Views/Messages/Messages.swift +++ b/Meshtastic/Views/Messages/Messages.swift @@ -26,21 +26,6 @@ struct Messages: View { @Binding var unreadDirectMessages: Int - // Aliases the navigation state for the NavigationSplitView sidebar selection - private var messagesSelection: Binding { - Binding( - get: { - guard case .messages(let state) = router.navigationState else { - return nil - } - return state - }, - set: { newValue in - router.navigationState = .messages(newValue) - } - ) - } - @State var node: NodeInfoEntity? @State private var userSelection: UserEntity? // Nothing selected by default. @State private var channelSelection: ChannelEntity? // Nothing selected by default. @@ -49,7 +34,7 @@ struct Messages: View { var body: some View { NavigationSplitView(columnVisibility: $columnVisibility) { - List(selection: messagesSelection) { + List(selection: $router.navigationState.messages) { NavigationLink(value: MessagesNavigationState.channels()) { Label { Text("channels") @@ -88,11 +73,12 @@ struct Messages: View { .navigationBarTitleDisplayMode(.large) .navigationBarItems(leading: MeshtasticLogo()) } content: { - if case .messages(.channels) = router.navigationState { + switch router.navigationState.messages { + case .channels(let channelId, let messageId): ChannelList(node: $node, channelSelection: $channelSelection) - } else if case .messages(.directMessages) = router.navigationState { + case .directMessages(let userNum, let messageId): UserList(node: $node, userSelection: $userSelection) - } else if case .messages(nil) = router.navigationState { + case nil: Text("Select a conversation type") } } detail: { @@ -100,9 +86,9 @@ struct Messages: View { ChannelMessageList(myInfo: myInfo, channel: channelSelection) } else if let userSelection { UserMessageList(user: userSelection) - } else if case .messages(.channels) = router.navigationState { + } else if case .channels = router.navigationState.messages { Text("Select a channel") - } else if case .messages(.directMessages) = router.navigationState { + } else if case .directMessages = router.navigationState.messages { Text("Select a conversation") } }.onChange(of: router.navigationState) { _ in @@ -116,11 +102,7 @@ struct Messages: View { node = getNodeInfo(id: nodeId, context: context) } - guard case .messages(let state) = router.navigationState else { - return - } - - guard let state else { + guard let state = router.navigationState.messages else { channelSelection = nil userSelection = nil return diff --git a/Meshtastic/Views/Nodes/MeshMap.swift b/Meshtastic/Views/Nodes/MeshMap.swift index f471caaf..595abb15 100644 --- a/Meshtastic/Views/Nodes/MeshMap.swift +++ b/Meshtastic/Views/Nodes/MeshMap.swift @@ -123,7 +123,7 @@ struct MeshMap: View { MapSettingsForm(traffic: $showTraffic, pointsOfInterest: $showPointsOfInterest, mapLayer: $selectedMapLayer, meshMap: $isMeshMap) } .onChange(of: router.navigationState) { - guard case .map(let selectedNodeNum) = router.navigationState else { return } + guard case .map = router.navigationState.selectedTab else { return } // TODO: handle deep link for waypoints } .onChange(of: selectedMapLayer) { newMapLayer in diff --git a/Meshtastic/Views/Nodes/NodeList.swift b/Meshtastic/Views/Nodes/NodeList.swift index c0f16dd8..7df0283c 100644 --- a/Meshtastic/Views/Nodes/NodeList.swift +++ b/Meshtastic/Views/Nodes/NodeList.swift @@ -335,11 +335,8 @@ struct NodeList: View { } } .onChange(of: router.navigationState) { _ in - // Handle deep link routing - if case .nodes(let selected) = router.navigationState { - self.selectedNode = selected.flatMap { - getNodeInfo(id: $0, context: context) - } + if let selected = router.navigationState.nodeListSelectedNodeNum { + self.selectedNode = getNodeInfo(id: selected, context: context) } else { self.selectedNode = nil } diff --git a/Meshtastic/Views/Settings/Settings.swift b/Meshtastic/Views/Settings/Settings.swift index 814f3ab9..e88816ab 100644 --- a/Meshtastic/Views/Settings/Settings.swift +++ b/Meshtastic/Views/Settings/Settings.swift @@ -299,13 +299,10 @@ struct Settings: View { NavigationStack( path: Binding<[SettingsNavigationState]>( get: { - guard case .settings(let route) = router.navigationState, let setting = route else { - return [] - } - return [setting] + [router.navigationState.settings].compactMap { $0 } }, set: { newPath in - router.navigationState = .settings(newPath.first) + router.navigationState.settings = newPath.first } ) ) { diff --git a/MeshtasticTests/RouterTests.swift b/MeshtasticTests/RouterTests.swift index 81b07276..a3ecee6d 100644 --- a/MeshtasticTests/RouterTests.swift +++ b/MeshtasticTests/RouterTests.swift @@ -5,53 +5,144 @@ import XCTest final class RouterTests: XCTestCase { - func testInitialState() throws { - XCTAssertEqual(Router().navigationState, .bluetooth) + func testInitialState() async throws { + let router = await Router() + let tab = await router.navigationState.selectedTab + XCTAssertEqual(tab, .bluetooth) } - func testRouteTo() throws { - let router = Router(navigationState: .bluetooth) - router.route(to: .settings(.about)) - XCTAssertEqual(router.navigationState, .settings(.about)) + func testRouteMessages() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///messages", + NavigationState(selectedTab: .messages) + ) } - func testRouteURL() throws { - // Messages - try assertRoute("meshtastic:///messages", .messages()) - try assertRoute( + func testRouteMessagesWithChannelIdAndMessageId() async throws { + try await assertRoute( + router: Router(), "meshtastic:///messages?channelId=0&messageId=1122334455", - .messages(.channels(channelId: 0, messageId: 1122334455)) + NavigationState( + selectedTab: .messages, + messages: .channels( + channelId: 0, + messageId: 1122334455 + ) + ) ) - try assertRoute( + } + + func testRouteMessagesWithUserNumAndMessageId() async throws { + try await assertRoute( + router: Router(), "meshtastic:///messages?userNum=123456789&messageId=9876543210", - .messages(.directMessages(userNum: 123456789, messageId: 9876543210)) + NavigationState( + selectedTab: .messages, + messages: .directMessages( + userNum: 123456789, + messageId: 9876543210 + ) + ) ) + } - // Bluetooth - try assertRoute("meshtastic:///bluetooth", .bluetooth) + func testRouteBluetooth() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///bluetooth", + NavigationState(selectedTab: .bluetooth) + ) + } - // Nodes - try assertRoute("meshtastic:///nodes", .nodes()) - try assertRoute("meshtastic:///nodes?nodenum=1234567890", .nodes(selectedNodeNum: 1234567890)) + func testRouteNodes() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///nodes", + NavigationState(selectedTab: .nodes) + ) + } - // Map - try assertRoute("meshtastic:///map", .map()) - try assertRoute("meshtastic:///map?waypointId=123456", .map(.waypoint(123456))) - try assertRoute("meshtastic:///map?nodenum=1234567890", .map(.selectedNode(1234567890))) + func testRouteNodesWithNodeNum() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///nodes?nodenum=1234567890", + NavigationState( + selectedTab: .nodes, + nodeListSelectedNodeNum: 1234567890 + ) + ) + } - // Settings - try assertRoute("meshtastic:///settings", .settings()) - try assertRoute("meshtastic:///settings/about", .settings(.about)) - try assertRoute("meshtastic:///settings/invalidSetting", .settings()) + func testRouteMap() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///map", + NavigationState(selectedTab: .map) + ) + } + + func testRouteMapWithWaypointId() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///map?waypointId=123456", + NavigationState( + selectedTab: .map, + map: .waypoint(123456) + ) + ) + } + + func testRouteMapWithNodeNum() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///map?nodenum=1234567890", + NavigationState( + selectedTab: .map, + map: .selectedNode(1234567890) + ) + ) + } + + func testRouteSettings() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///settings", + NavigationState( + selectedTab: .settings + ) + ) + } + + func testRouteSettingsAbout() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///settings/about", + NavigationState( + selectedTab: .settings, + settings: .about + ) + ) + } + + func testRouteSettingsInvalidSetting() async throws { + try await assertRoute( + router: Router(), + "meshtastic:///settings/invalidSetting", + NavigationState( + selectedTab: .settings + ) + ) } private func assertRoute( - router: Router = Router(), + router: Router, _ urlString: String, _ destination: NavigationState - ) throws { + ) async throws { let url = try XCTUnwrap(URL(string: urlString)) - router.route(url: url) - XCTAssertEqual(router.navigationState, destination) + await router.route(url: url) + let state = await router.navigationState + XCTAssertEqual(state, destination) } }