Skip to content

Commit

Permalink
fix(overlay): prevent blurry connected overlays (#1784)
Browse files Browse the repository at this point in the history
  • Loading branch information
jelbourn authored and kara committed Nov 17, 2016
1 parent c2597b6 commit 303dd69
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 20 deletions.
6 changes: 4 additions & 2 deletions e2e/components/menu/menu-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ export class MenuPage {

expectMenuLocation(el: ElementFinder, {x,y}: {x: number, y: number}) {
el.getLocation().then((loc) => {
expect(loc.x).toEqual(x);
expect(loc.y).toEqual(y);
// Round the values because we expect the menu overlay to also have been rounded
// to avoid blurriness due to subpixel rendering.
expect(loc.x).toEqual(Math.round(x));
expect(loc.y).toEqual(Math.round(y));
});
}

Expand Down
5 changes: 3 additions & 2 deletions src/lib/core/overlay/position/connected-position-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,9 @@ export class ConnectedPositionStrategy implements PositionStrategy {
* @param overlayPoint
*/
private _setElementPosition(element: HTMLElement, overlayPoint: Point) {
let x = overlayPoint.x;
let y = overlayPoint.y;
// Round the values to prevent blurry overlays due to subpixel rendering.
let x = Math.round(overlayPoint.x);
let y = Math.round(overlayPoint.y);

// TODO(jelbourn): we don't want to always overwrite the transform property here,
// because it will need to be used for animations.
Expand Down
32 changes: 16 additions & 16 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,13 @@ describe('MdMenu', () => {
// In "before" position, the right sides of the overlay and the origin are aligned.
// To find the overlay left, subtract the menu width from the origin's right side.
const expectedLeft = triggerRect.right - overlayRect.width;
expect(overlayRect.left.toFixed(2))
.toEqual(expectedLeft.toFixed(2),
expect(overlayRect.left)
.toBe(Math.round(expectedLeft),
`Expected menu to open in "before" position if "after" position wouldn't fit.`);

// The y-position of the overlay should be unaffected, as it can already fit vertically
expect(overlayRect.top.toFixed(2))
.toEqual(triggerRect.top.toFixed(2),
expect(overlayRect.top)
.toBe(Math.round(triggerRect.top),
`Expected menu top position to be unchanged if it can fit in the viewport.`);
});

Expand All @@ -184,13 +184,13 @@ describe('MdMenu', () => {
// In "above" position, the bottom edges of the overlay and the origin are aligned.
// To find the overlay top, subtract the menu height from the origin's bottom edge.
const expectedTop = triggerRect.bottom - overlayRect.height;
expect(overlayRect.top.toFixed(2))
.toEqual(expectedTop.toFixed(2),
expect(overlayRect.top)
.toBe(Math.round(expectedTop),
`Expected menu to open in "above" position if "below" position wouldn't fit.`);

// The x-position of the overlay should be unaffected, as it can already fit horizontally
expect(overlayRect.left.toFixed(2))
.toEqual(triggerRect.left.toFixed(2),
expect(overlayRect.left)
.toBe(Math.round(triggerRect.left),
`Expected menu x position to be unchanged if it can fit in the viewport.`);
});

Expand All @@ -214,12 +214,12 @@ describe('MdMenu', () => {
const expectedLeft = triggerRect.right - overlayRect.width;
const expectedTop = triggerRect.bottom - overlayRect.height;

expect(overlayRect.left.toFixed(2))
.toEqual(expectedLeft.toFixed(2),
expect(overlayRect.left)
.toBe(Math.round(expectedLeft),
`Expected menu to open in "before" position if "after" position wouldn't fit.`);

expect(overlayRect.top.toFixed(2))
.toEqual(expectedTop.toFixed(2),
expect(overlayRect.top)
.toBe(Math.round(expectedTop),
`Expected menu to open in "above" position if "below" position wouldn't fit.`);
});

Expand All @@ -236,14 +236,14 @@ describe('MdMenu', () => {

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

// As designated "above" position won't fit on screen, the menu should fall back
// to "below" mode, where the top edges of the overlay and trigger are aligned.
expect(overlayRect.top.toFixed(2))
.toEqual(triggerRect.top.toFixed(2),
expect(overlayRect.top)
.toBe(Math.round(triggerRect.top),
`Expected menu to open in "below" position if "above" position wouldn't fit.`);
});

Expand Down

0 comments on commit 303dd69

Please sign in to comment.