Issue 305 - Create Dark and Tinted Icons for the App #319

rajhraval · 2024-10-03T04:45:38Z

What it Does

Closes FEATURE - Tinted App Icons for iOS 18 #305
The app supports dark mode and tinted app icons.

How I Tested

Run the app after following the instructions in Contributing.md
Go To Settings -> Change App Icon -> Pick an app icon
Close the App -> Tap and Hold on the Home Screen -> Click on Edit and then on Customise
Select Light To Test The Light Mode App Icon Variant
Select Dark To Test the Dark Mode App Icon Variant
Select Tinted to Test the Tinted App Icon Variant

Notes

In the Change App Icon View, the icons are not visible, this is not correct as user doesn't how the app icon looks like before the user applies it.

Screenshot

DemoAppICon.mov

…d code for app icon image preview

mikaelacaron

I tried to tap on the red car, still in light mode, and it failed with
Updating icon to Optional("AppIcon-car-red") failed.
actually they're all failing right now

Updating icon to Optional("AppIcon-car-dark") failed.
Updating icon to Optional("AppIcon-car-red") failed.
Updating icon to Optional("AppIcon-car-yellow") failed.
Updating icon to Optional("AppIcon-car-black") failed.
Updating icon to Optional("AppIcon-car-orange") failed.
Updating icon to Optional("AppIcon-car-yellow") failed.
Updating icon to Optional("AppIcon-car-black") failed.
Updating icon to Optional("AppIcon-car-orange") failed.

AppIcon-Car-Dark "dark version" of this doesn't have an icon, should it?

mikaelacaron · 2024-10-04T14:11:15Z

Basic-Car-Maintenance.xcodeproj/project.pbxproj

 				CODE_SIGN_ENTITLEMENTS = "Basic-Car-Maintenance/Basic_Car_Maintenance.entitlements";
 				CODE_SIGN_STYLE = Automatic;
 				DEAD_CODE_STRIPPING = YES;
 				DEVELOPMENT_ASSET_PATHS = "\"Basic-Car-Maintenance/Preview Content\"";
+				DEVELOPMENT_TEAM = SJ64B32FPH;


Don't commit this! look back at the contributing guidelines with the xccconfig file steps

it looks like you changed the development team in the project, which you shouldn't do

This still needs to be resolve dplease!

mikaelacaron · 2024-10-04T14:12:03Z

Basic-Car-Maintenance/Assets.xcassets/AppIcon-Car-Orange.appiconset/Tinted.png

Do we actually need tinted versions of all the colored cars? cause in reality they'll all be the same? so can we just use one asset for the tinted color versions?

In reality they are the same, but if I remove the tinted icons, it will kind of tint the light/dark variant which isn't a proper asset to apply tint.

Nevermind! Kinda irrelevant
I meant do we need multiple of the same asset for all the tinted icons? Or can we use the same asset for all of the colors? seeing Figam, it looks like it's the same, but I couldn't tell just looking at the git diff

(we can resolve this thread)

mikaelacaron · 2024-10-04T14:21:17Z

Be sure to merge your changes from dev into your branch!
Once you fix everything, please click the re-review button

rajhraval · 2024-10-04T15:31:23Z

I tried to tap on the red car, still in light mode, and it failed with Updating icon to Optional("AppIcon-car-red") failed. actually they're all failing right now
Updating icon to Optional("AppIcon-car-dark") failed.
Updating icon to Optional("AppIcon-car-red") failed.
Updating icon to Optional("AppIcon-car-yellow") failed.
Updating icon to Optional("AppIcon-car-black") failed.
Updating icon to Optional("AppIcon-car-orange") failed.
Updating icon to Optional("AppIcon-car-yellow") failed.
Updating icon to Optional("AppIcon-car-black") failed.
Updating icon to Optional("AppIcon-car-orange") failed.
AppIcon-Car-Dark "dark version" of this doesn't have an icon, should it?

This is weird, it's working fine for me while running simulator.
For AppIcon-Car-Dark, it's already dark should it doesn't require, actually I feel it's redundant.
Because AppIcon-Car-Black's dark variant is the AppIcon-Car-Dark icon itself.

mikaelacaron · 2024-10-04T15:43:52Z

For AppIcon-Car-Dark, it's already dark should it doesn't require, actually I feel it's redundant.

I agree, let's just remove it now, cause that was made pre-iOS 18

Do the icons show on the ChooseAppIconView for you? They still weren't showing for me

The errors can also mean the files aren't properly added to the project? And target membership?

rajhraval · 2024-10-04T16:02:43Z

I agree, let's just remove it now, cause that was made pre-iOS 18

Cool will remove the AppIconDark

Do the icons show on the ChooseAppIconView for you? They still weren't showing for me

No they don't because there are no images present for those icons, we need to have separate images created for them so UIImage(named: ) returns value currently they return a UIImage() (Empty Value)

The errors can also mean the files aren't properly added to the project? And target membership?
You are running in Simulator correct and iOS 18 right?, the files are added and target is also just fine.

mikaelacaron · 2024-10-04T16:02:45Z

comment whenever that is done please! cause you can't click re-review again haha

rajhraval · 2024-10-05T04:26:06Z

Done @mikaelacaron, regarding the errors, I still can't reproduce them.

mikaelacaron

I figured out why there's an error, see comments below! also for me the icons aren't showing on the ChooseAppIconView, but if you are seeing them, let me know.
don't fix it, we can make a new issue for this

Basic-Car-Maintenance/Shared/Models/AppIcon.swift

mikaelacaron · 2024-10-05T16:53:09Z

Basic-Car-Maintenance.xcodeproj/project.pbxproj

 				CODE_SIGN_ENTITLEMENTS = "Basic-Car-Maintenance/Basic_Car_Maintenance.entitlements";
 				CODE_SIGN_STYLE = Automatic;
 				DEAD_CODE_STRIPPING = YES;
 				DEVELOPMENT_ASSET_PATHS = "\"Basic-Car-Maintenance/Preview Content\"";
+				DEVELOPMENT_TEAM = SJ64B32FPH;


This still needs to be resolve dplease!

rajhraval · 2024-10-06T05:26:16Z

All done.

rajhraval · 2024-10-08T01:25:33Z

Any update @mikaelacaron ?

mikaelacaron

Great work!! the development team was removed and not properly set (see the contributing.md file about how to set this up properly. I'll fix it manually for this PR

also note I have multiple days to review PRs, there's many things to mange, and I may not get to every PR within 24 hours

mikaelacaron · 2024-10-08T14:04:41Z

Basic-Car-Maintenance.xcodeproj/project.pbxproj

 				DEAD_CODE_STRIPPING = YES;
 				DEVELOPMENT_ASSET_PATHS = "\"Basic-Car-Maintenance/Preview Content\"";
+				DEVELOPMENT_TEAM = "";
 				ENABLE_HARDENED_RUNTIME = YES;
 				ENABLE_PREVIEWS = YES;
 				GENERATE_INFOPLIST_FILE = YES;


Suggested change

DEAD_CODE_STRIPPING = YES;

DEVELOPMENT_ASSET_PATHS = "\"Basic-Car-Maintenance/Preview Content\"";

DEVELOPMENT_TEAM = "";

ENABLE_HARDENED_RUNTIME = YES;

ENABLE_PREVIEWS = YES;

GENERATE_INFOPLIST_FILE = YES;

DEAD_CODE_STRIPPING = YES;

DEVELOPMENT_ASSET_PATHS = "\"Basic-Car-Maintenance/Preview Content\"";

ENABLE_HARDENED_RUNTIME = YES;

ENABLE_PREVIEWS = YES;

GENERATE_INFOPLIST_FILE = YES;

This should be fully removed, as opposed to be set to empty

mikaelacaron · 2024-10-08T14:05:01Z

Basic-Car-Maintenance.xcodeproj/project.pbxproj

 				DEVELOPMENT_ASSET_PATHS = "\"Basic-Car-Maintenance/Preview Content\"";
+				DEVELOPMENT_TEAM = "";
 				ENABLE_HARDENED_RUNTIME = YES;
 				ENABLE_PREVIEWS = YES;


Suggested change

DEVELOPMENT_ASSET_PATHS = "\"Basic-Car-Maintenance/Preview Content\"";

DEVELOPMENT_TEAM = "";

ENABLE_HARDENED_RUNTIME = YES;

ENABLE_PREVIEWS = YES;

DEVELOPMENT_ASSET_PATHS = "\"Basic-Car-Maintenance/Preview Content\"";

ENABLE_HARDENED_RUNTIME = YES;

ENABLE_PREVIEWS = YES;

removed

It was set to none, but it shouldn't be

rajhraval added 4 commits October 2, 2024 11:53

Added app icons and fixed app icon preview image issue

6afe15f

Added Dark and Tinted Icons for Original Icon and reverted back to ol…

b3684d4

…d code for app icon image preview

Minor modifier change

c51ad29

Fill changes for icon

51a6f89

rajhraval requested a review from mikaelacaron as a code owner October 3, 2024 04:45

mikaelacaron requested changes Oct 4, 2024

View reviewed changes

rajhraval requested a review from mikaelacaron October 4, 2024 15:32

Removed App Icon Dark

4b4a650

mikaelacaron requested changes Oct 5, 2024

View reviewed changes

Removed Info

4b94b95

rajhraval requested a review from mikaelacaron October 7, 2024 03:23

mikaelacaron approved these changes Oct 8, 2024

View reviewed changes

Remove development team

3e3c863

It was set to none, but it shouldn't be

mikaelacaron merged commit 2ba41ae into mikaelacaron:dev Oct 8, 2024
1 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 305 - Create Dark and Tinted Icons for the App #319

Issue 305 - Create Dark and Tinted Icons for the App #319

rajhraval commented Oct 3, 2024

mikaelacaron left a comment

mikaelacaron Oct 4, 2024

mikaelacaron Oct 5, 2024

mikaelacaron Oct 4, 2024

rajhraval Oct 4, 2024 •

edited

Loading

mikaelacaron Oct 5, 2024

mikaelacaron commented Oct 4, 2024 •

edited

Loading

rajhraval commented Oct 4, 2024

mikaelacaron commented Oct 4, 2024

rajhraval commented Oct 4, 2024

mikaelacaron commented Oct 4, 2024

rajhraval commented Oct 5, 2024

mikaelacaron left a comment

mikaelacaron Oct 5, 2024

rajhraval commented Oct 6, 2024

rajhraval commented Oct 8, 2024 •

edited

Loading

mikaelacaron left a comment

mikaelacaron Oct 8, 2024

mikaelacaron Oct 8, 2024

Issue 305 - Create Dark and Tinted Icons for the App #319

Issue 305 - Create Dark and Tinted Icons for the App #319

Conversation

rajhraval commented Oct 3, 2024

What it Does

How I Tested

Notes

Screenshot

mikaelacaron left a comment

Choose a reason for hiding this comment

mikaelacaron Oct 4, 2024

Choose a reason for hiding this comment

mikaelacaron Oct 5, 2024

Choose a reason for hiding this comment

mikaelacaron Oct 4, 2024

Choose a reason for hiding this comment

rajhraval Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

mikaelacaron Oct 5, 2024

Choose a reason for hiding this comment

mikaelacaron commented Oct 4, 2024 • edited Loading

rajhraval commented Oct 4, 2024

mikaelacaron commented Oct 4, 2024

rajhraval commented Oct 4, 2024

mikaelacaron commented Oct 4, 2024

rajhraval commented Oct 5, 2024

mikaelacaron left a comment

Choose a reason for hiding this comment

mikaelacaron Oct 5, 2024

Choose a reason for hiding this comment

rajhraval commented Oct 6, 2024

rajhraval commented Oct 8, 2024 • edited Loading

mikaelacaron left a comment

Choose a reason for hiding this comment

mikaelacaron Oct 8, 2024

Choose a reason for hiding this comment

mikaelacaron Oct 8, 2024

Choose a reason for hiding this comment

rajhraval Oct 4, 2024 •

edited

Loading

mikaelacaron commented Oct 4, 2024 •

edited

Loading

rajhraval commented Oct 8, 2024 •

edited

Loading