-
-
Notifications
You must be signed in to change notification settings - Fork 313
Start migration away from global top app bar #3132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of nits - looks good 👍
}, | ||
) | ||
} | ||
}*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this go away now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, removed here #3129
}, | ||
) { paddingValues -> | ||
if (node != null) { | ||
@Suppress("ViewModelForwarding") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably want to address this at some point and hoist appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't introduced by this PR. I was actually starting on decoupling screens from UiViewModel
, but I noticed some screens using UiViewModel.setTitle()
. Global app bar seemed reasonable to tackle first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a plan.
# Conflicts: # app/src/main/java/com/geeksville/mesh/ui/connections/ConnectionsScreen.kt
Removes global top app bar in favor of local ones for the following screens. A new extension function
NavDestination.hasGlobalAppBar()
was added toMain.kt
to exclude screens from global top app bar usage to support incremental migration. There are no visual changes. This should unblock #3010.ConnectionsRoutes.Connections
ContactsRoutes.Contacts
MapRoutes.Map
NodeDetailRoutes.NodeMap
NodesRoutes.Nodes
NodesRoutes.NodeDetail
SettingsRoutes.Settings
Screen_recording_20250917_164852.webm