Skip to content

Update Media Card to check for Supported Features#4850

Merged
bramkragten merged 11 commits into
home-assistant:devfrom
zsarnett:media-card-updates
Feb 17, 2020
Merged

Update Media Card to check for Supported Features#4850
bramkragten merged 11 commits into
home-assistant:devfrom
zsarnett:media-card-updates

Conversation

@zsarnett
Copy link
Copy Markdown
Contributor

@zsarnett zsarnett commented Feb 11, 2020

Type of change

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

@bramkragten
Copy link
Copy Markdown
Member

Still some big differences, mainly:

  • Play button state doesn't get disabled
  • Unavailable state -> Play button + image color
  • Size of album covers
  • The progress bar doesn't disappear when stopped playing
  • Animation on height change is missing
  • Missing now playing info
  • Long media title isn't cut off (ellipsis)

Before:
image
media-before

After:
image
media-after

@zsarnett
Copy link
Copy Markdown
Contributor Author

  • Play button state doesn't get disabled

Fixed. Disabled when the state is off

  • Unavailable state -> Play button + image color

Fixed. Play button is no longer rendered and the image is darkened when the state is unavailable

  • Size of album covers

This is something I changed to be consistent with the card size. I have made it so all the albums are the same size if the card is playing

  • The progress bar doesn't disappear when stopped playing

I disagree with the progress bar being removed if the state is paused. I would still expect the player to know my place and continue when I hit play.

  • Animation on height change is missing

Fixed. Height is animated when the image is changed

  • Missing now playing info

Fixed. This was changed the be Secondary_Info: Primary_Info when the card was converted. I have reverted this change.

  • Long media title isn't cut off (ellipsis)

Fixed. Media Title is not set to show ellipsis when longer than one row of text

@zsarnett
Copy link
Copy Markdown
Contributor Author

@balloob The demo (Arsaboo) has the image pulling from api folder. The new system tries to find that and convert to base64 but for that image, in particular, it isn't found. Resulting in a blue background

@balloob
Copy link
Copy Markdown
Member

balloob commented Feb 16, 2020

I don't understand, we shouldn't try to fetch the image unless there is one? Also why are we converting it to base64? It's already a url. If it needs auth we should use a signed url.

@zsarnett
Copy link
Copy Markdown
Contributor Author

The reason the image shows up blank before the recent change was that an entity image was supplied "api/.../image" URL. But that URL was not found and no image was returned. The previous way doesn't check to see if that image exists or not. We are only converting to base64 if the image is a local image.

@zsarnett
Copy link
Copy Markdown
Contributor Author

#4489 Also Adding this here as something that may need to be checked for. Will use this PR to update

Comment thread src/panels/lovelace/cards/hui-media-control-card.ts Outdated
Comment thread src/panels/lovelace/cards/hui-media-control-card.ts Outdated
Comment thread src/panels/lovelace/cards/hui-media-control-card.ts Outdated
Comment thread src/panels/lovelace/cards/hui-media-control-card.ts Outdated
return;
}

fetchMediaPlayerThumbnailWithCache(this.hass, this._config.entity).then(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@balloob didn't we move the thumbnails away from the ws?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the status on how we should fetch the image? Should it be reverted to using the image URL or is this the best method. This is the method it was using before the conversion.

Comment thread src/panels/lovelace/cards/hui-media-control-card.ts Outdated
Comment thread src/panels/lovelace/cards/hui-media-control-card.ts Outdated
Comment thread src/panels/lovelace/cards/hui-media-control-card.ts Outdated
Comment thread src/panels/lovelace/cards/hui-media-control-card.ts Outdated
Comment thread src/panels/lovelace/cards/hui-media-control-card.ts Outdated
@zsarnett
Copy link
Copy Markdown
Contributor Author

Fixes: #4503

@bramkragten bramkragten merged commit af3626b into home-assistant:dev Feb 17, 2020
@zsarnett zsarnett deleted the media-card-updates branch February 17, 2020 20:18
@bramkragten bramkragten mentioned this pull request Feb 19, 2020
@lock lock Bot locked and limited conversation to collaborators Feb 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Media player card doesn't check supported features

4 participants