-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
Change background connection icon #7336
Conversation
Fixes signalapp#7255 FREEBIE This commit changes the background connection icon to one that can be distinguished from the incoming message icon.
I don't understand what the icon is supposed to represent? I think it would be better if the icon were a variation of the Signal icon. |
@moxie0 something like this:
|
Oh wow that's a nice icon @p4nci — if you send the .xml to me I can push it to this PR... |
Both of the icons look great @p4nci, but I think icon 3 best maintains Signal's visual identity (doesn't require shrinking the root icon). |
#7278 suggested using separate notification channels, so that the "background connection" notification could be disabled by the user without affecting other Signal notifications. I think that would be a cleaner fix. Current master branch (commit 0bbe83f) targets API level 25 (not 26) and has no mentions of NotificationChannel. |
Some of those mockups look pretty good, please reopen when available |
unfortunately @iadmir did not react to my request (pm @community forum), so please can someone provide the original SVG Signal icon? Making a vector graphic from a png file it does NOT work as expected, at least for me 😞 |
@p4nci is anything here helpful to you? https://github.com/signalapp/Signal-Design/tree/master/android/icons/app_icon |
@michaelkirk-signal yes, thanks. Somehow I missed this one 😡 |
@p4nci I'm happy to change my original PR branch to use your new icon, FYI—I want to get this pushed through ASAP! |
update: |
@p4nci thanks for the mention, I received the first notification but I forgot about this.
|
@iadmir would it be possible to convert that to a SVG? That way we could simply add it as an icon asset and go from there. |
Sure, there is compressed svg. |
@iadmir just one thing, it's the wrong signal icon (the "symmetric" one, see also #5936 (comment)) btw. still 🤒 edit: big thanks @iadmir for the all work |
Oh you're right, good observation, I edited the previous comment with attached svg |
@p4nci it is sufficient, but requires converting into an Android-ready |
I use Ai for vector design and there isn't option to save as .xml but I think you can export this format with Adobe InDesign. |
this: one more thing: |
When I need to convert svg icons to VectorDrawables, I use this tool: https://inloop.github.io/svg2android/ The conversion yields:
|
The background color should of course be removed. 😉 |
@p4nci here's the current problem with the PR: the icon needs to be scaled within its squared window so that it is the proper size for a notification. (Right now, it takes up about 75% of the window.) @iadmir — would it be possible for you to make a small edit of your original icon (which is awesome) so that it uses 100% of the height of the SVG window? I tried to fiddle with it myself, but don't have the necessary SVG knowledge. Thanks! |
@milesmcc Try this. icon-signal-2-modified-nikwen.zip |
Ignore the xml file in the zip above. I tried it and it seems as if the conversion using the svg2android online tool messed up. The svg from the zip, however, is correct. Converting the svg to xml using Android studio, I got this, which seems to work well:
|
@milesmcc Sent you a PR: milesmcc#1 |
@nikwen — thank you! I will rebase and resubmit tomorrow. |
Thank *you*, @milesmcc. :)
|
Hi there! Just merged a new icon for this into 4.30. Special thanks to @p4nci who reached out and sent the icon design. Thanks, everyone! |
Contributor checklist
Fixes #1234
syntaxDescription
This pull request changes the icon for the 'background connection enabled' notification to a new icon,
ic_background_connection_dark_24dp
so that it can be differentiated from message notifications in the status bar. Fixes #7255 (though improves the situation is perhaps more accurate).This is what the expanded notification drawer used to look like (sensitive data removed):
Note how this results in two Signal-icon notifications in the status bar when a message notification is also present:
This pull request changes the background connection icon to
ic_background_connection_dark_24dp
(a new icon created by @ruddfawcett for this purpose), making the message notification icon different from that of the background connection notification, so that the two can be easily differentiated:As for the icon itself, it's supposed to be a cross between a signal coming out of a person—to indicate that the device is connected in the background—and a lock, to go along with the theme of Signal as a secure messenger.
This change affects only a small number of users (i.e. users whose phones run Android without Google Play Services), and is only particularly relevant to users of Android >= 8.1 (the background connection notification does not appear in the status bar in lower versions). That is to say, not many will see this icon.
This contribution comes out of a desire to fix something that has been nagging at me ever since I updated to Android 8.1, and I imagine that I'm not the only user who is bothered by the confusing background connection icon. (It looks like I always have a notification.)
I'm happy to change the icon if the chosen one seems inappropriate, though I like the icon.
This contribution is a freebie.
(This pull request is an improved version of this old pull request. I was asked to change the icon and resubmit.)