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: update template for angular 9 #11852

Merged
merged 10 commits into from Aug 12, 2020
Merged

Conversation

janvennemann
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-27856

Optional Description:

Updated template for Titanium Angular with support for the new Webpack build pipeline and Angular 9.

Test case:

  1. Create a new Titanium Angular app with ti create --template angular-default or appc new --ng
  2. Build and run the app
  3. Verify that the example works. You should see a very simple app with a welcome screen shown on first launch. Tapping on the button opens a second screen, which is the main view of the app. The main screen shows a simple data binding example with a button and a label. Clicking the button will update the label text, and reset after 5 taps.

@build
Copy link
Contributor

build commented Jul 28, 2020

Fails
🚫 Tests have failed, see below for more information.
Messages
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖 ❌ 2 tests have failed There are 2 tests failing and 708 skipped out of 7990 total tests.

Tests:

ClassnameNameTimeError
android.emulator.Titanium.Apppause/resume events (9)5.466
Error: timeout of 5000ms exceeded
at Titanium.<anonymous> (/ti-mocha.js:6535:53120)
android.emulator.Titanium.UI.TextArea.focused (9)9.927
Error: timeout of 5000ms exceeded
at Titanium.<anonymous> (/ti-mocha.js:6535:53120)

Generated by 🚫 dangerJS against e43ef29

@@ -10,6 +21,15 @@ module.exports = {
'iphone/TitaniumKit/TitaniumKit/Sources/API/TopTiModule.m': [
'npm run ios-sanity-check --'
],
'!(**/locales/**/*).js': 'eslint',
'*.js': async files => {
const filtered = await asyncFilter(files, async file => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this was added. Won't eslint ignore the passed in file if it's in the .eslintignore file anyways?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and no. Before eslint will ignore a file, it tries to load all applicable config files. For files in the Angular template directory this would include the .eslintrc.js. This fails because it contains a plugin and parser which we don't have installed, which then fails the pre-commit hook.

templates/app/angular-default/hooks/post-create.js Outdated Show resolved Hide resolved
templates/app/angular-default/hooks/post-create.js Outdated Show resolved Hide resolved
@@ -2,6 +2,10 @@

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to comment on the image above, but looks to me like you removed the templates/app/angular-default/template/DefaultIcon.png which actually ha date Angular image and put back the red titanium logo? Was that intentional?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with templates/app/angular-default/template/src/assets/android/appicon.png below and android/templates/app/angular-default/template/src/assets/android/appicon.png above. So I assume this is intentional. Just curious why you wouldn't align the launch icons and the app icons.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't really happy with having only one logo. Previously it was only the Angular logo, but then our Titanium logo is completely missing which i didn't like. I then only used the Titanium logo, but then nothing indicated that this was an Angular app, which i also wasn't happy with. So i settled for the Titanium logo as the app icon, and the Angular logo as the launch logo.

Now that i'm thinking about it again, it maybe useful to have kind of a mashup logo that has both our Titanium Logo and the Angular logo in it. I'll ask marketing if they can come up with something.

@janvennemann
Copy link
Contributor Author

@ewanharris @sgtcoolguy addressed review comments and updated the PR to use a shared post-create hook from #11858

Copy link
Collaborator

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me. Should this land after #11858 now that it has a dependency on the post-create hook from that PR?

@janvennemann
Copy link
Contributor Author

@ewanharris Yup, that's the plan!

@sgtcoolguy sgtcoolguy merged commit fb5e200 into tidev:master Aug 12, 2020
@build
Copy link
Contributor

build commented Aug 12, 2020

The backport to 9_3_X failed:

The process 'git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Check out the target branch
git checkout 9_3_X
# Check out your branch
git checkout -b backport-11852-to-9_3_X
# Apply the commits from the PR
curl -s https://github.com/appcelerator/titanium_mobile/commit/a08973a1f61de3f2ee416d61a2277e4b4a58dd56.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/504412e86b35334316eb6faa2cb0dfafb0de8904.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/e1140ccc90d613ad12230674c772b7fe1fde1ef1.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/a11d097ccc233dc53b39b11cea7f9a2b37126bdf.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/57b9c0ffca74c83424626e7071d33fd2338b9d33.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/956a4ab3a082432d8db1a1ff557960f2d76cb443.patch | git am -3 --ignore-whitespace
# Push it to GitHub
git push --set-upstream origin backport-11852-to-9_3_X

Then, create a pull request where the base branch is 9_3_X and the compare/head branch is backport-11852-to-9_3_X.

sgtcoolguy pushed a commit that referenced this pull request Aug 12, 2020
* update readme and use shared post-create hook
* split dev dependencies
* unify app icon and launch image
* vscode settings

Fixes TIMOB-27856
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants