Get album name #88
Get album name #88
Conversation
Try to get the album name and display it on the player module and on Discord as "LargeImageText"
src/scripts/discord.js
Outdated
| @@ -32,6 +32,7 @@ const observer = (event, arg) => { | |||
| startTimestamp: parseInt(now), | |||
| endTimestamp: parseInt(remaining), | |||
| largeImageKey: mediaInfoModule.mediaInfo.image, | |||
| largeImageText: (mediaInfoModule.mediaInfo.album) ? mediaInfoModule.mediaInfo.album : 'TIDAL HiFi', | |||
There was a problem hiding this comment.
According to this line it will fallback to TIDAL HiFi but I think we should keep the original line which is currently used by the app and also used by the idle status still with this change
Tidal HiFi ${app.getVersion()}
|
On another note I just tested this and noticed that it rarely ever picked up any album in the beginning until a while later where it started to pick up basically every album which is good enough. |
Thanks for the input. It's not 100% consistent, sometimes it can't get the album name for some reason, but still I think it works okay. Any idea how to improve this? |
Sadly no considering how Tidal deals with album names I think this is the best solution possible. |
There was a problem hiding this comment.
Approved with minor changes requested.
Overall it looks pretty good, as @Mar0xy already mentioned Tidal (and their attributes..) make it hard to perfectly nail down any info.
Please create a version 2.7.0 entry in the Changelog and explain that album artwork is retrieved on a "best-effort" based system (feel free to link this PR). No need to change any other version numbers, that'll be done on release anyway.
Please let me know whether you want to do them yourself or whether you want me to do them.
Thanks for the contribution!
post comment clarity edit: please include a line in the readme for album art:
- album-art in integrations ([best-effort](https://github.com/Mastermindzh/tidal-hifi/pull/88#pullrequestreview-840814847))| //If listening to an album, get its name from the header title | ||
| if(window.location.href.includes('/album/')) { | ||
| const albumName = window.document.querySelector(this.album_header_title); | ||
| if(albumName) { |
There was a problem hiding this comment.
I take it that the url either includes album or playlist? Never both?
If so, please wrap the second if in an "elseif"
src/preload.js
Outdated
|
|
||
| //If listening to a playlist or a mix, get album name from the list | ||
| if(window.location.href.includes('/playlist/') || window.location.href.includes('/mix/')) { | ||
| if(currentPlayStatus === 'playing') { |
There was a problem hiding this comment.
Please use the constants statuses.playing instead of the string 'playing'
Updated preload.js with the requested changes
|
Thank you for your review! |
|
Available in 2.7.0 Enjoy! |
Tries to get the album name and display it on the player module and on Discord as "LargeImageText".
KDE media player
Discord
Edit: Guess it fixes this issue