feat(overlay): support custom offsets #1660
Conversation
googlebot
added
the
cla: yes
| this._offsetX = offset.x; | ||
| this._offsetY = offset.y; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
Thinking about it more, what do you think of having separate methods for x and y? As it is now, you have to specify both of them.
There was a problem hiding this comment.
Yeah, I like that. withOffsetX and withOffsetY, I'm guessing?
kara
force-pushed
the
overlay-offset
branch
3 times, most recently
from
c2ef9e6 to
9180afc
Compare
|
@jelbourn Comment addressed |
|
|
||
| overlayConfig.direction = this.dir; | ||
|
|
||
| return overlayConfig; | ||
| } | ||
|
|
||
| /** Sets up the position strategy with its initial configuration. */ | ||
| private _setUpPosition(): ConnectedPositionStrategy { |
There was a problem hiding this comment.
It's a bit odd that there's getPosition and setUpPosition. Combine them into one method like _createPositionStrategy?
private _createPositionStrategy() {
let pos = this.positions[0];
let originPoint = {originX: pos.originX, originY: pos.originY};
let overlayPoint = {overlayX: pos.overlayX, overlayY: pos.overlayY};
return this._overlay.position()
.connectedTo(this.origin.elementRef, originPoint, overlayPoint)
.withDirection(this.dir)
.withOffsetX(this.offsetX)
.withOffsetY(this.offsetY);
}There was a problem hiding this comment.
I also just noticed that it should be originX in the first argument, not overlayX.
There was a problem hiding this comment.
Also that this.positions[0] can be pulled out.
| @@ -89,11 +95,23 @@ export class ConnectedPositionStrategy implements PositionStrategy { | |||
| } | |||
|
|
|||
| /** Sets the layout direction so the overlay's position can be adjusted to match. */ | |||
| setDirection(dir: 'ltr' | 'rtl') { | |||
| withDirection(dir: 'ltr' | 'rtl'): ConnectedPositionStrategy { | |||
There was a problem hiding this comment.
The return type should actually just be this (which is a cool but little-known TS feature)
|
@jelbourn Okay, ready for another pass. |
|
LGTM |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This PR adds support for custom offsets for overlays. This will pave the way for correctly aligning the selected option over the trigger in the md-select component.
Fixes #879
r: @jelbourn