Skip to content

Commit

Permalink
fix(dialog): avoid subpixel rendering issues and refactor GlobalPosit…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
crisbeto committed Nov 22, 2016
1 parent 3b80a6c commit 0db70a4
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 99 deletions.
17 changes: 13 additions & 4 deletions src/lib/core/overlay/_overlay.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions src/lib/core/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,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();
}
Expand Down
113 changes: 77 additions & 36 deletions src/lib/core/overlay/position/global-position-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -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);
}));
});

Expand Down
111 changes: 53 additions & 58 deletions src/lib/core/overlay/position/global-position-strategy.ts
Original file line number Diff line number Diff line change
@@ -1,87 +1,71 @@
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 = '';

/** 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 _cssPosition: string = 'static';
private _topOffset: string = '';
private _bottomOffset: string = '';
private _leftOffset: string = '';
private _rightOffset: string = '';
private _alignItems: string = '';
private _justifyContent: string = '';
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;
}

/**
* Centers the overlay horizontally with an optional offset.
* Clears any previously set horizontal position.
*/
centerHorizontally(offset = '0px') {
this._left = '50%';
this._right = '';
this._translateX = ['-50%', offset];
centerHorizontally(offset = '') {
this.left(offset);
this._justifyContent = 'center';
return this;
}

/**
* Centers the overlay vertically with an optional offset.
* Clears any previously set vertical position.
*/
centerVertically(offset = '0px') {
this._top = '50%';
this._bottom = '';
this._translateY = ['-50%', offset];
centerVertically(offset = '') {
this.top(offset);
this._alignItems = 'center';
return this;
}

Expand All @@ -90,24 +74,35 @@ 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;
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);
}

let styles = element.style;
let parentStyles = (element.parentNode as HTMLElement).style;

// TODO(jelbourn): we don't want to always overwrite the transform property here,
// because it will need to be used for animations.
let tranlateX = this._reduceTranslateValues('translateX', this._translateX);
let translateY = this._reduceTranslateValues('translateY', this._translateY);
styles.position = this._cssPosition;
styles.marginTop = this._topOffset;
styles.marginLeft = this._leftOffset;
styles.marginBottom = this._bottomOffset;
styles.marginRight = this._rightOffset;

applyCssTransform(element, `${tranlateX} ${translateY}`);
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;
}
}
}
3 changes: 3 additions & 0 deletions src/lib/core/overlay/position/position-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,7 @@ export interface PositionStrategy {

/** Updates the position of the overlay element. */
apply(element: Element): Promise<void>;

/** Cleans up any DOM modifications made by the position strategy, if necessary. */
dispose?(): void;
}
1 change: 0 additions & 1 deletion src/lib/snack-bar/snack-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ export class MdSnackBar {
private _createOverlay(): OverlayRef {
let state = new OverlayState();
state.positionStrategy = this._overlay.position().global()
.fixed()
.centerHorizontally()
.bottom('0');
return this._overlay.create(state);
Expand Down

0 comments on commit 0db70a4

Please sign in to comment.