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

feat!: Use single 1024x1024 icon #1309

Closed
wants to merge 1 commit into from
Closed

feat!: Use single 1024x1024 icon #1309

wants to merge 1 commit into from

Conversation

jcesarmobile
Copy link
Member

@jcesarmobile jcesarmobile commented Apr 14, 2023

Xcode 14 allows to use a single 1024x1024 icon instead of having all the icon sizes.

It's breaking because it requires Xcode 14, but since Xcode 14.1 is going to be required for app store submissions starting the 25 of April, I think we are good.

Removed the icon sizes link because it was broken

Closes #1019
closes #1233

@dpogue
Copy link
Member

dpogue commented Apr 14, 2023

We should make sure this works properly if someone just puts <icon src="res/ios/icon.png" /> in their config.xml (with no sizes specified).

There's a part of me that thinks it would be good to have some support for customizing the notification and watch icons, but that would involve dynamically adding them to the .xcassets Contents.json only if they were specified, which feels like it would be error-prone if people are mixing manual changes and automatic ones 😞

@codecov-commenter
Copy link

codecov-commenter commented Apr 14, 2023

Codecov Report

Merging #1309 (1b6a41b) into master (3d6c71a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1309   +/-   ##
=======================================
  Coverage   78.48%   78.48%           
=======================================
  Files          15       15           
  Lines        1780     1780           
=======================================
  Hits         1397     1397           
  Misses        383      383           
Impacted Files Coverage Δ
lib/prepare.js 85.02% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dpogue dpogue added this to the 7.0.0 milestone Apr 14, 2023
@jcesarmobile
Copy link
Member Author

If using single icon, Xcode only allows a single icon, doesn’t allow to add a different icon notifications.
It allows a different icon for watchOS, but at the moment it was using the same as for iOS

@dpogue
Copy link
Member

dpogue commented Apr 15, 2023

Ahh okay

@dpogue
Copy link
Member

dpogue commented Apr 15, 2023

Note: Closes #1019 & closes #1233

@breautek
Copy link
Contributor

breautek commented May 3, 2023

It's also breaking in the sense it requires iOS 12+

Screenshot 2023-05-03 at 9 48 57 AM

So I think we also need to bump CordovaLib min target to iOS 12, which doesn't have to be part of this PR, but it will need to be set for this feature.

@breautek
Copy link
Contributor

FWIW, we've gone back in our apps and just supplied the icons that were missing (so that we aren't relying on a feature that may not even make it into release).

This appears to cover all current icons, at least for the iPhone models.

@erisu
Copy link
Member

erisu commented Jun 6, 2023

I believe it was decided to staying on iOS 11, so I think this PR can be closed.
Is a nice idea though. Maybe in a future release.

@jcesarmobile jcesarmobile deleted the single-icon branch June 6, 2023 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants