Unwrap MainDesign#138
Conversation
b035743 to
f68765a
Compare
f68765a to
7265d36
Compare
There was a problem hiding this comment.
Pull request overview
This PR follows up on #119 by removing the legacy MainDesign wrapper and splitting the main screen into a pure @Composable UI (MainScreen) backed by a new MainViewModel, with navigation handled by MainActivity.
Changes:
- Introduces
MainViewModelto own main-screen state, events, and polling logic. - Refactors
MainScreeninto a ViewModel-driven Compose entrypoint plus a statelessMainContent. - Updates
MainActivityto usesetContent {}and wire navigation callbacks directly intoMainScreen.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/src/main/kotlin/com/github/kr328/clash/main/vm/MainViewModel.kt | New ViewModel for main screen state/event handling, service start/stop, and traffic polling. |
| app/src/main/kotlin/com/github/kr328/clash/main/ui/MainScreen.kt | Replaces MainDesign with a Compose screen that binds to MainViewModel and renders MainContent. |
| app/src/main/kotlin/com/github/kr328/clash/main/MainActivity.kt | Converts main activity to a Compose setContent host and provides navigation callbacks. |
Comments suppressed due to low confidence (1)
app/src/main/kotlin/com/github/kr328/clash/main/ui/MainScreen.kt:118
- Inside
LaunchedEffect(eventState)the snackbar calls are wrapped inscope.launch { ... }, butLaunchedEffectalready runs in a coroutine. Launching a second coroutine meansconsumeEvent()runs immediately and the snackbar work becomes fire-and-forget, which can lead to multiple snackbars overlapping/out-of-order. Prefer callingsnackbarHostState.showSnackbar(...)directly in theLaunchedEffectblock and only consuming the event after the snackbar/action handling completes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
app/src/main/kotlin/com/github/kr328/clash/main/ui/MainScreen.kt:116
- Inside
LaunchedEffect(eventState), the snackbar calls are wrapped inscope.launch { ... }. Those launched coroutines aren’t cancelled if theLaunchedEffectis restarted (e.g., event changes quickly), which can lead to snackbars/actions running for stale events and adds avoidable coroutine nesting. SinceLaunchedEffectis already a coroutine scope, callsnackbarHostState.showSnackbar(...)directly in thewhenbranch and handle the result there.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
app/src/main/kotlin/com/github/kr328/clash/main/ui/MainScreen.kt:140
- Because the app uses edge-to-edge (
BaseActivity.enableEdgeToEdge()), placingSnackbarHostatAlignment.BottomCenterwithout anyWindowInsetspadding can cause snackbars to be drawn under the navigation/gesture bar on some devices. ApplyWindowInsets.safeDrawing(or equivalent bottom inset padding) to the snackbar host/container so it always appears above system bars.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Refs #119.