Update to @angular 2.0.0-rc0 packages and latest CLI #384

Merged
merged 5 commits into from May 4, 2016

Projects

None yet

3 participants

@jelbourn
Member
jelbourn commented May 2, 2016

In-progress PR to run CI tests w/ new world.

Also I rewrote all of the checkbox tests to be dramatically cleaner.

@googlebot googlebot added the cla: yes label May 2, 2016
@jelbourn jelbourn changed the title from WIP: new world to WIP: new world 0.0.0-4 May 2, 2016
@jelbourn jelbourn changed the title from WIP: new world 0.0.0-4 to WIP: new world latest May 2, 2016
@kara kara and 1 other commented on an outdated diff May 2, 2016
src/components/checkbox/checkbox.spec.ts
- let el = fixture.debugElement.query(By.css('.md-checkbox'));
- expect(el.nativeElement.getAttribute('tabindex')).toEqual(String(newTabindex));
- });
+ it('should emit a change event when the `checked` value changes', () => {
+ // TODO(jelbourn): this *should* work with fakeAsync, but there seems to be a bug.
+ // As a short-term soltution, use a promise (which jasmine knows how to understand).
@kara
kara May 2, 2016 Collaborator

Typo: soltution -> solution

@kara kara and 1 other commented on an outdated diff May 2, 2016
src/components/checkbox/checkbox.ts
- this._indeterminate = false;
- this._checked = checked;
- this._transitionCheckState(
- this._checked ? TransitionCheckState.Checked : TransitionCheckState.Unchecked);
- this.change.emit(this._checked);
+ let wasAlreadyInitialized = this._isInitialized;
+ this._isInitialized = true;
+
+ if (checked != this.checked) {
+ this._indeterminate = false;
+ this._checked = checked;
+ this._transitionCheckState(
+ this._checked ? TransitionCheckState.Checked : TransitionCheckState.Unchecked);
+
+ // Only fire a change event if this isn't the first time the checked property is ever set.
+ if (wasAlreadyInitialized) {
@kara
kara May 2, 2016 Collaborator

Nitpick: It seems like we could remove the wasAlreadyInitialized variable if we move the setting of this._isInitialized = true to the end of the method. e.g.

if (this._isInitialized) {
   this.change.emit(this._checked);
}
this._isInitialized = true;

What do you think?

@jelbourn
jelbourn May 2, 2016 Member

Good catch, done.

@kara kara and 1 other commented on an outdated diff May 2, 2016
src/core/portal/portal.spec.ts
import {TemplatePortalDirective} from './portal-directives';
import {Portal} from './portal';
import {ComponentPortal} from './portal';
import {PortalHostDirective} from './portal-directives';
-import {fakeAsync} from 'angular2/testing';
-import {flushMicrotasks} from 'angular2/testing';
-import {DynamicComponentLoader} from 'angular2/core';
+import {fakeAsync} from '@angular/core/testing';
+import {flushMicrotasks} from '@angular/core/testing';
@kara
kara May 2, 2016 edited Collaborator

Looks like we can consolidate these two (fakeAsync, flushMicrotasks), since they are both @angular/core/testing

@kara kara and 1 other commented on an outdated diff May 2, 2016
src/core/portal/portal.spec.ts
import {TemplatePortalDirective} from './portal-directives';
import {Portal} from './portal';
import {ComponentPortal} from './portal';
import {PortalHostDirective} from './portal-directives';
-import {fakeAsync} from 'angular2/testing';
-import {flushMicrotasks} from 'angular2/testing';
-import {DynamicComponentLoader} from 'angular2/core';
+import {fakeAsync} from '@angular/core/testing';
+import {flushMicrotasks} from '@angular/core/testing';
+import {DynamicComponentLoader} from '@angular/core';
@kara
kara May 2, 2016 edited Collaborator

Might want to bundle with the @angular/core imports in line 9?

@kara kara commented on an outdated diff May 2, 2016
src/core/portal/portal.ts
@@ -1,15 +1,15 @@
-import {TemplateRef, Type, ViewContainerRef} from 'angular2/core';
-import {ElementRef} from 'angular2/core';
-import {ComponentRef} from 'angular2/core';
+import {TemplateRef, Type, ViewContainerRef} from '@angular/core';
+import {ElementRef} from '@angular/core';
+import {ComponentRef} from '@angular/core';
@kara
kara May 2, 2016 Collaborator

Bundle these three imports into one statement?

@kara
Collaborator
kara commented May 2, 2016

Left a few nitpicky comments about consolidating imports, typos, etc. But looks good to me otherwise.

@kara kara and 1 other commented on an outdated diff May 2, 2016
test/karma-test-shim.js
},
+ '@angular/core': {
@kara
kara May 2, 2016 Collaborator

Nitpick: we could probably build this object with a function and an array of bundle names to save the copy-pasting. But probably outside the scope of this PR

@jelbourn
jelbourn May 2, 2016 Member

Good observation, done.

@jelbourn jelbourn feat: update material to new @angular packages
35f270b
@kara kara added the pr: lgtm label May 2, 2016
jelbourn added some commits May 2, 2016
@jelbourn jelbourn remove old angular2 package
f8983fb
@jelbourn jelbourn bump to 2.0.0-rc.0
722a455
@jelbourn
Member
jelbourn commented May 3, 2016

@hansl Going to wait for new CLI version on top of new packages before submitting this.

@jelbourn
Member
jelbourn commented May 3, 2016

Also going to 2.0.0-rc.0 involved updating the router, which was a very simple change.

@laco0416 laco0416 referenced this pull request in laco0416/angular2-example May 4, 2016
Merged

feat(build): Update to rc #2

2 of 2 tasks complete
jelbourn added some commits May 4, 2016
@jelbourn jelbourn update to latest cli over rc0
ca1dbfa
@jelbourn jelbourn temporarily make e2e an allowed failure
0ecf408
@jelbourn jelbourn changed the title from WIP: new world latest to Update to @angular 2.0.0-rc0 packages and latest CLI May 4, 2016
@jelbourn jelbourn merged commit 04c8a1f into angular:master May 4, 2016

2 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jelbourn jelbourn deleted the jelbourn:newest-packages branch May 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment