-
Notifications
You must be signed in to change notification settings - Fork 105
Add like status to metadata #148
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,9 @@ ipcRenderer.on('notification', (_, metadata) => showNotification(metadata)) | |
|
||
function sendTrackMetadata(sender) { | ||
const artworkURL = getArtworkURL() | ||
sender.send('trackMetadata', { artworkURL }) | ||
const likeStatus = getLikeStatus() | ||
const trackMetadata = { artworkURL: artworkURL, isLiked: likeStatus } | ||
sender.send('trackMetadata', { trackMetadata }) | ||
} | ||
|
||
function navigate(url) { | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this selector would be less fragile this way: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think |
||
if(liked) { | ||
return true | ||
} else { | ||
return false | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This if/then block can be simplified to |
||
} | ||
|
||
const Notification = window.Notification | ||
// Disable SoundCloud's own notifications, because: | ||
// - They are not silent on macOS | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
const { TouchBar } = require('electron') | ||
|
||
const { TouchBarButton, TouchBarLabel, TouchBarSpacer } = TouchBar | ||
const MAX_TITLE_LENGTH = 38 | ||
const MAX_TITLE_LENGTH = 39 | ||
|
||
module.exports = function touchBarMenu(window, soundcloud) { | ||
const nextTrack = new TouchBarButton({ | ||
|
@@ -31,14 +31,35 @@ module.exports = function touchBarMenu(window, soundcloud) { | |
icon: `${__dirname}/res/like.png`, | ||
click: () => { | ||
soundcloud.likeUnlike() | ||
toggleLikeUnlikeIcon() | ||
} | ||
}) | ||
|
||
function toggleLikeUnlikeIcon() { | ||
if(likeUnlike.icon == `${__dirname}/res/like.png`) { | ||
likeUnlike.icon = `${__dirname}/res/liked.png` | ||
} else { | ||
likeUnlike.icon = `${__dirname}/res/like.png` | ||
} | ||
} | ||
|
||
const repost = new TouchBarButton({ | ||
icon: `${__dirname}/res/repost.png`, | ||
click: () => { | ||
soundcloud.repost() | ||
} | ||
}) | ||
|
||
const trackInfo = new TouchBarLabel() | ||
|
||
soundcloud.on('play', ({ title, subtitle }) => { | ||
soundcloud.on('play', ({ title, subtitle, trackMetadata }) => { | ||
playPause.icon = `${__dirname}/res/pause.png` | ||
trackInfo.label = formatTitle(title, subtitle) | ||
if(trackMetadata.isLiked) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for your feedback. I'm not used to open source projects/javascript/node so I am trying to learn as I go.
|
||
likeUnlike.icon = `${__dirname}/res/liked.png` | ||
} else { | ||
likeUnlike.icon = `${__dirname}/res/like.png` | ||
} | ||
}) | ||
|
||
soundcloud.on('pause', () => { | ||
|
@@ -50,6 +71,7 @@ module.exports = function touchBarMenu(window, soundcloud) { | |
playPause, | ||
nextTrack, | ||
likeUnlike, | ||
repost, | ||
new TouchBarSpacer({ size: 'flexible' }), | ||
trackInfo, | ||
new TouchBarSpacer({ size: 'flexible' }) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI
const foo = 'bar'; const baz = { foo }
results in the variablebaz
having a value{ foo: 'bar' }
therefore what we should have here is something like this: