Repository navigation
feat(overlay): add connected position strategy - #335
Conversation
googlebot
added
cla: yes
c72ed60 to
8bb60ad
Compare
|
|
||
|
|
||
| /** | ||
| * Gets the horizontal (x) "start"" dimension based on whether the overlay is in an RTL context. |
926d3ad to
7704250
Compare
|
Added tests for |
9777480 to
0a8bfc8
Compare
|
Tests are green now. Swapped the browser setup to use iOS8 instead of iOS7 |
0a8bfc8 to
efa3f5c
Compare
| x = originStartX + (originRect.width / 2); | ||
| } else { | ||
| x = pos.originX == 'start' ? originStartX : originEndX; | ||
| } |
There was a problem hiding this comment.
Nitpick: You could split this into _getOriginConnectionX and _getOriginConnectionY methods to make it a bit more focused.
| let value = transformValue.trim(); | ||
|
|
||
| element.style.transform = value; | ||
| element.style.webkitTransform = value; |
There was a problem hiding this comment.
Do we need anything for moz here or is that covered by transform?
|
Done reviewing. Just a few typos and questions, but looks good to me. @hansl, @robertmesserle ? |
| private _appViewManager: AppViewManager) { | ||
| private _appViewManager: AppViewManager, | ||
| private _positionBuilder: OverlayPositionBuilder) { | ||
| } |
There was a problem hiding this comment.
Single line for { } for consistency?
0b1fbe7 to
ff3f8c4
Compare
ff3f8c4 to
dd69a53
Compare
|
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. |
R: @hansl @robertmesserle @kara
I'm still missing tests for
ViewportRuler, but I wanted to get the review started while I work on those.(and also see if this passes on CI)