Add firewall-config and firewall-applet icons
ClosedPublic

Authored by ndavis on Apr 2 2018, 7:39 AM.

Details

Summary

48px color icons for firewall-config and firewall-applet. firewall-applet is a symlink to firewall-config.
22px, 16px monochrome icons for the system tray: firewall-applet, firewall-applet-error and firewall-applet-panic. firewall-applet and firewall-applet-error are symlinks to security-high and security-low, but firewall-applet-panic is a new icon because security-medium does not accurately represent panic mode.

Diff Detail

Repository
R266 Breeze Icons
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
ndavis requested review of this revision.Apr 2 2018, 7:39 AM
ndavis added a comment.Apr 2 2018, 7:40 AM
This comment was removed by ndavis.
ndavis edited the summary of this revision. (Show Details)Apr 2 2018, 7:44 AM
ndavis added a reviewer: VDG.
ndavis edited the summary of this revision. (Show Details)Apr 2 2018, 7:48 AM
ndavis edited the summary of this revision. (Show Details)
ndavis added a comment.Apr 2 2018, 7:54 AM

Screenshot of the icons in use

ndavis added a comment.EditedApr 2 2018, 8:13 AM

firewall-applet-panic in use. Panic mode blocks all network traffic. Unlike security-medium, the shield is full, but half of it is icon yellow. I chose icon yellow because it's supposed to be the color for warnings.

Can someone review this?

ngraham requested changes to this revision.Sep 20 2018, 2:25 AM
ngraham added a subscriber: ngraham.

Oops, it looks like this patch totally got missed, sorry!

Not sure I like that firewall-applet-error is linked to security-low. To me, a firewall error should have some more scary red iconography in it. Error == red.

For that matter, I would expect an icon with the word "panic" in it to also have red. "panic == even worse than "error"!

This revision now requires changes to proceed.Sep 20 2018, 2:25 AM
Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. · View Herald TranscriptSep 20 2018, 2:25 AM

Oops, it looks like this patch totally got missed, sorry!

Not sure I like that firewall-applet-error is linked to security-low. To me, a firewall error should have some more scary red iconography in it. Error == red.

Fair enough. I will consider changing it to an all red icon to indicate the severity, especially for the 16px icon which lacks much of any indication that something is seriously wrong.

For that matter, I would expect an icon with the word "panic" in it to also have red. "panic == even worse than "error"!

Panic mode in FirewallD is not worse than Error. Panic mode is enabled by the user. It is something that the user enables when they need to stop all network traffic to the system.

Panic mode in FirewallD is not worse than Error. Panic mode is enabled by the user. It is something that the user enables when they need to stop all network traffic to the system.

Oh! In that case, I would say it should look extra secure somehow. Rather than using the orange warning color and iconography, maybe figure out a way to make it look even stronger than normal. Maybe a giant lock? Just throwing out ideas.

https://developer.gnome.org/icon-naming-spec/

security-highThe icon used to indicate that the security level of a connection is known to be secure, using strong encryption and a valid certificate.
security-mediumThe icon used to indicate that the security level of a connection is presumed to be secure, using strong encryption, and a certificate that could not be automatically verified, but which the user has chosen to trust.
security-lowThe icon used to indicate that the security level of a connection is presumed to be insecure, either by using weak encryption, or by using a certificate that the could not be automatically verified, and which the user has not chosent to trust.

Based on this information, I have decided to redo the firewall-applet icons because the original meaning of the security-* icons does not match what I am reusing them for. Instead, the firewall-applet icons will look like brick walls.

Panic mode in FirewallD is not worse than Error. Panic mode is enabled by the user. It is something that the user enables when they need to stop all network traffic to the system.

Oh! In that case, I would say it should look extra secure somehow. Rather than using the orange warning color and iconography, maybe figure out a way to make it look even stronger than normal. Maybe a giant lock? Just throwing out ideas.

Thanks. Not sure how I could make a big lock work with the new design, but I'll consider putting one in the lower right corner, similar to other 16 and 22 px breeze icons.

ndavis updated this revision to Diff 41971.EditedSep 20 2018, 8:57 AM

Change icons to brickwall style

ndavis retitled this revision from Add firewalld icons to Add firewall-config and firewall-applet icons.Sep 20 2018, 8:59 AM
ndavis updated this revision to Diff 41972.EditedSep 20 2018, 9:07 AM

