Skip to content
This repository was archived by the owner on Dec 4, 2017. It is now read-only.

docs(toh): Replaced window.history with location service #2439

Merged
merged 1 commit into from
Sep 26, 2016

Conversation

brandonroberts
Copy link
Contributor

Also updated references of "Component Router" to "Router"

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

LGTM aside from one minor change which I believe is unnecessary. See line comments.

@@ -38,7 +40,7 @@ export class HeroDetailComponent implements OnInit {

// #docregion goBack
goBack(): void {
window.history.back();
this.location.back();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice.

@@ -29,11 +31,11 @@ export class HeroDetailComponent implements OnInit {
// #docregion save
save(): void {
this.heroService.update(this.hero)
.then(this.goBack);
.then(() => this.goBack());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change? You are replacing a function g by an anonymous function which does nothing but call g. I.e., g and () => g() are equivalent, though the former is shorter.

Copy link
Contributor Author

@brandonroberts brandonroberts Sep 22, 2016

Choose a reason for hiding this comment

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

It throws an error of Cannot read property 'location' of nulll error when just providing the callback function when the e2e tests ran. I can use this.goBack.bind(this) instead of you think that would be cleaner.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, your change is actually the right thing to do to ensure that this gets bound properly in goBack.

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -29,11 +31,11 @@ export class HeroDetailComponent implements OnInit {
// #docregion save
save(): void {
this.heroService.update(this.hero)
.then(this.goBack);
.then(() => this.goBack());
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, your change is actually the right thing to do to ensure that this gets bound properly in goBack.

@wardbell wardbell merged commit 556e406 into angular:master Sep 26, 2016
@wardbell wardbell deleted the toh-location-service branch September 26, 2016 01:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants