benwiley4000 / cassette

📼 A flexible media player component library for React that requires no up-front config
https://benwiley4000.github.io/cassette/styleguide
MIT License
185 stars 28 forks source link

Add media title attribute #344

Closed danielr18 closed 5 years ago

danielr18 commented 5 years ago

In order to display the title of what you are playing in iOS notification center / lock screen, the media element needs to have a title attribute.

See: https://stackoverflow.com/questions/28303376/is-it-possible-to-change-the-ios-8-lock-screen-audio-label-when-playing-from-web

The code is done in the same way as for the poster attribute, and considering that doesn't have any documentation (that I could find in the repo), I didn't document this either.

benwiley4000 commented 5 years ago

Thanks for this! I didn't know that iOS supported system level media controls for HTML at all - I knew it didn't support the MediaSession API so I assumed it wasn't a thing.

I'm sorry to make this complicated but I noticed this getTitleForTrack prop seems to have a super similar role to the getDisplayText prop in MediaControls from the @cassette/player package. I think it's ok to have two separate (optional) props but they should both have the same default behavior. Here's what I think we should do before we merge:

  1. Take the getDisplayText internal util from @cassette/player and move it into @cassette/core as an undocumented export. Update MediaControls, MediaProgress and MediaPlayerControls to import the getDisplayText util from the @cassette/core package.
  2. Use getDisplayText as the default value for the getTitleForTrack prop.
  3. Maybe getTitleForTrack should be getMediaTitleAttributeForTrack - kind of verbose but makes it clear what exactly this is.

If you feel like doing that feel free.. otherwise I can get to it when I have some time.

BTW I should document getPosterImageForTrack - I just added that recently and forgot to document it at the time. Likewise, I think we should document this new getTitleForTrack prop.

P.S. I also just realized we probably have a bug where the user-supplied getDisplayText prop for the player is only used for the div title attribute and not for the displayed text in the status bar. That's not really related to your issue I think, so I can write a separate issue.

benwiley4000 commented 5 years ago

@danielr18 I might make those changes tonight, unless you're already working on it.

benwiley4000 commented 5 years ago

Thanks again! This is published in v2.0.0-alpha.21. As noted: