From 42bc5743677ea0d0aeb2611b8df78a2dd4884a36 Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Tue, 8 Nov 2016 19:53:21 -0800 Subject: [PATCH 1/4] fix(overlay): prevent blurry connected overlays --- src/lib/core/overlay/position/connected-position-strategy.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lib/core/overlay/position/connected-position-strategy.ts b/src/lib/core/overlay/position/connected-position-strategy.ts index 0668cb799d6f..b87988a5914d 100644 --- a/src/lib/core/overlay/position/connected-position-strategy.ts +++ b/src/lib/core/overlay/position/connected-position-strategy.ts @@ -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. From 8f5c301a53a9f8a40c6d29984adc905c08788638 Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Wed, 16 Nov 2016 18:46:39 -0800 Subject: [PATCH 2/4] fix e2e test --- e2e/components/menu/menu-page.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/e2e/components/menu/menu-page.ts b/e2e/components/menu/menu-page.ts index 3938ad7e8a0a..d8d3f0ae1100 100644 --- a/e2e/components/menu/menu-page.ts +++ b/e2e/components/menu/menu-page.ts @@ -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)); }); } From c6dce11c066028b92a487e2d4b1bb7ffd2000522 Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Wed, 16 Nov 2016 19:12:54 -0800 Subject: [PATCH 3/4] fix unit tests --- src/lib/menu/menu.spec.ts | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 6afac6724276..52445bf6fc16 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -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) + .toEqual(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) + .toEqual(Math.round(triggerRect.top), `Expected menu top position to be unchanged if it can fit in the viewport.`); }); @@ -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) + .toEqual(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) + .toEqual(Math.round(triggerRect.left), `Expected menu x position to be unchanged if it can fit in the viewport.`); }); @@ -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) + .toEqual(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) + .toEqual(Math.round(expectedTop), `Expected menu to open in "above" position if "below" position wouldn't fit.`); }); @@ -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) + .toEqual(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) + .toEqual(Math.round(triggerRect.top), `Expected menu to open in "below" position if "above" position wouldn't fit.`); }); From 85739634f623d9b756df2e676c23733aee77cbea Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Thu, 17 Nov 2016 11:20:03 -0800 Subject: [PATCH 4/4] use toBe instead of toEqual --- src/lib/menu/menu.spec.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 52445bf6fc16..862578080607 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -156,12 +156,12 @@ describe('MdMenu', () => { // To find the overlay left, subtract the menu width from the origin's right side. const expectedLeft = triggerRect.right - overlayRect.width; expect(overlayRect.left) - .toEqual(Math.round(expectedLeft), + .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) - .toEqual(Math.round(triggerRect.top), + .toBe(Math.round(triggerRect.top), `Expected menu top position to be unchanged if it can fit in the viewport.`); }); @@ -185,12 +185,12 @@ describe('MdMenu', () => { // To find the overlay top, subtract the menu height from the origin's bottom edge. const expectedTop = triggerRect.bottom - overlayRect.height; expect(overlayRect.top) - .toEqual(Math.round(expectedTop), + .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) - .toEqual(Math.round(triggerRect.left), + .toBe(Math.round(triggerRect.left), `Expected menu x position to be unchanged if it can fit in the viewport.`); }); @@ -215,11 +215,11 @@ describe('MdMenu', () => { const expectedTop = triggerRect.bottom - overlayRect.height; expect(overlayRect.left) - .toEqual(Math.round(expectedLeft), + .toBe(Math.round(expectedLeft), `Expected menu to open in "before" position if "after" position wouldn't fit.`); expect(overlayRect.top) - .toEqual(Math.round(expectedTop), + .toBe(Math.round(expectedTop), `Expected menu to open in "above" position if "below" position wouldn't fit.`); }); @@ -237,13 +237,13 @@ 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) - .toEqual(Math.round(triggerRect.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) - .toEqual(Math.round(triggerRect.top), + .toBe(Math.round(triggerRect.top), `Expected menu to open in "below" position if "above" position wouldn't fit.`); });