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

Fix Finder sidebar icon to work as a "template" image #4367

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

elsiehupp
Copy link
Member

Fixes #3695

According to the Apple documentation, "template" images must be exclusively "monochromatic images that are drawn just using black and transparency", but desktop/theme/colored/Nextcloud-sidebar.svg looks like this:

Since this bug popped up without changes in the code or assets in question, my guess is that the current version of Xcode or macOS is being clever and interpreting the white as a color to be inverted, while previously it treated the white as just an opaque transparency mask. This seems to be due to a change in how non-monochrome "template" images are supported with the release of SF Symbols 3. Among other things, SVG images now seem to be natively supported, as best I can tell from the documentation here.

I can't test this theory because macOS builds are currently broken, so I'm posting an updated version of Nextcloud-sidebar.svg that should work properly as a conventional "template" image without actually put it through a build first.

@elsiehupp
Copy link
Member Author

GPG signing seems to be broken since I stopped using GitHub's commit-email obfuscation service. I probably need to create a new GPG key.

@claucambra
Copy link
Collaborator

claucambra commented Mar 21, 2022

We have a branch with a black SVG image and this did not fix the issue unfortunately

@claucambra
Copy link
Collaborator

We do have an update on this bug report #4325 that the issue has been fixed on 12.3, is this the case? I am for now not able to test on this macOS version

@elsiehupp
Copy link
Member Author

@claucambra do you know how to do a macOS build currently? The existing instructions are broken.

@claucambra
Copy link
Collaborator

claucambra commented Mar 22, 2022

@claucambra do you know how to do a macOS build currently? The existing instructions are broken.

Personally I build it by installing QtCreator and OpenSSL from brew. You'll have to add -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl to the initial CMake parameters

@claucambra
Copy link
Collaborator

I'm on macOS 12.3 now and the black sidebar icon bug has now been fixed by Apple (yay!)

Still, we can merge this so that we fully adhere to the guidelines in case things break in the future

@elsiehupp
Copy link
Member Author

I'm on macOS 12.3 now and the black sidebar icon bug has now been fixed by Apple (yay!)

Still, we can merge this so that we fully adhere to the guidelines in case things break in the future

After I got this in my email I updated to 12.3, and it fixed things for me, too.

Looking at the SF Symbols 3 documentation there are some other aspects of the namespace/specification that might be useful, like idk annotated weights and whatnot. (This would require using the raw SVG rather than rasterizing it.)

Anyway, I made a version of the icon that tweaks the geometry slightly in order to be more in line with SF Symbols. The first two are the tweaked version of the icon, and the second two are a slightly rationalized version of the existing icon.

nextcloud-sidebar-sf-symbols-blacknextcloud-sidebar-sf-symbols-whitenextcloud-sidebar-original-blacknextcloud-sidebar-original-white

And here's a screenshot of my Finder sidebar to show how the existing icon looks next to the standard SF Symbols icons:

finder-sidebar

The main thing I changed is making the circles tangent to each other at the centerlines of their strokes, which subsequently allowed me to change the line weight. An SF Symbols regular-size/medium-weight icon seems to have roughly 7px line weight within a 128px canvas. I also tweaked the geometry of the circles to make them have simple integer ratios: the large circle has a diameter of 49px, and the smaller circles have a diameter of 28px, both numbers which are intentionally integer multiples of 7px (the line weight).

FWIW if I were to get carried away I could thoroughly "Mac-ify" the rest of the monochrome icons for the macOS build, but that really has nothing to do with this pull request (and neither, really, does the "rationalized" SF-Symbols version of the sidebar icon).

Back on the original topic, while I did use stroke width for the versions of the icon I attached to this comment, I could with not too much difficulty make visually indistinguishable versions that use masking instead, so that the SVG could completely eliminate style information.

As it is, the rationalized version of the icon I have attached dramatically simplifies the SVG markup, from this:

