Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use macOS-specific application icon #4707

Merged
merged 2 commits into from
Jul 12, 2022
Merged

Use macOS-specific application icon #4707

merged 2 commits into from
Jul 12, 2022

Conversation

claucambra
Copy link
Collaborator

@claucambra claucambra commented Jul 6, 2022

Signed-off-by: Claudio Cambra [email protected]

@elsiehupp
Copy link
Member

Is this for the purpose of facilitating #4367? (As I read it this pull request is basically just clarifying that the particular SVG in question is only used for the Finder sidebar and not, say, the Nautilus sidebar, which wasn't terribly clear from the previous nomenclature.)

@claucambra
Copy link
Collaborator Author

Is this for the purpose of facilitating #4367? (As I read it this pull request is basically just clarifying that the particular SVG in question is only used for the Finder sidebar and not, say, the Nautilus sidebar, which wasn't terribly clear from the previous nomenclature.)

No, this is to set the application's icon to your Big Sur-style icon in #4631

@elsiehupp
Copy link
Member

On some level I think it would be useful to clarify in the filename that the sidebar icon is specifically used by the Finder, but it could also make sense make the nomenclature more generic, like Nextcloud-{platform}-shell-sidebar, if you wanted to facilitate slight variations for, e.g., Windows Explorer, Nautilus, Dolphin, elementary Files, etc. (How the distinction for different Linux shells would be implemented would probably be a bit more complicated than, say, the distinction between Windows and macOS.)

@elsiehupp
Copy link
Member

For instance, #2903 touches on the Windows Explorer sidebar icon, and apparently I opened #2950 to address Linux-shell sidebar icons, as well.

@elsiehupp
Copy link
Member

In the case of Linux shells that use the XDG Icon Theme Specification, it could work to assign the Nextcloud folder a filesystem attribute with the icon name and then provide the Nextcloud (or vendor) logo as icon, though I'm not 100% sure how that would work with sandboxed versions of Nextcloud Desktop such as Flatpak.

This is a bit of a digression, but again my main point here is that Nextcloud-{platform}-shell-sidebar could potentially facilitate better shell-sidebar icon support on platforms other than macOS.

@elsiehupp
Copy link
Member

Is this for the purpose of facilitating #4367? (As I read it this pull request is basically just clarifying that the particular SVG in question is only used for the Finder sidebar and not, say, the Nautilus sidebar, which wasn't terribly clear from the previous nomenclature.)

No, this is to set the application's icon to your Big Sur-style icon in #4631

Ah, okay!

@claucambra
Copy link
Collaborator Author

In the case of Linux shells that use the XDG Icon Theme Specification, it could work to assign the Nextcloud folder a filesystem attribute with the icon name and then provide the Nextcloud (or vendor) logo as icon, though I'm not 100% sure how that would work with sandboxed versions of Nextcloud Desktop such as Flatpak.

This is a bit of a digression, but again my main point here is that Nextcloud-{platform}-shell-sidebar could potentially facilitate better shell-sidebar icon support on platforms other than macOS.

I think this naming structure would make things clearer, but it would also require changes to our branding infra (which I am not super familiar with). Worth discussing though :)

NEXTCLOUD.cmake Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #4707 (e8cc369) into master (bc01db7) will increase coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4707      +/-   ##
==========================================
+ Coverage   56.41%   56.57%   +0.15%     
==========================================
  Files         138      138              
  Lines       17068    17068              
==========================================
+ Hits         9629     9656      +27     
+ Misses       7439     7412      -27     
Impacted Files Coverage Δ
src/libsync/discovery.cpp 84.86% <0.00%> (+0.29%) ⬆️
src/libsync/syncengine.cpp 85.74% <0.00%> (+0.54%) ⬆️
src/libsync/vfs/cfapi/cfapiwrapper.cpp 77.34% <0.00%> (+1.92%) ⬆️
src/libsync/vfs/cfapi/vfs_cfapi.cpp 86.46% <0.00%> (+2.62%) ⬆️
src/libsync/vfs/cfapi/hydrationjob.cpp 56.45% <0.00%> (+3.76%) ⬆️

@camilasan
Copy link
Member

/backport to stable-3.5

@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-4707-e8cc3690ce6e9793e6da1d4252a72992e6f2b6d5-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@sonarcloud
Copy link

sonarcloud bot commented Jul 12, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@claucambra claucambra merged commit 1ec3c38 into master Jul 12, 2022
@claucambra claucambra deleted the feature/macos-icon branch July 12, 2022 23:48
@backportbot-nextcloud
Copy link

The backport to stable-3.5 failed. Please do this backport manually.

@mgallien mgallien added this to the 3.6.0 milestone Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants