Feat/snapserver group UI #57
Conversation
guerman5
left a comment
guerman5
left a comment
There was a problem hiding this comment.
This PR introduces the Snapserver group UI with sticky headers, volume controls, and better client grouping, making multi-device audio smoother. The ViewModel refactor and added tests look solid. With 35 files changed, we should keep an eye on edge cases.
| NowPlayingScreenContent( | ||
| uiState = uiState, | ||
| groups = viewModel.groups, | ||
| onClientVolumeChange = viewModel::onClientVolumeChange, |
There was a problem hiding this comment.
SnarserverGroups call shows onClientVolumeChange twice in the diff. Once here and once in Line115. Make sure final code only passes this parameter once.
There was a problem hiding this comment.
it may appear as being passed twice but in reality is being hoisted by the topmost NowPlayingScreen which in turns calls the function on the viewModel, making it the final point of action
|
|
||
| @Preview(showBackground = true) | ||
| @Composable | ||
| fun NowPlayingScreenContentPreview() { |
There was a problem hiding this comment.
The mock group/client construction is duplicated between Broadcaster and NowPlaying previews; you could extract a small helper to avoid repeating that structure, but that’s a minor nit.
There was a problem hiding this comment.
Minor NIT agree.
garpernica
left a comment
garpernica
left a comment
There was a problem hiding this comment.
Nice improvements, NowPlaying has its own ViewModel + UI state more testable and preview-friendly. Snapserver used from both Broadcaster and NowPlaying reduces duplication and keeps the Snapserver group UI consistent.
Changes
NowPlayingScreenviewModel, being in charge of audio channel selection and snapcast control interaction