<?xml version="1.0" encoding="utf-8"?>
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1024 1024">
<!-- This SVG must be black in order to render correctly as a macOS "template" image. -->
<path d="M513.6,266.9c-106.7,0-196.3,73.1-223.6,171.3
	c-23.9-52.6-76.5-89.8-137.6-89.8C69.2,348.5,0.8,416.9,0.8,500c0,83.2,68.4,151.6,151.6,151.6c61.1,0,113.7-37.3,137.5-89.9
	c27.4,98.3,117,171.4,223.7,171.4c106.1,0,195.2-72.3,223.2-169.7c24.3,51.6,76.1,88.2,136.5,88.2c83.2,0,151.6-68.4,151.6-151.6
	c0-83.2-68.4-151.6-151.6-151.6c-60.4,0-112.2,36.6-136.5,88.2C708.8,339.2,619.7,266.9,513.6,266.9L513.6,266.9z M513.6,355.9
	c80.1,0,144.1,64,144.1,144.1c0,80.1-64,144.2-144.1,144.1c-80.1,0-144.1-64-144.1-144.1C369.5,419.9,433.5,355.9,513.6,355.9
	L513.6,355.9z M152.3,437.4c35.1,0,62.6,27.5,62.6,62.6c0,35.1-27.5,62.6-62.6,62.6c-35.1,0-62.6-27.5-62.6-62.6
	C89.8,464.9,117.2,437.4,152.3,437.4L152.3,437.4z M873.2,437.4c35.1,0,62.6,27.5,62.6,62.6c0,35.1-27.5,62.6-62.6,62.6
	c-35.1,0-62.6-27.5-62.6-62.6C810.6,464.9,838.1,437.4,873.2,437.4L873.2,437.4z"/>
</svg>

(Which already has cruft removed from the original...)

To this:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="100%" height="100%" viewBox="0 0 128 128" version="1.1" xmlns="http://www.w3.org/2000/svg" style="fill:none;stroke:black;stroke-width:11px;">
    <circle id="right" cx="109" cy="64" r="13.5"/>
    <circle id="center" cx="64" cy="64" r="23.5"/>
    <circle id="left" cx="19" cy="64" r="13.5"/>
</svg>

With masking I would still be able to make the icon entirely out of circles, rather than using béziers, like the current version does.

@elsiehupp
Copy link
Member Author

Here's the original icon using masks:

nextcloud-sidebar-original-mask-blacknextcloud-sidebar-original-mask-white

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="100%" height="100%" viewBox="0 0 128 128" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <circle id="left" cx="19" cy="64" r="19" mask="url(#left-clip)"/>
    <circle id="center" cx="64" cy="64" r="29" mask="url(#center-clip)"/>
    <circle id="right" cx="109" cy="64" r="19" mask="url(#right-clip)"/>
    <defs>
        <mask id="left-clip">
            <rect x="0" y="0" width="100%" height="100%" fill="white"/>
            <circle cx="19" cy="64" r="8"/>
        </mask>
        <mask id="center-clip">
            <rect x="0" y="0" width="100%" height="100%" fill="white"/>
            <circle cx="64" cy="64" r="18"/>
        </mask>
        <mask id="right-clip">
            <rect x="0" y="0" width="100%" height="100%" fill="white"/>
            <circle cx="109" cy="64" r="8"/>
        </mask>
    </defs>
</svg>

@elsiehupp
Copy link
Member Author

I wouldn't exactly say it's less complicated than the bézier version, but it's definitely more human-readable!

@elsiehupp
Copy link
Member Author

This is a slightly different version of the masked SVG, where there is only one mask for the whole image, rather than one mask per circle:

nextcloud-sidebar-original-mask-combo-blacknextcloud-sidebar-original-mask-combo-white

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="100%" height="100%" viewBox="0 0 128 128" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <g mask="url(#composite-mask)">
        <circle id="left-circle" cx="19" cy="64" r="19"/>
        <circle id="center-circle" cx="64" cy="64" r="29"/>
        <circle id="right-circle" cx="109" cy="64" r="19"/>
    </g>
    <defs>
        <mask id="composite-mask">
            <rect id="mask-canvas" x="0" y="0" width="100%" height="100%" fill="white"/>
            <circle id="left-circle-cutout" cx="19" cy="64" r="8"/>
            <circle id="center-circle-cutout" cx="64" cy="64" r="18"/>
            <circle id="right-circle-cutout" cx="109" cy="64" r="8"/>
        </mask>
    </defs>
</svg>

I also gave elements IDs even where unnecessary, just to annotate what's what.

I think this version is a bit easier to grok than the previous one. What do you think? I need to replace my commit with one that has the correct GPG signature anyway, so I can easily use any of these for the pull request.

@claucambra
Copy link
Collaborator

I'm on macOS 12.3 now and the black sidebar icon bug has now been fixed by Apple (yay!)
Still, we can merge this so that we fully adhere to the guidelines in case things break in the future

After I got this in my email I updated to 12.3, and it fixed things for me, too.

Looking at the SF Symbols 3 documentation there are some other aspects of the namespace/specification that might be useful, like idk annotated weights and whatnot. (This would require using the raw SVG rather than rasterizing it.)

Anyway, I made a version of the icon that tweaks the geometry slightly in order to be more in line with SF Symbols. The first two are the tweaked version of the icon, and the second two are a slightly rationalized version of the existing icon.

nextcloud-sidebar-sf-symbols-blacknextcloud-sidebar-sf-symbols-whitenextcloud-sidebar-original-blacknextcloud-sidebar-original-white

And here's a screenshot of my Finder sidebar to show how the existing icon looks next to the standard SF Symbols icons:

finder-sidebar

The main thing I changed is making the circles tangent to each other at the centerlines of their strokes, which subsequently allowed me to change the line weight. An SF Symbols regular-size/medium-weight icon seems to have roughly 7px line weight within a 128px canvas. I also tweaked the geometry of the circles to make them have simple integer ratios: the large circle has a diameter of 49px, and the smaller circles have a diameter of 28px, both numbers which are intentionally integer multiples of 7px (the line weight).

FWIW if I were to get carried away I could thoroughly "Mac-ify" the rest of the monochrome icons for the macOS build, but that really has nothing to do with this pull request (and neither, really, does the "rationalized" SF-Symbols version of the sidebar icon).

Back on the original topic, while I did use stroke width for the versions of the icon I attached to this comment, I could with not too much difficulty make visually indistinguishable versions that use masking instead, so that the SVG could completely eliminate style information.

As it is, the rationalized version of the icon I have attached dramatically simplifies the SVG markup, from this:

<?xml version="1.0" encoding="utf-8"?>
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1024 1024">
<!-- This SVG must be black in order to render correctly as a macOS "template" image. -->
<path d="M513.6,266.9c-106.7,0-196.3,73.1-223.6,171.3
	c-23.9-52.6-76.5-89.8-137.6-89.8C69.2,348.5,0.8,416.9,0.8,500c0,83.2,68.4,151.6,151.6,151.6c61.1,0,113.7-37.3,137.5-89.9
	c27.4,98.3,117,171.4,223.7,171.4c106.1,0,195.2-72.3,223.2-169.7c24.3,51.6,76.1,88.2,136.5,88.2c83.2,0,151.6-68.4,151.6-151.6
	c0-83.2-68.4-151.6-151.6-151.6c-60.4,0-112.2,36.6-136.5,88.2C708.8,339.2,619.7,266.9,513.6,266.9L513.6,266.9z M513.6,355.9
	c80.1,0,144.1,64,144.1,144.1c0,80.1-64,144.2-144.1,144.1c-80.1,0-144.1-64-144.1-144.1C369.5,419.9,433.5,355.9,513.6,355.9
	L513.6,355.9z M152.3,437.4c35.1,0,62.6,27.5,62.6,62.6c0,35.1-27.5,62.6-62.6,62.6c-35.1,0-62.6-27.5-62.6-62.6
	C89.8,464.9,117.2,437.4,152.3,437.4L152.3,437.4z M873.2,437.4c35.1,0,62.6,27.5,62.6,62.6c0,35.1-27.5,62.6-62.6,62.6
	c-35.1,0-62.6-27.5-62.6-62.6C810.6,464.9,838.1,437.4,873.2,437.4L873.2,437.4z"/>
</svg>

(Which already has cruft removed from the original...)

To this:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="100%" height="100%" viewBox="0 0 128 128" version="1.1" xmlns="http://www.w3.org/2000/svg" style="fill:none;stroke:black;stroke-width:11px;">
    <circle id="right" cx="109" cy="64" r="13.5"/>
    <circle id="center" cx="64" cy="64" r="23.5"/>
    <circle id="left" cx="19" cy="64" r="13.5"/>
</svg>

With masking I would still be able to make the icon entirely out of circles, rather than using béziers, like the current version does.

