fix(dialog): avoid subpixel rendering issues and refactor GlobalPosit…

…ionStrategy

Refactors the `GlobalPositionStrategy` to use flexbox for positioning. This avoids the subpixel rendering issues that come with using transforms for centering.

Fixes angular#932.

src/lib/core/overlay/_overlay.scss

@@ -8,10 +8,7 @@
 
   // TODO(jelbourn): change from the `md` prefix to something else for everything in the toolkit.
 
-  // The overlay-container is an invisible element which contains all individual overlays.
-  .md-overlay-container {
-    position: fixed;
-
+  .md-overlay-container, .md-global-overlay-wrapper {
     // Disable events from being captured on the overlay container.
     pointer-events: none;
 
@@ -20,9 +17,21 @@
     left: 0;
     height: 100%;
     width: 100%;
+  }
+
+  // The overlay-container is an invisible element which contains all individual overlays.
+  .md-overlay-container {
+    position: fixed;
     z-index: $md-z-index-overlay-container;
   }
 
+  // Wrapper element for the panes that use the GlobalPositionStrategy.
+  .md-global-overlay-wrapper {
+    display: flex;
+    position: absolute;
+    z-index: $md-z-index-overlay;
+  }
+
   // A single overlay pane.
   .md-overlay-pane {
     position: absolute;

src/lib/core/overlay/overlay-ref.ts

@@ -43,6 +43,12 @@ export class OverlayRef implements PortalHost {
   }
 
   dispose(): void {
+    let strategy = this._state.positionStrategy;
+
+    if (strategy && typeof strategy.dispose === 'function') {
+      strategy.dispose();
+    }
+
     this._detachBackdrop();
     this._portalHost.dispose();
   }

src/lib/core/overlay/position/global-position-strategy.spec.ts

@@ -13,28 +13,45 @@ describe('GlobalPositonStrategy', () => {
   beforeEach(() => {
     element = document.createElement('div');
     strategy = new GlobalPositionStrategy();
+    document.body.appendChild(element);
   });
 
-  it('should set explicit (top, left) position to the element', fakeAsyncTest(() => {
-    strategy.top('10px').left('40%').apply(element);
+  afterEach(() => {
+    strategy.dispose();
+  });
+
+  it('should position the element to the (top, left) with an offset', fakeAsyncTest(() => {
+    strategy.top('10px').left('40px').apply(element);
 
     flushMicrotasks();
 
-    expect(element.style.top).toBe('10px');
-    expect(element.style.left).toBe('40%');
-    expect(element.style.bottom).toBe('');
-    expect(element.style.right).toBe('');
+    let elementStyle = element.style;
+    let parentStyle = (element.parentNode as HTMLElement).style;
+
+    expect(elementStyle.marginTop).toBe('10px');
+    expect(elementStyle.marginLeft).toBe('40px');
+    expect(elementStyle.marginBottom).toBe('');
+    expect(elementStyle.marginRight).toBe('');
+
+    expect(parentStyle.justifyContent).toBe('flex-start');
+    expect(parentStyle.alignItems).toBe('flex-start');
   }));
 
-  it('should set explicit (bottom, right) position to the element', fakeAsyncTest(() => {
+  it('should position the element to the (bottom, right) with an offset', fakeAsyncTest(() => {
     strategy.bottom('70px').right('15em').apply(element);
 
     flushMicrotasks();
 
-    expect(element.style.top).toBe('');
-    expect(element.style.left).toBe('');
-    expect(element.style.bottom).toBe('70px');
-    expect(element.style.right).toBe('15em');
+    let elementStyle = element.style;
+    let parentStyle = (element.parentNode as HTMLElement).style;
+
+    expect(elementStyle.marginTop).toBe('');
+    expect(elementStyle.marginLeft).toBe('');
+    expect(elementStyle.marginBottom).toBe('70px');
+    expect(elementStyle.marginRight).toBe('15em');
+
+    expect(parentStyle.justifyContent).toBe('flex-end');
+    expect(parentStyle.alignItems).toBe('flex-end');
   }));
 
   it('should overwrite previously applied positioning', fakeAsyncTest(() => {
@@ -44,61 +61,85 @@ describe('GlobalPositonStrategy', () => {
     strategy.top('10px').left('40%').apply(element);
     flushMicrotasks();
 
-    expect(element.style.top).toBe('10px');
-    expect(element.style.left).toBe('40%');
-    expect(element.style.bottom).toBe('');
-    expect(element.style.right).toBe('');
-    expect(element.style.transform).not.toContain('translate');
+    let elementStyle = element.style;
+    let parentStyle = (element.parentNode as HTMLElement).style;
+
+    expect(elementStyle.marginTop).toBe('10px');
+    expect(elementStyle.marginLeft).toBe('40%');
+    expect(elementStyle.marginBottom).toBe('');
+    expect(elementStyle.marginRight).toBe('');
+
+    expect(parentStyle.justifyContent).toBe('flex-start');
+    expect(parentStyle.alignItems).toBe('flex-start');
 
     strategy.bottom('70px').right('15em').apply(element);
 
     flushMicrotasks();
 
-    expect(element.style.top).toBe('');
-    expect(element.style.left).toBe('');
-    expect(element.style.bottom).toBe('70px');
-    expect(element.style.right).toBe('15em');
-    expect(element.style.transform).not.toContain('translate');
+    expect(element.style.marginTop).toBe('');
+    expect(element.style.marginLeft).toBe('');
+    expect(element.style.marginBottom).toBe('70px');
+    expect(element.style.marginRight).toBe('15em');
+
+    expect(parentStyle.justifyContent).toBe('flex-end');
+    expect(parentStyle.alignItems).toBe('flex-end');
   }));
 
   it('should center the element', fakeAsyncTest(() => {
     strategy.centerHorizontally().centerVertically().apply(element);
 
     flushMicrotasks();
 
-    expect(element.style.top).toBe('50%');
-    expect(element.style.left).toBe('50%');
-    expect(element.style.transform).toContain('translateX(-50%)');
-    expect(element.style.transform).toContain('translateY(-50%)');
+    let parentStyle = (element.parentNode as HTMLElement).style;
+
+    expect(parentStyle.justifyContent).toBe('center');
+    expect(parentStyle.alignItems).toBe('center');
   }));
 
   it('should center the element with an offset', fakeAsyncTest(() => {
     strategy.centerHorizontally('10px').centerVertically('15px').apply(element);
 
     flushMicrotasks();
 
-    expect(element.style.top).toBe('50%');
-    expect(element.style.left).toBe('50%');
-    expect(element.style.transform).toContain('translateX(-50%)');
-    expect(element.style.transform).toContain('translateX(10px)');
-    expect(element.style.transform).toContain('translateY(-50%)');
-    expect(element.style.transform).toContain('translateY(15px)');
+    let elementStyle = element.style;
+    let parentStyle = (element.parentNode as HTMLElement).style;
+
+    expect(elementStyle.marginLeft).toBe('10px');
+    expect(elementStyle.marginTop).toBe('15px');
+
+    expect(parentStyle.justifyContent).toBe('center');
+    expect(parentStyle.alignItems).toBe('center');
+  }));
+
+  it('should make the element position: static', fakeAsyncTest(() => {
+    strategy.apply(element);
+
+    flushMicrotasks();
+
+    expect(element.style.position).toBe('static');
   }));
 
-  it('should default the element to position: absolute', fakeAsyncTest(() => {
+  it('should wrap the element in a `md-global-overlay-wrapper`', fakeAsyncTest(() => {
     strategy.apply(element);
 
     flushMicrotasks();
 
-    expect(element.style.position).toBe('absolute');
+    let parent = element.parentNode as HTMLElement;
+
+    expect(parent.classList.contains('md-global-overlay-wrapper')).toBe(true);
   }));
 
-  it('should make the element position: fixed', fakeAsyncTest(() => {
-    strategy.fixed().apply(element);
+
+  it('should remove the parent wrapper from the DOM', fakeAsync(() => {
+    strategy.apply(element);
 
     flushMicrotasks();
 
-    expect(element.style.position).toBe('fixed');
+    expect(document.body.contains(element.parentNode)).toBe(true);
+
+    strategy.dispose();
+
+    expect(document.body.contains(element.parentNode)).toBe(false);
   }));
 
   it('should set the element width', fakeAsync(() => {

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

@@ -1,67 +1,53 @@
-import {applyCssTransform} from '../../style/apply-transform';
 import {PositionStrategy} from './position-strategy';
 
 
 /**
  * A strategy for positioning overlays. Using this strategy, an overlay is given an
- * explicit position relative to the browser's viewport.
+ * explicit position relative to the browser's viewport. We use flexbox, instead of
+ * transforms, in order to avoid issues with subpixel rendering which can cause the
+ * element to become blurry.
  */
 export class GlobalPositionStrategy implements PositionStrategy {
-  private _cssPosition: string = 'absolute';
-  private _top: string = '';
-  private _bottom: string = '';
-  private _left: string = '';
-  private _right: string = '';
+  private _cssPosition: string = 'static';
+  private _topOffset: string = '';
+  private _bottomOffset: string = '';
+  private _leftOffset: string = '';
+  private _rightOffset: string = '';
+  private _alignItems: string = '';
+  private _justifyContent: string = '';
   private _width: string = '';
   private _height: string = '';
-
-  /** Array of individual applications of translateX(). Currently only for centering. */
-  private _translateX: string[] = [];
-
-  /** Array of individual applications of translateY(). Currently only for centering. */
-  private _translateY: string[] = [];
-
-  /** Sets the element to use CSS position: fixed */
-  fixed() {
-    this._cssPosition = 'fixed';
-    return this;
-  }
-
-  /** Sets the element to use CSS position: absolute. This is the default. */
-  absolute() {
-    this._cssPosition = 'absolute';
-    return this;
-  }
+  private _wrapper: HTMLElement;
 
   /** Sets the top position of the overlay. Clears any previously set vertical position. */
   top(value: string) {
-    this._bottom = '';
-    this._translateY = [];
-    this._top = value;
+    this._bottomOffset = '';
+    this._topOffset = value;
+    this._alignItems = 'flex-start';
     return this;
   }
 
   /** Sets the left position of the overlay. Clears any previously set horizontal position. */
   left(value: string) {
-    this._right = '';
-    this._translateX = [];
-    this._left = value;
+    this._rightOffset = '';
+    this._leftOffset = value;
+    this._justifyContent = 'flex-start';
     return this;
   }
 
   /** Sets the bottom position of the overlay. Clears any previously set vertical position. */
   bottom(value: string) {
-    this._top = '';
-    this._translateY = [];
-    this._bottom = value;
+    this._topOffset = '';
+    this._bottomOffset = value;
+    this._alignItems = 'flex-end';
     return this;
   }
 
   /** Sets the right position of the overlay. Clears any previously set horizontal position. */
   right(value: string) {
-    this._left = '';
-    this._translateX = [];
-    this._right = value;
+    this._leftOffset = '';
+    this._rightOffset = value;
+    this._justifyContent = 'flex-end';
     return this;
   }
 
@@ -96,14 +82,8 @@ export class GlobalPositionStrategy implements PositionStrategy {
    * Clears any previously set horizontal position.
    */
   centerHorizontally(offset = '') {
-    this._left = '50%';
-    this._right = '';
-    this._translateX = ['-50%'];
-
-    if (offset) {
-      this._translateX.push(offset);
-    }
-
+    this.left(offset);
+    this._justifyContent = 'center';
     return this;
   }
 
@@ -112,14 +92,8 @@ export class GlobalPositionStrategy implements PositionStrategy {
    * Clears any previously set vertical position.
    */
   centerVertically(offset = '') {
-    this._top = '50%';
-    this._bottom = '';
-    this._translateY = ['-50%'];
-
-    if (offset) {
-      this._translateY.push(offset);
-    }
-
+    this.top(offset);
+    this._alignItems = 'center';
     return this;
   }
 
@@ -128,26 +102,37 @@ export class GlobalPositionStrategy implements PositionStrategy {
    * TODO: internal
    */
   apply(element: HTMLElement): Promise<void> {
-    element.style.position = this._cssPosition;
-    element.style.top = this._top;
-    element.style.left = this._left;
-    element.style.bottom = this._bottom;
-    element.style.right = this._right;
-    element.style.width = this._width;
-    element.style.height = this._height;
+    if (!this._wrapper) {
+      this._wrapper = document.createElement('div');
+      this._wrapper.classList.add('md-global-overlay-wrapper');
+      element.parentNode.insertBefore(this._wrapper, element);
+      this._wrapper.appendChild(element);
+    }
 
-    // TODO(jelbourn): we don't want to always overwrite the transform property here,
-    // because it will need to be used for animations.
-    let translateX = this._reduceTranslateValues('translateX', this._translateX);
-    let translateY = this._reduceTranslateValues('translateY', this._translateY);
+    let styles = element.style;
+    let parentStyles = (element.parentNode as HTMLElement).style;
 
-    applyCssTransform(element, `${translateX} ${translateY}`);
+    styles.position = this._cssPosition;
+    styles.marginTop = this._topOffset;
+    styles.marginLeft = this._leftOffset;
+    styles.marginBottom = this._bottomOffset;
+    styles.marginRight = this._rightOffset;
+    styles.width = this._width;
+    styles.height = this._height;
+
+    parentStyles.justifyContent = this._justifyContent;
+    parentStyles.alignItems = this._alignItems;
 
     return Promise.resolve(null);
   }
 
-  /** Reduce a list of translate values to a string that can be used in the transform property */
-  private _reduceTranslateValues(translateFn: string, values: string[]) {
-    return values.map(t => `${translateFn}(${t})`).join(' ');
+  /**
+   * Removes the wrapper element from the DOM.
+   */
+  dispose(): void {
+    if (this._wrapper && this._wrapper.parentNode) {
+      this._wrapper.parentNode.removeChild(this._wrapper);
+      this._wrapper = null;
+    }
   }
 }