It looks like this switches us from the version of the logo with a bluer-green background and a smaller Z:

to the version with a paler, yellower-green background and a larger Z:

Was that change intentional? I'm not immediately sure which logo is preferable (would have to go look around a bit), but we 100% definitely should not change it by accident -- and the commit message should make super clear that that's what it's doing. (Better yet, make it a separate commit from the revision to the icon format/silhouette.)

Yeah, I noticed that.
I think the original icon was generated incorrectly or the color profile was messed up.
The new version seems to be the correct one because it matches the icon colors of the iOS version.

Hmm -- well, the bluer one seems closer to what I see on the webapp. Before making a change here, let's discuss in #design. I bet there's some history between these two logos that someone there will be glad to tell us about.

In any event, this is very much the kind of thing that we should not do by accident, or silently without making clear in the commit message.

How did you generate the new icons -- what kind of input did you provide to Android Asset Studio?

I used the PNG already in the project that is named zulip-1024.png

I used the PNG already in the project that is named zulip-1024.png

Cool -- that'll be good information for the commit message. :-)

Looks like that one is inside ios/, which fits with the fact that that look is what we have on iOS.

It seems I did add this file a year ago.
And to answer the obvious question: No, I have no idea where I got it from, but definitely did not generate or modify it myself.

To summarize the state of this PR explicitly in one place:

• The information about where the source image came from (zulip-1024.png) is super helpful and definitely goes in the commit message.
• If we change between logos like this:
  • The commit message must say clearly that we're doing that, and why.
  • Preferably, it should be a separate commit from the one that changes the shapes of the existing files and adds the _round files.
  • We should only do so after asking on #design, to make sure we're moving in the same direction as the webapp and the rest of the project.

Looking a bit closer at the "which logo" question: looking for logo images in zulip/zulip , they're pretty consistently of the bluer style we're using on Android now.

Indeed. I agree our iOS logo the one that should change.
I'll ask in #design for a good source image to use.

From that #design thread , looks like the best available source image for the current logo is our static/images/logo/zulip-icon-512x512.png in zulip/zulip. As the name suggests, it's a 512px PNG; the largest of these launcher icons are 144px, so that should be good enough to let us unblock this.

We actually already have a copy of that in our repo, as ios/zulip-logo.png and also static/img/zulip-logo.png. Different from ios/ZulipMobile/.../zulip-1024.png, despite the similar names.

Seems like it'll be an open design task for someone to reconstruct a vector, or super-high-res, version of the logo to let us produce exactly-matching logos for all formats across different platforms. If and when someone does that, we can go on to ship that version.

We absolutely need the 1024x1024 resolution for iOS. Possibly larger, as Apple regularly update their requirements and they just released the major new versions.

Also, to make it clear, this is not a blocker for the upgrade to RN 0.56 and RN 0.57 so we don't need to rush it.

Also, to make it clear, this is not a blocker for the upgrade to RN 0.56 and RN 0.57 so we don't need to rush it.

Oh hmm. I'd thought this was a requirement for increasing the targetSdkVersion, but I guess you didn't quite say that. If it isn't, then this isn't a priority.

On the other hand:

We absolutely need the 1024x1024 resolution for iOS. Possibly larger

OK, but this shouldn't block updating the icon shapes on Android, regardless of how low or high priority the latter task has.

