fix(nav): remote admin nodenum + Nav3 consolidation and improvements#5478
Merged
Conversation
The getRadioConfigViewModel function relied solely on searching the NavBackStack for SettingsRoute.Settings to extract destNum. In a multi-backstack setup, the captured backStack reference passed to settingsGraph() may not reflect the correct backstack when the entry composable renders (e.g., navigating to settings from the nodes tab via remote admin). This caused destNum to resolve to null, falling back to local node config. Fix: accept an explicit destNumOverride parameter in getRadioConfigViewModel and pass the route's destNum directly from entry<SettingsRoute.Settings> and entry<SettingsRoute.SettingsGraph> args. Sub-routes continue to use the backstack search as fallback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the separate SettingsGraph route type and make Settings implement Graph directly. The back button is now derived from stack position (isTabRoot check) rather than having two distinct route types. This simplifies the navigation architecture by: - Eliminating dual-type backstack search in getRadioConfigViewModel - Reducing the Settings tab to a single entry point - Making deep links and tab navigation use the same route type Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove ConnectionsGraph, ChannelsGraph, NodesGraph, and ContactsGraph route types. Their primary routes (Connections, Channels, Nodes, Contacts) now implement Graph directly and derive back button visibility from stack position (isTabRoot check). This matches the pattern established in the Settings consolidation and eliminates redundant types that existed solely to distinguish tab-root from pushed-from-another-tab navigation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add predictivePopTransitionSpec for gesture-driven back animation - Wrap backStack mutation lambdas in dropUnlessResumed to prevent double-navigation during exit transitions - Persist currentTabRoute across process death using rememberSaveable with a custom Saver (CurrentTabSaver) - Remove redundant NodeDetailGraph route; deep links now push [Nodes, NodeDetail(destNum)] directly - Add SharedTransitionLayout wrapper to enable future shared element transitions between navigation scenes - Update detekt baselines for route type renames Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remote admin settings was always showing the locally connected node's config because
destNumwas not reliably extracted from the navigation backstack. This PR fixes that root cause, then consolidates redundant navigation route types and implements several Nav3 best-practice improvements identified in a thorough audit against official patterns.Changes
Remote admin fix:
destNumOverrideparameter togetRadioConfigViewModel(), passingargs.destNumdirectly from route args instead of searching the backstack for a parent entryNavigation graph consolidation:
*Graphroute types (SettingsGraph,ConnectionsGraph,ChannelsGraph,NodesGraph,ContactsGraph,NodeDetailGraph)Graphdirectly; back button visibility derived from stack position (isTabRootcheck)[Nodes, NodeDetail(destNum)]instead of[Nodes, NodeDetailGraph(destNum), NodeDetail(destNum)]Nav3 audit improvements:
predictivePopTransitionSpecfor gesture-driven back animation supportbackStackmutation lambdas indropUnlessResumedto prevent double-navigation during exit transitionscurrentTabRouteacross process death usingrememberSaveablewith a customCurrentTabSaverSharedTransitionLayoutwrapper to enable future shared element transitionsTesting
spotlessApply detekt assembleDebug test allTestsMultiBackstackconstructor