Add like status to metadata #148
Conversation
|
Perhaps the dock menu and controls menu should reflect on whether or not the track is playing or if the track is liked. Will look into this. |
There was a problem hiding this comment.
Hey, nice work, thanks a lot for your contributions!
I left a few inline notes.
In order to help the work reviewers and maintainers (in this case me:) please consider the following in the future:
- Keep pull requests small and focused. This one contains commits from the "repost branch" which is completely unrelated and independent from touch bar like state management.
- Make sure you run
npm run eslintand address the errors before pushing commits.
Thanks a lot!
| sender.send('trackMetadata', { artworkURL }) | ||
| const likeStatus = getLikeStatus() | ||
| const trackMetadata = { artworkURL: artworkURL, isLiked: likeStatus } | ||
| sender.send('trackMetadata', { trackMetadata }) |
There was a problem hiding this comment.
FYI const foo = 'bar'; const baz = { foo } results in the variable baz having a value { foo: 'bar' } therefore what we should have here is something like this:
const artworkURL = getArtworkURL()
const isLiked = getLikeStatus()
sender.send('trackMetadata', { artworkURL, isLiked })| @@ -33,6 +35,15 @@ function getArtworkURL() { | |||
| return null | |||
| } | |||
|
|
|||
| function getLikeStatus() { | |||
| const liked = document.querySelector('.sc-button-like.playbackSoundBadge__like.sc-button-selected') | |||
There was a problem hiding this comment.
I think this selector would be less fragile this way: .playbackSoundBadge__actions .playbackSoundBadge__like.sc-button-selected. That would also make it more similar to the one used in getArtworkURL above.
| return true | ||
| } else { | ||
| return false | ||
| } |
There was a problem hiding this comment.
This if/then block can be simplified to !!liked.
| @@ -33,6 +35,15 @@ function getArtworkURL() { | |||
| return null | |||
| } | |||
|
|
|||
| function getLikeStatus() { | |||
| const liked = document.querySelector('.sc-button-like.playbackSoundBadge__like.sc-button-selected') | |||
There was a problem hiding this comment.
Do you think const selectedLikeButton would better reflect the contents of this variable?
| playPause.icon = `${__dirname}/res/pause.png` | ||
| trackInfo.label = formatTitle(title, subtitle) | ||
| if(trackMetadata.isLiked) { |
There was a problem hiding this comment.
The liked/not liked state management is fundamentally flawed here. The liked state can be changed in many other ways than hitting the like button on TouchBar: users can click (several different) like buttons on SoundCloud itself, use keyboard shortcut, menus, etc. This means the TouchBar like state will only be consistent if the user exclusively uses the TouchBar controls, which is unlikely.
What I would suggest is trying to figure out if we can trigger 'liked' events from SoundCloud (preload.js) independent from what triggered the state change and then implement the button state management in a similar fashion to play/pause management here (eg. soundcloud.on('liked')). One possible means of change detection would be using DOM mutation observers.
I know this sounds complicated but I also think it's not worth merging a half-assed implementation.
Do you think you can figure this out?
There was a problem hiding this comment.
Thank you for your feedback. I'm not used to open source projects/javascript/node so I am trying to learn as I go.
- The branch was based on the others because I was originally going to try and incorporate the reposted status into the metadata as well.
- I had no idea about the node linting, I will be sure to do that in the future.
- You bring up a great point about the liked/not liked state management. I hadn't even thought of that. This would also solve the problem of detecting the repost state. I will look into DOM mutation observers and see if I can figure it out. I think the best approach would be to look for the little pop up in the top right and filter for the text "liked" and "reposted".
This determines if a song is currently liked or not. If liked, the TouchBar button reflects this by turning orange. The button then toggles its color based on whether or not the song is liked.
I am attempting to do the same for the repost button but I am not quite sure how to determine if a song has already been reposted.