In making a bunch of updates to our Gradle config tonight (see #2789 (comment) ), I found that in fact this does block increasing our targetSdkVersion to 26. (Otherwise, our launcher icon gets shrunken and set on a white disk, which looks pretty careless.)

And targeting 26 is a requirement for uploading new releases to Google Play after the end of this month. So it may not be directly coupled to the RN upgrade, but it has its own urgency 😉

• There are launcher icons in debug/ now too; so if it's easy, we should do the same things there. (I think if nothing else we need to make ic_launcher_round.png exist so it successfully shadows the other.)
  • Also we need xxxhdpi there, if we're going to add it in main/.
• Where's web_hi_res_512.png come from? It looks to be not identical to any other file I see.
  • The webapp has static/images/logo/zulip-icon-512x512.png ; and we actually have that ourselves already as static/img/zulip-logo.png, plus ios/zulip-logo.png.
  • Oh hmm looks like this one is the yellower version. Let's use the other one. :)
• The new _round is different from the existing image, though the latter is also round. Is that intended? If so, it looks fine to me -- but the commit message should mention the change.
• Speaking of which, the commit message should mention that this is adding an xxxhdpi variant.

Differences from the last time reviewed:

• debug icons reworked and added both normal and circular
• mentioned the xxxhdpi in the commit message
• also added to all variants
• removed the web_hi_res_512.png file
• the new _round version has to be much more correct as it is created with an officially approved tool, and also carefully matches the spacing of the non-round version. Any difference with the old one should be in favor of the new one
• updated the iOS icon too

I see there's a new version of this branch, but because there's no comment from you I'm not sure what the intended status of it is.

The new version addresses some of my comments (thanks!). This one is still open:

• The new _round is different from the existing image, though the latter is also round. Is that intended? If so, it looks fine to me -- but the commit message should mention the change.

Also there are about three different things apparently happening here, and the commit message is super confusing:

• We're adding xxxhdpi versions. (What's the story here -- should we have been doing this all along and you just happened to notice while doing this? is it newly going to be required?)
• We're moving the existing round icons to ic_launcher_round.png and making non-round ic_launcher.png icons. (This gives better results on targetSdkVersion of 26+.)
• We're replacing the round icons with slightly different round icons. (Why?)

It's hard to tell that from this commit message. The summary line says "Replace default icon", which I think describes something a previous version of this change did but this version doesn't.

Oh ha, your comment showed up when I submitted mine. :-) Reading...

Higher rez images usually start being required at some point. Not sure when the xxxhdi did added, but my guess is that it is used on tablets or phones with super-high dpi.

OK, thanks for those point-by-point replies! That's very helpful.

the new _round version has to be much more correct as it is created with an officially approved tool, and also carefully matches the spacing of the non-round version. Any difference with the old one should be in favor of the new one

OK, cool. The commit message should say we're making this change, though, and why :-)

Also

an officially approved tool

Oh huh, really? The tool you mentioned doesn't sound official.

The official thing looks to be this:
https://developer.android.com/studio/write/image-asset-studio#create-adaptive

The link to https://romannurik.github.io/AndroidAssetStudio/ was from a couple of official places, and before Android had the Image Asset Studio, they linked to the old version of the AndroidAssetStudio that is why I said 'approved' but not 'official'. Looking at the controls available in Image Asse Studio (which I wasn't aware of before) they look identical to the ones in the 'approved, not official' one.

OK, cool.

Would you please address my comments? These are still open:

The commit message should say we're making this change, though, and why :-)

there are about three different things apparently happening here, and the commit message is super confusing: ... It's hard to tell that from this commit message.

The summary line says "Replace default icon", which I think describes something a previous version of this change did but this version doesn't.

About the Replace default icon... I think it is worded correctly - previously the default icon was round, and now it is not. The added round icon is used only when the settings of the launcher are for a round icon, previously.

Still, I am rewording it to 'replace default icon's shape'

Updated the commit message.

Thanks for this update!

I still found this commit message confusing, so I went and looked things up myself. What source were you relying on for your understanding of the guidelines? I don't think this is correct.

Since Android 7.0 the launcher supports rounded icons.

I think you're referring to this feature, which is in 7.1:
https://developer.android.com/about/versions/nougat/android-7.1#circular-icons

That contains this instruction:

Because of this, apps should make sure to define both android:icon and android:roundIcon resources when responding to launcher intents.

But I don't see android:roundIcon in the manifest.

And in fact when I test this on Android 8.1 (on an emulated Pixel 2 XL), it looks wrong:

I'm going to go ahead and bump targetSdkLevel without merging this. That'll give a result a bit like YouTube's icon in that latter screenshot, which isn't the end of the world and I think is better than this version. When we have a working version of this ready, we can merge it to get back to our current round icon.

In fact, it turns out that the status quo is friendlier than we thought!

Here's what I wrote as a low-priority item on #3075 :

Empirically, on my device on stock Android 9, our icon with 19.0.100 shows up perfectly fine in the launcher the same as it always has, but maybe some launchers that prefer square icons will behave less well? Also the debug icon gets adjusted and looks less good, but that's a very low priority.

At some point we should check how a recent version (19.2.102, released today to Play Store prod, has the targetSdkVersion upgrade) looks on some other launchers, and might find some adjustments are needed and circle back to this area. In the meanwhile I'll close this PR; I think that will end up being a separate bit of work.