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(infoWindow): emit event when closed #317

Closed
wants to merge 3 commits into from
Closed

feat(infoWindow): emit event when closed #317

wants to merge 3 commits into from

Conversation

alexweber
Copy link
Contributor

initial stab at #306

@sebholstein
Copy link
Owner

@alexweber thanks for the PR!
Can you format your code with gulp clang:format? Thanks

@@ -45,6 +45,11 @@ let infoWindowId = 0;
export class SebmGoogleMapInfoWindow implements OnDestroy,
OnChanges {
/**
* Emits an event when the info window is closed.
*/
@Output() infoWindowClosed = new EventEmitter();
Copy link
Owner

Choose a reason for hiding this comment

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

Event names should be in present tense, so infoWindowClose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SebastianM Thanks, done

@alexweber
Copy link
Contributor Author

@SebastianM I've ran the code through the formatter and some other files also got picked up, I believe I have this in another PR too but since this is the one you're looking at happy to get it in here.

Thanks!

@alexweber
Copy link
Contributor Author

@SebastianM Emitting the info Window itself in this version, we gotta emit a value as far as I know... let me know, thanks!

@@ -47,7 +47,7 @@ export class SebmGoogleMapInfoWindow implements OnDestroy,
/**
* Emits an event when the info window is closed.
*/
@Output() infoWindowClose = new EventEmitter();
@Output() infoWindowClose: EventEmitter<SebmGoogleMapInfoWindow> = new EventEmitter();
Copy link
Owner

@sebholstein sebholstein May 17, 2016

Choose a reason for hiding this comment

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

you can use EventEmitter<void> here, so you don't have to emit anything here

@sebholstein
Copy link
Owner

added one last comment. can you address this comment and squash your commit into one. than im happy to merge it. thanks man!

sebholstein pushed a commit that referenced this pull request May 19, 2016
@sebholstein
Copy link
Owner

@alexweber made the changes really quick - it's now merge. thank you

@alexweber
Copy link
Contributor Author

@SebastianM Awesome thanks you beat me to it :)

@alexweber alexweber deleted the feature-info-window-close-event-306 branch May 19, 2016 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants