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

fix: fix the missing icons in newly created apps on iOS devices #4843

Merged
merged 1 commit into from
Jul 12, 2019

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Jul 12, 2019

Currently the icons of newly created apps are not displayed when running the application on iOS device. This happens as the icons are not included into .pbxproject file. This happens as the prepareProject method is not called due to this check https://github.com/NativeScript/nativescript-cli/blob/release/lib/services/platform/prepare-native-platform-service.ts#L35. This check is based on hasChangesRequirePrepare from projectChangesService which value is based on appResourcesChanged and signingChanged - https://github.com/NativeScript/nativescript-cli/blob/release/lib/services/project-changes-service.ts#L27. The values of those properties are not populated correctly initially when the application is without platforms directory.

This functionality was broken with this PR https://github.com/NativeScript/nativescript-cli/pull/4836/files. As this PR changes the behavior of this if, now it never passes and respectively appResourcesChanged and signingChanged are with false values. In other words, we've broken the functionality that set initial values of those properties.

In order to set correctly the initial values of those properties, we decided deleting the setNativePlatformStatus call after adding the platform. This functionally was added in this PR #4139. So here come the question how calling setNativePlatformStatus after adding the platform reflects the current issue with missing icons on device.

setNativePlatformStatus method sets the nativePlatformStatus property in _prepareInfo and saves it to .nsprepareinfo file which happens when adding the platform. After that, CLI calls ensurePrepareInfo method https://github.com/NativeScript/nativescript-cli/blob/release/lib/services/project-changes-service.ts#L160. As we've already saved prepareInfo to .nsprepareinfo file, this if always passes https://github.com/NativeScript/nativescript-cli/blob/release/lib/services/project-changes-service.ts#L162 and we've never reached the code that sets the default values here https://github.com/NativeScript/nativescript-cli/blob/release/lib/services/project-changes-service.ts#L183. This happens as we've already saved .nsprepareinfo from setNativePlatformStatus method. As calling setNativePlatformStatus after adding the platform was introduced as a bug fix when supporting legacy and bundle workflow, we decided to delete this call. This way, we'll ensure that initially when the project is without platforms directory, this code we'll be called https://github.com/NativeScript/nativescript-cli/blob/release/lib/services/project-changes-service.ts#L183 and the initial values of properties will be set correctly.

PR Checklist

What is the current behavior?

What is the new behavior?

Fixes issue #4842

@cla-bot cla-bot bot added the cla: yes label Jul 12, 2019
@Fatme Fatme force-pushed the fatme/remove-set-native-platform-status branch from ae9c2e9 to 423f77e Compare July 12, 2019 10:06
@rosen-vladimirov
Copy link
Contributor

test cli-smoke cli-device package_version#rc

@Fatme Fatme merged commit eedfd5e into release Jul 12, 2019
@Fatme Fatme deleted the fatme/remove-set-native-platform-status branch July 12, 2019 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants