macos: Add a sidebar icon #5928
Conversation
In the process, use an iconset to generate the icns using iconutil. Also add some missing icon resolutions according to the guidelines. Issue owncloud#296
|
@jturcotte this is dope! 😎 Will try it later, can you post a screenshot to see how does it look for reference? |
|
I just picked the same icon we have and put it there, and this is the result:
It would be better if a graphics designer would do it but I think it's not too bad already. |
| @@ -232,7 +232,7 @@ if (NOT DEFINED APPLICATION_ICON_NAME) | |||
| set(APPLICATION_ICON_NAME ${APPLICATION_SHORTNAME}) | |||
| endif() | |||
|
|
|||
| kde4_add_app_icon( ownCloud "${theme_dir}/colored/${APPLICATION_ICON_NAME}-icon*.png") | |||
| kde4_add_app_icon( ownCloud "${theme_dir}/colored/${APPLICATION_ICON_NAME}-*.png") | |||
There was a problem hiding this comment.
The files in theme/colored still have the "-icon" in their name, any reason to change this kde4_add_app_icon thingy?
There was a problem hiding this comment.
Yes that's to be able to also get files like owncloud-sidebar-36.png in the glob. -icon is for the application icon.
There was a problem hiding this comment.
Aaah it's because of macro (KDE4_ADD_APP_ICON appsources pattern) and has nothing to do with kd4...
There was a problem hiding this comment.
your cmake-magic looks good.
Please only merge after @SamuAlfageme confirmed it builds and works from rotor.
@michaelstingl ownbrander should supply a -128 icon and also all the -sidebar- icons. (It will however not fail if those are not supplied, right @jturcotte ?)
|
@guruz @jturcotte can any of you guys change the PR's target so I can build the installers with rotor (remember the limitations to build fork's PR with it 😕) while keeping master clean 'till I do some good ol' testin'? |
|
ACK, @jturcotte please push to this repo, not your own |
|
I've been under the impression that QA is happening in master, and I think that it's not worth doing custom rotor builds on feature branches to test once, and then test again for any possible merge issue in master, but this is for the QA team to decide. It seems to be the consensus for that reason that the team should make pull request from the main repo only and I'll do it if you really think that this kind of consistency is worth the proliferating amount of stale branches, the main reason why I've been avoiding it personally. I can't retarget the pull request, so for this time I think it should be fine. I tested the build on my machine and I'm pretty confident it's going to pass on rotor as well. Even if I does break the build, that I add a commit here or on master afterwards won't change anything as long as I push the fix shortly after the merge. |
|
@jturcotte i've deleted like 10-15 branches now from that list |
|
@jturcotte great work, it works for me on 10.12.5 |
|
@jturcotte is there a way to override the icon to be the default's (i.e. the old one) in case of a branding client with no alternative choice? |
|
I think the default won't copy the icon into the bundle if it is not found in the theme. Then the OS will behave as currently
…Sent from my iPhone
On 2. Aug 2017, at 09:04, Samuel Alfageme ***@***.***> wrote:
@jturcotte is there a way to override the icon to be the default's (i.e. the old one) in case of a branding client with no alternative choice?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@michaelstingl Is there any theming documentation I could update to list needed and optional icon sizes with their name? |
|
@jturcotte we can probably ease @jvillafanez's job updating ownBrander by posting it on https://github.com/owncloud/ownbrander/issues/748 - I think there's no theming docs available |
|
@jturcotte also, would it be hard to consider a migration path for existing local sync folders to be "updated" in the finder sidebar with the new icon when upgrading the client? |
I'm working on that, there is a bug -> #5928 (comment) |
|
So cool, nice work @jturcotte! |
|
See the linked PR which is related for depending on which wizard you use. There is no need for a migration path, the Finder will automatically pick up the sidebar icon :) |
In the process, use an iconset to generate the icns using iconutil.
Also add some missing icon resolutions according to the guidelines.
Issue #296