Skip to content

Conversation

crisbeto
Copy link
Member

Currently we have a handful of components where we set the transform-origin depending on the position of their overlay. This ends up being a fair bit of similar logic that is scattered across the different components. These changes consolidate that logic into an option on the FlexibleConnectedPositionStrategy.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 15, 2018
@crisbeto crisbeto force-pushed the flexible-position-transform-origin branch from 770b747 to 3a71f1a Compare April 22, 2018 04:05
@mmalerba mmalerba removed their request for review April 24, 2018 23:16
* @param selector CSS selector that will be used to find the target
* elements onto which to set the transform origin.
*/
withTransformOriginOn(selector: string): this {
Copy link
Member

Choose a reason for hiding this comment

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

Add documentation for this in overlay.md?

@crisbeto crisbeto force-pushed the flexible-position-transform-origin branch from 3a71f1a to bffeaff Compare May 2, 2018 16:37
@crisbeto
Copy link
Member Author

crisbeto commented May 2, 2018

Sorry for the delay @jelbourn, it seems like I missed the notification. I've rebased the branch and have addressed the feedback.

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

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release and removed pr: needs review labels May 3, 2018
@ngbot
Copy link

ngbot bot commented May 4, 2018

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

1 similar comment
@ngbot
Copy link

ngbot bot commented May 4, 2018

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

…igin based on the current position

Currently we have a handful of components where we set the `transform-origin` depending on the position of their overlay. This ends up being a fair bit of similar logic that is scattered across the different components. These changes consolidate that logic into an option on the `FlexibleConnectedPositionStrategy`.
@crisbeto crisbeto force-pushed the flexible-position-transform-origin branch from bffeaff to 4883a56 Compare May 5, 2018 11:48
* @param delay Amount of milliseconds to the delay showing the tooltip.
*/
show(position: TooltipPosition, delay: number): void {
show(delay: number): void {
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking API change; causing some issues trying to sync

Copy link
Member Author

Choose a reason for hiding this comment

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

I can see how it could break somebody, but this is on a private component that is only exposed through an underscored property.

Copy link
Member

Choose a reason for hiding this comment

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

It's from a team that's extending the tooltip. I guess I can update them.

@jelbourn jelbourn merged commit d26735c into angular:master May 11, 2018
@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 8, 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 target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants