Skip to content

Conversation

kara
Copy link
Contributor

@kara kara commented Oct 25, 2016

This PR passes all the overlay config properties through to the connected overlay directive.

r: @jelbourn

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 25, 2016
Copy link
Member

@jelbourn jelbourn 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 a couple of minor comments

}

set hasBackdrop(value: any) {
this._hasBackdrop = value === '' || (!!value && value !== 'false');
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was looking for something shared. But looks like that's not in master?

Copy link
Member

Choose a reason for hiding this comment

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

Not in master yet; I'd copy the logic and add a TODO

}

/** Event emitted when the backdrop is clicked. */
@Output() backdropClick = new EventEmitter();
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this require a generic type?

Copy link
Contributor Author

@kara kara Oct 25, 2016

Choose a reason for hiding this comment

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

Yeah, I think it does. It's not throwing, so maybe it's implicitly typed to EventEmitter<void> when set this way? Regardless I'll type it explicitly just in case.

@kara kara added the action: merge The PR is ready for merge by the caretaker label Oct 25, 2016
@kara kara merged commit 6f322cf into angular:master Oct 25, 2016
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants