-
Notifications
You must be signed in to change notification settings - Fork 875
docs(toh): Replaced window.history with location service #2439
Conversation
There was a problem hiding this 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(); |
There was a problem hiding this comment.
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()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
.
There was a problem hiding this 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()); |
There was a problem hiding this comment.
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
.
Also updated references of "Component Router" to "Router"