-
Notifications
You must be signed in to change notification settings - Fork 101
Add repost button to touchbar #147
base: master
Are you sure you want to change the base?
Conversation
Perhaps we should have media controls on the left, title in the center and the repost/like options on the right? Or some variation? What are your thoughts |
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.
The new icons render much uglier than the old ones or the macOS built-in ones due to reduced resolution. Could you please restore the old icons or provide high resolution ones?
@@ -3,7 +3,7 @@ | |||
const { TouchBar } = require('electron') | |||
|
|||
const { TouchBarButton, TouchBarLabel, TouchBarSpacer } = TouchBar | |||
const MAX_TITLE_LENGTH = 38 | |||
const MAX_TITLE_LENGTH = 39 |
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.
With one more button on the TouchBar I would expect the title length to become shorter not longer.
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.
@salomvary I recreated the icons using the exact assets from Soundcloud. They are actually in SVG which you seemed to want earlier. Perhaps that would help.
I tested the title length and 39 is actually the maximum, the increase buttons seemed to have no effect. It seems although the 39 char length has more to do with the electron implementation (a bug possibly?)
I don't have a strong opinion on button placement but I noticed the space available for the title has become quite tiny. Do we actually need all these buttons sacrificing a readable title? |
@salomvary I have opened an issue with electron that is currently being resolved in order to tighten up the buttons and free up space on the TouchBar. Personally I feel as though the buttons are worth it. The keyboard (and thus the TouchBar) is more for acting rather than looking (unless you look at the keys when you type). I guess my thought process is that buttons on the touch bar are more important than displaying the title, which would already be visible in the application. |
i would love to see this PR merged. Is there anything major stopping it? Also it seems as though the touch bar isn't working at all on my 2019 16" macbook pro |
@Impakt As this PR is quite old, it will take a bit of effort to make it work again and do some testing. Any help here would be appreciated. |
i actually pulled this repo and made the changes (the files that needed changing were very similar to how they were when the PR was created) on the master branch. When soundcleod has focus though the touch bar is empty. Getting the touch bar working again would be the hard part, i think adding repost would be easy. I'm happy to test and help out as much as I can but i don't know electron or typescript that well (however been a java developer for over 20 years). Edit: I even compared your touchbar code with the sample here in case some API had changed https://www.electronjs.org/docs/api/touch-bar |
I actually just got the touchbar working! i wrapped the array of touchbar controls at |
Adds a repost button to the TouchBar. I also redid the icons, they now use the exact same assets as Soundcloud.