Skip to content

Commit b5cc8e6

Browse files
committed
refactor(connected-position-strategy): use top/left instead of transforms for positioning
Switches the `ConnectedPositionStrategy` to use `top` and `left` for positioning, instead of `translateX` and `translateY`. This avoids having to do extra work to prevent subpixel rendering issues, while freeing up the transforms to be used on animations or for offsets.
1 parent cb3bb7a commit b5cc8e6

File tree

5 files changed

+30
-31
lines changed

5 files changed

+30
-31
lines changed

src/lib/core/overlay/overlay-directives.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ describe('Overlay directives', () => {
122122
// expected x value is the starting x + offset x
123123
const expectedX = startX + 5;
124124
const pane = overlayContainerElement.children[0] as HTMLElement;
125-
expect(pane.style.transform).toContain(`translateX(${expectedX}px)`);
125+
expect(pane.style.left).toBe(expectedX + 'px');
126126
});
127127

128128
it('should set the offsetY', () => {
@@ -138,7 +138,7 @@ describe('Overlay directives', () => {
138138
// expected y value is the starting y + trigger height + offset y
139139
// 30 + 20 + 45 = 95px
140140
const pane = overlayContainerElement.children[0] as HTMLElement;
141-
expect(pane.style.transform).toContain(`translateY(95px)`);
141+
expect(pane.style.top).toBe('95px');
142142
});
143143

144144
});