Fix mixed up firewall-applet and firewall-applet-panic (22px)

ndavis added a comment.EditedSep 20 2018, 9:14 AM

Hmm. Something was preventing me from changing the name of firewall-applet to firewall-applet-panic (apps/22) and vice versa. I own all of the files in that directory (noah:users), but the file names kept being reset to their current state. I fixed it by removing the files and then re-adding a copy of them from another directory.

ndavis updated this revision to Diff 41973.Sep 20 2018, 9:22 AM

Fix mixed up firewall-applet and firewall-applet-panic (22px)

Regular


Panic

Not sure if I want to try to cause an error on my own system just to show how the error icon looks.

ngraham accepted this revision.Sep 21 2018, 3:05 AM

I like the brick wall metaphor, which does a good job of communicating the point, while being visually distinct from the shield.

Looks great to me! Let's see if any other VDG folks have opinions in the next day or two, and if not, I'll ship it.

This revision is now accepted and ready to land.Sep 21 2018, 3:05 AM

I like the regular icon, looks great!

Though I'm unsure about the panic icon. It communicates "all is well" to me instead of "all network traffic blocked". Maybe there is a better way to achieve this. An orange icon with a spread out hand in front of it? Just an idea.

alex-l added a subscriber: alex-l.Sep 21 2018, 11:39 AM

Very cool icons! But in the panic one the padlock is too small :-/ what a about a checkmark (✅) on the wall instead?

abetts added a subscriber: abetts.Sep 21 2018, 1:15 PM

If I understand right, there are three states of firewall security that you can be in. Maybe we could use the traffic lights metaphor and have green for low, yellow for medium and red for high?

bruns added a subscriber: bruns.Sep 21 2018, 4:57 PM

How about these - error, "trusted zone", panic, "public/untrusted"

Seems good to me. Guys?

ngraham added a comment.EditedSep 21 2018, 5:05 PM

I like your depiction of the logical consequences of the firewall being depicted as a wall, @bruns. That seems great. I think a further exploration is warranted:

Error: wall is damaged or compromised; maybe holes in it or a pile of brick rubble! Red color is appropriate here

Trusted zone: doorway or open door in wall - yours seems perfect.

Panic: extra security: could use iconography of bars, swords, shields, locks, etc. Anything to connote "nobody's getting through here!" Not sure red is the right color here.

Public/untrusted: only some entry allowed; closed door would work. Yours seems a bit too unfriendly with those vertical bars maybe.

bruns added a comment.Sep 21 2018, 6:05 PM

bruns added a comment.Sep 21 2018, 6:13 PM

Alternate error version:

Now that's what I'm talkin' about. Those are awesome in my book. Other VDG folks, thoughts?

I solemnly approve! +1

I think I'm confused about the process here. Are we using @bruns's icons now?

How about these - error, "trusted zone", panic, "public/untrusted"

Firewall-applet does not use trusted and untrusted icons. There is just normal, panic and error.

ndavis added a comment.EditedSep 22 2018, 3:11 AM