I like the first (skinnier) icon a lot, it definitely integrates with the other sidebar icons better than our current one does. @jancborchardt thoughts? There's nothing major to change here code-wise so this feels like more of a design discussion :)

@elsiehupp
Copy link
Member Author

elsiehupp commented Mar 25, 2022

And here's a version that I think is even more legible even if it's a bit verbose:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="100%" height="100%" viewBox="0 0 128 128" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <rect id="left-circle-canvas" x="0" y="0" width="100%" height="100%" mask="url(#left-circle-mask)"/>
    <rect id="center-circle-canvas" x="0" y="0" width="100%" height="100%" mask="url(#center-circle-mask)"/>
    <rect id="right-circle-canvas" x="0" y="0" width="100%" height="100%" mask="url(#right-circle-mask)"/>
    <defs>
        <mask id="left-circle-mask">
            <circle id="left-circle-outer" cx="19" cy="64" r="19" fill="white"/>
            <circle id="left-circle-inner" cx="19" cy="64" r="8" fill="black"/>
        <mask id="center-circle-mask">
        </mask>
            <circle id="center-circle-outer" cx="64" cy="64" r="29" fill="white"/>
            <circle id="center-circle-inner" cx="64" cy="64" r="18" fill="black"/>
        <mask id="right-circle-mask">
        </mask>
            <circle id="right-circle-outer" cx="109" cy="64" r="19" fill="white"/>
            <circle id="right-circle-inner" cx="109" cy="64" r="8" fill="black"/>
        </mask>
    </defs>
</svg>

Unless there's a strong reason to use fill rather than strokes, though, I think just using the circles with stroke widths would be simplest:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="100%" height="100%" viewBox="0 0 128 128" version="1.1" xmlns="http://www.w3.org/2000/svg" fill="none" stroke="black" stroke-width="11px">
    <circle id="right" cx="109" cy="64" r="13.5"/>
    <circle id="center" cx="64" cy="64" r="23.5"/>
    <circle id="left" cx="19" cy="64" r="13.5"/>
</svg>

(IIRC the main reason to use fill rather than strokes is that GTK "symbolic" icons don't support strokes... but Nextcloud uses Qt rather than GTK, and anyway this particular asset is going directly through Xcode, so it probably doesn't matter.)

@elsiehupp
Copy link
Member Author

And here's the equivalent version of the SF-Symbols-style icon:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="100%" height="100%" viewBox="0 0 128 128" version="1.1" xmlns="http://www.w3.org/2000/svg" fill="none" stroke="black" stroke-width="7px">
    <circle id="right" cx="102.5" cy="64" r="14"/>
    <circle id="center" cx="64" cy="64" r="24.5"/>
    <circle id="left" cx="25.5" cy="64" r="14"/>
</svg>

@mgallien
Copy link
Collaborator

@elsiehupp thanks for your contribution
once we reach a state where this PR can be merged we will wait a bit more due to the preparation of the next feature release

@sonarcloud

This comment was marked as outdated.

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! :)

@codecov

This comment was marked as outdated.

@elsiehupp elsiehupp force-pushed the finder-sidebar-icon-fix branch 3 times, most recently from 3f0b264 to c527d68 Compare June 27, 2022 23:02
@elsiehupp
Copy link
Member Author

Since this PR is still pending, I changed it to use the SF Symbols lineweights. I also changed the filename to emphasize that the icon is only used on macOS, and I imagine that if you were to merge the corresponding change to CMakeLists.txt you would need to change the corresponding filenames in all the OEM builds, as well.

@elsiehupp
Copy link
Member Author

And I fixed my GPG key! Yay!

@sonarcloud
Copy link

sonarcloud bot commented Jul 1, 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

@github-actions github-actions bot added the stale label Oct 25, 2023
@claucambra claucambra force-pushed the finder-sidebar-icon-fix branch 2 times, most recently from 1d1e0ab to 29a6330 Compare October 17, 2024 07:52
@claucambra claucambra merged commit 32b035b into master Oct 17, 2024
9 of 14 checks passed
@claucambra claucambra deleted the finder-sidebar-icon-fix branch October 17, 2024 07:55
@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-4367-b04edf1b62ecb453d129248f7275b67ec35eefb1-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.

Copy link

sonarcloud bot commented Oct 17, 2024

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.

Finder sidebar icon displays incorrectly
5 participants