src/lib/core/overlay/position/connected-position-strategy.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {PositionStrategy} from './position-strategy';
22
import {ElementRef} from '@angular/core';
33
import {ViewportRuler} from './viewport-ruler';
4-
import {applyCssTransform} from '../../style/apply-transform';
54
import {
65
ConnectionPositionPair,
76
OriginConnectionPosition,
@@ -22,10 +21,10 @@ export class ConnectedPositionStrategy implements PositionStrategy {
2221
private _dir = 'ltr';
2322

2423
/** The offset in pixels for the overlay connection point on the x-axis */
25-
private _offsetX: number = 0;
24+
private _x: number = 0;
2625

2726
/** The offset in pixels for the overlay connection point on the y-axis */
28-
private _offsetY: number = 0;
27+
private _y: number = 0;
2928

3029
/** Whether the we're dealing with an RTL context */
3130
get _isRtl() {
@@ -112,13 +111,13 @@ export class ConnectedPositionStrategy implements PositionStrategy {
112111

113112
/** Sets an offset for the overlay's connection point on the x-axis */
114113
withOffsetX(offset: number): this {
115-
this._offsetX = offset;
114+
this._x = offset;
116115
return this;
117116
}
118117

119118
/** Sets an offset for the overlay's connection point on the y-axis */
120119
withOffsetY(offset: number): this {
121-
this._offsetY = offset;
120+
this._y = offset;
122121
return this;
123122
}
124123

@@ -196,8 +195,8 @@ export class ConnectedPositionStrategy implements PositionStrategy {
196195
}
197196

198197
return {
199-
x: originPoint.x + overlayStartX + this._offsetX,
200-
y: originPoint.y + overlayStartY + this._offsetY
198+
x: originPoint.x + overlayStartX + this._x,
199+
y: originPoint.y + overlayStartY + this._y
201200
};
202201
}
203202

@@ -227,18 +226,11 @@ export class ConnectedPositionStrategy implements PositionStrategy {
227226
* @param overlayPoint
228227
*/
229228
private _setElementPosition(element: HTMLElement, overlayPoint: Point) {
230-
// Round the values to prevent blurry overlays due to subpixel rendering.
231-
let x = Math.round(overlayPoint.x);
232-
let y = Math.round(overlayPoint.y);
233-
234-
// TODO(jelbourn): we don't want to always overwrite the transform property here,
235-
// because it will need to be used for animations.
236-
applyCssTransform(element, `translateX(${x}px) translateY(${y}px)`);
229+
element.style.left = overlayPoint.x + 'px';
230+
element.style.top = overlayPoint.y + 'px';
237231
}
238232
}
239233

240234

241235
/** A simple (x, y) coordinate. */
242236
type Point = {x: number, y: number};
243-
244-

src/lib/menu/menu-trigger.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
HorizontalConnectionPos,
2525
VerticalConnectionPos,
2626
} from '../core';
27-
import { Subscription } from 'rxjs/Subscription';
27+
import {Subscription} from 'rxjs/Subscription';
2828
import {MenuPositionX, MenuPositionY} from './menu-positions';
2929

3030
/**

src/lib/menu/menu.spec.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -148,19 +148,19 @@ describe('MdMenu', () => {
148148

149149
fixture.componentInstance.trigger.openMenu();
150150
fixture.detectChanges();
151-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
151+
const overlayPane = getOverlayPane();
152152
const triggerRect = trigger.getBoundingClientRect();
153153
const overlayRect = overlayPane.getBoundingClientRect();
154154

155155
// In "before" position, the right sides of the overlay and the origin are aligned.
156156
// To find the overlay left, subtract the menu width from the origin's right side.
157157
const expectedLeft = triggerRect.right - overlayRect.width;
158-
expect(overlayRect.left)
158+
expect(Math.round(overlayRect.left))
159159
.toBe(Math.round(expectedLeft),
160160
`Expected menu to open in "before" position if "after" position wouldn't fit.`);
161161

162162
// The y-position of the overlay should be unaffected, as it can already fit vertically
163-
expect(overlayRect.top)
163+
expect(Math.round(overlayRect.top))
164164
.toBe(Math.round(triggerRect.top),
165165
`Expected menu top position to be unchanged if it can fit in the viewport.`);
166166
});
@@ -177,19 +177,19 @@ describe('MdMenu', () => {
177177

178178
fixture.componentInstance.trigger.openMenu();
179179
fixture.detectChanges();
180-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
180+
const overlayPane = getOverlayPane();
181181
const triggerRect = trigger.getBoundingClientRect();
182182
const overlayRect = overlayPane.getBoundingClientRect();
183183

184184
// In "above" position, the bottom edges of the overlay and the origin are aligned.
185185
// To find the overlay top, subtract the menu height from the origin's bottom edge.
186186
const expectedTop = triggerRect.bottom - overlayRect.height;
187-
expect(overlayRect.top)
187+
expect(Math.round(overlayRect.top))
188188
.toBe(Math.round(expectedTop),
189189
`Expected menu to open in "above" position if "below" position wouldn't fit.`);
190190

191191
// The x-position of the overlay should be unaffected, as it can already fit horizontally
192-
expect(overlayRect.left)
192+
expect(Math.round(overlayRect.left))
193193
.toBe(Math.round(triggerRect.left),
194194
`Expected menu x position to be unchanged if it can fit in the viewport.`);
195195
});
@@ -207,18 +207,18 @@ describe('MdMenu', () => {
207207

208208
fixture.componentInstance.trigger.openMenu();
209209
fixture.detectChanges();
210-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
210+
const overlayPane = getOverlayPane();
211211
const triggerRect = trigger.getBoundingClientRect();
212212
const overlayRect = overlayPane.getBoundingClientRect();
213213

214214
const expectedLeft = triggerRect.right - overlayRect.width;
215215
const expectedTop = triggerRect.bottom - overlayRect.height;
216216

217-
expect(overlayRect.left)
217+
expect(Math.round(overlayRect.left))
218218
.toBe(Math.round(expectedLeft),
219219
`Expected menu to open in "before" position if "after" position wouldn't fit.`);
220220

221-
expect(overlayRect.top)
221+
expect(Math.round(overlayRect.top))
222222
.toBe(Math.round(expectedTop),
223223
`Expected menu to open in "above" position if "below" position wouldn't fit.`);
224224
});
@@ -230,23 +230,28 @@ describe('MdMenu', () => {
230230

231231
fixture.componentInstance.trigger.openMenu();
232232
fixture.detectChanges();
233-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
233+
const overlayPane = getOverlayPane();
234234
const triggerRect = trigger.getBoundingClientRect();
235235
const overlayRect = overlayPane.getBoundingClientRect();
236236

237237
// As designated "before" position won't fit on screen, the menu should fall back
238238
// to "after" mode, where the left sides of the overlay and trigger are aligned.
239-
expect(overlayRect.left)
239+
expect(Math.round(overlayRect.left))
240240
.toBe(Math.round(triggerRect.left),
241241
`Expected menu to open in "after" position if "before" position wouldn't fit.`);
242242

243243
// As designated "above" position won't fit on screen, the menu should fall back
244244
// to "below" mode, where the top edges of the overlay and trigger are aligned.
245-
expect(overlayRect.top)
245+
expect(Math.round(overlayRect.top))
246246
.toBe(Math.round(triggerRect.top),
247247
`Expected menu to open in "below" position if "above" position wouldn't fit.`);
248248
});
249249

250+
function getOverlayPane(): HTMLElement {
251+
let pane = overlayContainerElement.children[0] as HTMLElement;
252+
pane.style.position = 'absolute';
253+
return pane;
254+
}
250255
});
251256

252257
describe('animations', () => {

src/lib/select/select.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,8 @@ describe('MdSelect', () => {
527527
fixture.detectChanges();
528528

529529
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
530+
overlayPane.style.position = 'absolute';
531+
530532
const overlayRect = overlayPane.getBoundingClientRect();
531533
const triggerRect = trigger.getBoundingClientRect();
532534

0 commit comments

Comments
 (0)