Here are some new styles based on suggestions and @bruns's ideas. I've also tried to make one of the new styles looks similar to the status/*/state-* icons.

16px

22px

bruns added a comment.Sep 22 2018, 3:16 AM

16px

22px

If you use "{xxx, size=full}", you can avoid the scaling (preferable for small inline images).

If you use "{xxx, size=full}", you can avoid the scaling (preferable for small inline images).

Fixed. Thanks!

ndavis updated this revision to Diff 42119.Sep 22 2018, 6:33 AM

Change to bruns/state-* hybrid style

For the benefit of others, here's what they look like now:

22px:

48px:

I like firewall-config and firewall-applet-error as they are.

I think that firewall-applet looks maybe a bit too plain at its 22px size. The wall seems to need something.

I like the lock on firewall-applet-panic. I'm not sure about the orange color though. Orange means "warning". If I'm understanding you correctly, firewall-applet-panic should evoke feelings of maximum safety.

I like firewall-config and firewall-applet-error as they are.

I think that firewall-applet looks maybe a bit too plain at its 22px size. The wall seems to need something.

Maybe the emblem-checked icon should go in the lower right corner to match the style of the other two firewall-applet icons? I choose to make it fairly simple so that it would normally fit in with other icons in the system tray except for when there is an error or Panic Mode is enabled.

I like the lock on firewall-applet-panic. I'm not sure about the orange color though. Orange means "warning". If I'm understanding you correctly, firewall-applet-panic should evoke feelings of maximum safety.

Yes, but I also agree with @svenmauch about the previous version being too positive. If a user enables panic mode by accident, how will they know something might be wrong with their settings? As it is, firewall-applet-panic is the firewall-applet icon with the emblem-locked icon in the corner and I've applied the color of emblem-locked to the wall. While orange may mean "warning", it could mean any kind of warning. In this case, it's a warning that an extreme setting is being used, but there is a lock in the corner to show that it is at least safe.

Here's how the checkmark idea looks. I prefer the plain wall since it fits in with the other icons better.

16px

22px

Here's another idea with a shield in the middle, similar to the 48px icon. I still prefer the plain wall.

16px

22px

For the panic mode icon, how about leaving the wall itself black, and only the lock is orange?

I don't want to totally dominate the conversation here, so I'll step aside for a bit and let others have their say. But I want to let you know that I really appreciate your patience here, @ndavis. I'm sorry this there's been so much back-and-forth and that the patch sat ignored for so long. We'll do better for your next one!

For the panic mode icon, how about leaving the wall itself black, and only the lock is orange?

Like this? This is #4d4d4d (icon grey), the standard color for small breeze icons, not black.

I don't want to totally dominate the conversation here, so I'll step aside for a bit and let others have their say. But I want to let you know that I really appreciate your patience here, @ndavis. I'm sorry this there's been so much back-and-forth and that the patch sat ignored for so long. We'll do better for your next one!

Thank you! It's been great to see how other people come up with designs here.

ndavis updated this revision to Diff 42212.Sep 24 2018, 4:35 AM

Change wall color of firewall-applet-panic to \#4d4d4d (breeze) and \#f2f2f2 (breeze-dark)

While orange may mean "warning", it could mean any kind of warning. In this case, it's a warning that an extreme setting is being used, but there is a lock in the corner to show that it is at least safe.

Exactly what I thought!

Like this? This is #4d4d4d (icon grey), the standard color for small breeze icons, not black.

Looks great! +1

I think that firewall-applet looks maybe a bit too plain at its 22px size. The wall seems to need something.

I think the 22px firewall-applet looks really good and would fit perfectly with the notifications popup and task icons. I'm not sure why the 48px version is colored though?

Thank you for the superb work @ndavis & @bruns!

I think that firewall-applet looks maybe a bit too plain at its 22px size. The wall seems to need something.

I think the 22px firewall-applet looks really good and would fit perfectly with the notifications popup and task icons. I'm not sure why the 48px version is colored though?

It's just a symlink to the firewall-config icon. When I started this diff, firewall-applet showed up in searches for "firewall" in the application menus. That appears not to be the case anymore, so I will remove the 48px firewall-applet icon.

Thank you for the superb work @ndavis & @bruns!

Thanks!

ndavis updated this revision to Diff 42257.Sep 24 2018, 2:27 PM

Remove 48px firewall-applet icon

For the panic mode icon, how about leaving the wall itself black, and only the lock is orange?

Like this? This is #4d4d4d (icon grey), the standard color for small breeze icons, not black.

Perfect, I love it!

FWIW, I'm okay with the existing firewall-applet. So here's what we've got now, as viewed with cuttlefish:

I say +1!

However, there seems to be a problem for the breeze dark versions:

Shouldn't the wall be light-colored? Does it work for you when actually deployed and in use with breeze dark?

FWIW, I'm okay with the existing firewall-applet. So here's what we've got now, as viewed with cuttlefish:

I say +1!

However, there seems to be a problem for the breeze dark versions:

Shouldn't the wall be light-colored? Does it work for you when actually deployed and in use with breeze dark?

Yes, it should be #f2f2f2 and it is on my end. I have the correct color on my own copy and when I download the raw files from Phabricator. Are you sure you aren't experiencing some kind of bug or maybe your cache is old?

Hmm, I still get the issue in Cuttlefish after clearing the cache.

Is anyone else able to reproduce this issue with viewing the icons in Cuttlefish and turning on "Inverted"?

ngraham accepted this revision.Sep 25 2018, 5:02 PM

Never mind, it's a Cuttlefish issue. Everything looks good to me.

Do we have final sign-off by other VDG folks?

bruns added a comment.EditedSep 26 2018, 2:15 PM

Never mind, it's a Cuttlefish issue. Everything looks good to me.

Do we have final sign-off by other VDG folks?

Sorry to add some last minute disturbance, but is seems firewall-applet will get distinct firewall-applet-shields_up and firewall-applet-shields_down icons in the next version:
https://github.com/firewalld/firewalld/pull/394

So we need more icons, then?

bruns added a comment.Sep 26 2018, 2:18 PM

So we need more icons, then?

Preferably yes, but both would fall back to firewall-applet otherwise.

ndavis added a comment.EditedSep 26 2018, 5:13 PM

I'm down to add a shields-up state icon. shields-down can just be the normal icon.

How about this?

bruns added a comment.Sep 26 2018, 7:25 PM

I'm down to add a shields-up state icon. shields-down can just be the normal icon.

Thats completely fine IMHO, we just need two different icons for shields up and down.

bruns added a comment.Sep 26 2018, 7:30 PM

How about this?

Looks good - can you provide an overview png of the complete firewall-applet icon set? I.e. the (now four) status icons and the config icon?

Looks good - can you provide an overview png of the complete firewall-applet icon set? I.e. the (now four) status icons and the config icon?

bruns added a comment.Sep 26 2018, 7:51 PM

Looks good - can you provide an overview png of the complete firewall-applet icon set? I.e. the (now four) status icons and the config icon?

Great, thanks!
+1!

ndavis updated this revision to Diff 42390.Sep 26 2018, 7:53 PM

Add firewall-applet-shields_up icon

ngraham accepted this revision.Sep 26 2018, 8:44 PM

+1 ship it!

Final VDG review?

Love it! Ship it! +100

Shipping it!

This revision was automatically updated to reflect the committed changes.
dfaure added a subscriber: dfaure.Oct 6 2018, 8:48 AM

This commit breaks the unittest "scalabletest" :

FAIL! : ScalableTest::test_scalable(icons:Applications) The following icons are not available in a scalable directory:

firewall-applet-panic
firewall-applet
firewall-applet-error
firewall-applet-shields_up
 Loc: [/d/kde/src/5/frameworks/breeze-icons/autotests/scalabletest.cpp(262)]

FAIL! : ScalableTest::test_scalable(icons-dark:Applications) The following icons are not available in a scalable directory:

firewall-applet-panic
firewall-applet
firewall-applet-error
firewall-applet-shields_up
 Loc: [/d/kde/src/5/frameworks/breeze-icons/autotests/scalabletest.cpp(262)]

Please fix ;)

ndavis added a comment.Oct 7 2018, 2:59 PM

This commit breaks the unittest "scalabletest" :

FAIL! : ScalableTest::test_scalable(icons:Applications) The following icons are not available in a scalable directory:

firewall-applet-panic
firewall-applet
firewall-applet-error
firewall-applet-shields_up
 Loc: [/d/kde/src/5/frameworks/breeze-icons/autotests/scalabletest.cpp(262)]

FAIL! : ScalableTest::test_scalable(icons-dark:Applications) The following icons are not available in a scalable directory:

firewall-applet-panic
firewall-applet
firewall-applet-error
firewall-applet-shields_up
 Loc: [/d/kde/src/5/frameworks/breeze-icons/autotests/scalabletest.cpp(262)]

Please fix ;)

I'm afraid I don't understand the error or what to do about it. Aren't SVGs naturally scalable? Isn't the scalable directory unused in the Breeze icon theme because every icon is scalable?

dfaure added a comment.Oct 7 2018, 4:33 PM

I don't really know, but see https://phabricator.kde.org/D4254 for the full reasoning of the unittest.

bruns added a comment.Oct 7 2018, 5:18 PM

I don't really know, but see https://phabricator.kde.org/D4254 for the full reasoning of the unittest.

As far as I understand it, the themes are marked as scalable with a directory scope (icon theme spec), but there is no guarantee a theme is "complete".

AFAICS, it is sufficient to link the 48px config icon and the other 22px icons into the "scalable" directory.