Multiple media players per widget #2534
Conversation
jannis3005
changed the title
jannis3005
force-pushed
the
multiple-media-players-pwer-widget
branch
from
f6615de
to
ecedf21
Compare
There was a problem hiding this comment.
Interesting idea! I've added some code related comments.
GitHub doesn't allow me to comment on an image, but I'm seeing a pink tint on the bottom left?
I don't think the UI is very intuitive though. Have you considered changing it to make it more obvious you can select multiple entities (for example, another label "Entity ID(s)" or even using something like a dialog instead of a single text input)?
common/src/main/java/io/homeassistant/companion/android/database/AppDatabase.kt
Show resolved
Hide resolved
...ompanion/android/widgets/media_player_controls/MediaPlayerControlsWidgetConfigureActivity.kt
Outdated
Show resolved
Hide resolved
...o/homeassistant/companion/android/widgets/media_player_controls/MediaPlayerControlsWidget.kt
Show resolved
Hide resolved
...o/homeassistant/companion/android/widgets/media_player_controls/MediaPlayerControlsWidget.kt
Outdated
Show resolved
Hide resolved
...n/java/io/homeassistant/companion/android/database/widget/MediaPlayerControlsWidgetEntity.kt
Outdated
Show resolved
Hide resolved
…call's when finding active media player. add db migrations. add text color to media source label. set default value of showsource to false.
|
I think that should be all of the concerns adressed :) here's a screenshot of ne new preview (i guess i missed that when creating the pr)
|
...o/homeassistant/companion/android/widgets/media_player_controls/MediaPlayerControlsWidget.kt
Show resolved
Hide resolved
|
@jpelgrom anything else that needs to be addressed/should be changed? :) |
| if (artist != null) { | ||
| label = artist | ||
| } | ||
| if (artist == null && title != null) { | ||
| label = title | ||
| } | ||
| if (artist == null && title == null && album != null) { | ||
| label = album | ||
| } |
There was a problem hiding this comment.
Why not update the label using a when block? Because it will use the first match, that would allow removing all the == null checks.
label = when {
artist != null -> artist
title != null -> title
album != null -> album
else -> label
}| val se = binding.widgetTextConfigEntityId.text.split(",") | ||
| se.forEach { | ||
| val e = entities[it.trim()] | ||
| if (e != null) selectedEntities.add(e) | ||
| } | ||
|
|
||
| intent.putExtra( | ||
| MediaPlayerControlsWidget.EXTRA_ENTITY_ID, | ||
| selectedEntity!!.entityId | ||
| selectedEntities.map { e -> e?.entityId }.reduce { a, b -> "$a,$b" } |
There was a problem hiding this comment.
Renaming the parameter inside map isn't necessary, why not use it?
| selectedEntities = LinkedList() | ||
| val se = binding.widgetTextConfigEntityId.text.split(",") | ||
| se.forEach { | ||
| val e = entities[it.trim()] | ||
| if (e != null) selectedEntities.add(e) | ||
| } |
There was a problem hiding this comment.
I think a filter is better than a loop, but that's probably a personal preference.
selectedEntities = LinkedList(
binding.widgetTextConfigEntityId.text.split(",")
.mapNotNull { entities[it.trim()] }
)| try { | ||
| integrationUseCase.getEntity(mediaPlayerWidget.entityId) | ||
| mediaPlayerWidget.entityId.split(",").map { s -> integrationUseCase.getEntity(s.trim()) } |
There was a problem hiding this comment.
Renaming the parameter inside map isn't necessary, why not use it?
Summary
I've added the option to specify multiple media players per widget, so that one can control more than one media player through one widget. They are prioritized from left to right (meaning the widget shows the first media player from the left that has a state of playing. if nothing is playing, it shows the first player that is specified in the Entity ID Field.
In addition that, to make it more clear what the user is controlling, i've added a media source lable to the widget and an option to toggle it.
I've also updated the preview image of the media player widget to match the new design.
Screenshots
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#749
Any other notes