Replace item details app bar with header#124
Replace item details app bar with header#124formatBCE merged 12 commits intomusic-assistant:mainfrom
Conversation
seadowg
changed the title
| import kotlin.test.assertEquals | ||
|
|
||
| @RunWith(AndroidJUnit4::class) | ||
| class ItemDetailsTest { |
There was a problem hiding this comment.
This is by no means exhaustive, but I wanted to backfill at a test harness for at least the most critical parts before moving things around.
| ) : ViewModel() { | ||
|
|
||
| data class State( | ||
| val connectionState: SessionState, |
There was a problem hiding this comment.
We don't need that anymore?
|
|
||
| private val _state = MutableStateFlow( | ||
| State( | ||
| connectionState = SessionState.Disconnected.Initial, |
There was a problem hiding this comment.
This actually wasn't being used so it was helpful to just remove it for setting up test/preview state.
| ) | ||
|
|
||
| if (onToggleViewMode != null) { | ||
| IconButton(onClick = onToggleViewMode) { |
There was a problem hiding this comment.
I wasn't quite sure where this button should live in the new layout. Personally, I'd happily get rid of it and just display a grid or row dependent on type for the moment.
One interesting side effect of showing it here is that you get a grid/row selector in multiple places on artist screens (for albums and for tracks). This is a little weird because toggling on toggles the other, but eventually I think it would make sense to store a different layout mode for each type (rather than a global one). That's assuming we are going to keep the button!
There was a problem hiding this comment.
I would still leave the button - functionality is already there, and more choice for users is always better.
Probably worth making it smaller and putting somewhere on top-right? So it doesn't mess with main layout much.
Definitely no sense displaying it separately for albums and tracks. :) it's global, at least for now.
There was a problem hiding this comment.
We'd planned to put a top right overflow menu in for non-play actions though. We could just add it in there? Not quite as natural, but at least it's still there for people who want it.
There was a problem hiding this comment.
Yes, upon adding three-dot menu we can keep it there.
2 participants
Work towards #108
This reworks the item details UI:
There are a few things to do before closing off the issue (discussed there), but this felt like a good point to get feedback and merge in.