Skip to content

Commit

Permalink
fix: improve rounding (#715)
Browse files Browse the repository at this point in the history
  • Loading branch information
atomiks authored and FezVrasta committed Nov 17, 2018
1 parent 2d4400b commit 44be8de
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 9 deletions.
17 changes: 8 additions & 9 deletions packages/popper/src/modifiers/computeStyle.js
Expand Up @@ -2,6 +2,10 @@ import getSupportedPropertyName from '../utils/getSupportedPropertyName';
import find from '../utils/find';
import getOffsetParent from '../utils/getOffsetParent';
import getBoundingClientRect from '../utils/getBoundingClientRect';
import getRoundedOffsets from '../utils/getRoundedOffsets';
import isBrowser from '../utils/isBrowser';

const isFirefox = isBrowser && /Firefox/i.test(navigator.userAgent);

/**
* @function
Expand Down Expand Up @@ -37,15 +41,10 @@ export default function computeStyle(data, options) {
position: popper.position,
};

// Avoid blurry text by using full pixel integers.
// For pixel-perfect positioning, top/bottom prefers rounded
// values, while left/right prefers floored values.
const offsets = {
left: Math.floor(popper.left),
top: Math.round(popper.top),
bottom: Math.round(popper.bottom),
right: Math.floor(popper.right),
};
const offsets = getRoundedOffsets(
data,
window.devicePixelRatio < 2 || !isFirefox
);

const sideA = x === 'bottom' ? 'top' : 'bottom';
const sideB = y === 'right' ? 'left' : 'right';
Expand Down
46 changes: 46 additions & 0 deletions packages/popper/src/utils/getRoundedOffsets.js
@@ -0,0 +1,46 @@
/**
* @function
* @memberof Popper.Utils
* @argument {Object} data - The data object generated by `update` method
* @argument {Boolean} shouldRound - If the offsets should be rounded at all
* @returns {Object} The popper's position offsets rounded
*
* The tale of pixel-perfect positioning. It's still not 100% perfect, but as
* good as it can be within reason.
* Discussion here: https://github.com/FezVrasta/popper.js/pull/715
*
* Low DPI screens cause a popper to be blurry if not using full pixels (Safari
* as well on High DPI screens).
*
* Firefox prefers no rounding for positioning and does not have blurriness on
* high DPI screens.
*
* Only horizontal placement and left/right values need to be considered.
*/
export default function getRoundedOffsets(data, shouldRound) {
const { popper, reference } = data.offsets;

const isVertical = ['left', 'right'].indexOf(data.placement) !== -1;
const isVariation = data.placement.indexOf('-') !== -1;
const sameWidthOddness = reference.width % 2 === popper.width % 2;
const bothOddWidth = reference.width % 2 === 1 && popper.width % 2 === 1;
const noRound = v => v;

const horizontalToInteger = !shouldRound
? noRound
: isVertical || isVariation || sameWidthOddness
? Math.round
: Math.floor;
const verticalToInteger = !shouldRound ? noRound : Math.round;

return {
left: horizontalToInteger(
bothOddWidth && !isVariation && shouldRound
? popper.left - 1
: popper.left
),
top: verticalToInteger(popper.top),
bottom: verticalToInteger(popper.bottom),
right: horizontalToInteger(popper.right),
};
}
120 changes: 120 additions & 0 deletions packages/popper/tests/unit/getRoundedOffsets.js
@@ -0,0 +1,120 @@
import getRoundedOffsets from '../../src/utils/getRoundedOffsets';
import placements from '../../src/methods/placements';

const TOP = 200;
const BOTTOM = 300;
const EVEN_SIZE = { width: 20, height: 20, top: TOP, bottom: BOTTOM };
const ODD_SIZE = { width: 21, height: 21, top: TOP, bottom: BOTTOM };
const ROUNDS_UP = { left: 18.57127, right: 38.57127 };
const ROUNDED_UP = { left: 19, right: 39 };
const ROUNDED_DOWN = { left: 18, right: 38 };
const ALL_SIZE_COMBINATIONS = [
[EVEN_SIZE, EVEN_SIZE],
[EVEN_SIZE, ODD_SIZE],
[ODD_SIZE, EVEN_SIZE],
[ODD_SIZE, ODD_SIZE],
];
const variationPlacements = placements.filter(
placement => placement.indexOf('-') !== -1
);

describe('utils/getRoundedOffsets', () => {
it('Math.round()s when both popper and reference have even width', () => {
const offsets = getRoundedOffsets({
placement: 'bottom',
offsets: {
popper: { ...EVEN_SIZE, ...ROUNDS_UP },
reference: EVEN_SIZE,
},
}, true);
expect(offsets.left).toBe(ROUNDED_UP.left);
expect(offsets.right).toBe(ROUNDED_UP.right);
expect(offsets.top).toBe(TOP);
expect(offsets.bottom).toBe(BOTTOM);
});

it('Math.floor()s when popper and reference have a difference in width oddness', () => {
const offsets1 = getRoundedOffsets({
placement: 'bottom',
offsets: {
popper: { ...EVEN_SIZE, ...ROUNDS_UP },
reference: ODD_SIZE,
},
}, true);
const offsets2 = getRoundedOffsets({
placement: 'bottom',
offsets: {
popper: { ...ODD_SIZE, ...ROUNDS_UP },
reference: EVEN_SIZE,
},
}, true);
[offsets1, offsets2].forEach(offsets => {
expect(offsets.left).toBe(ROUNDED_DOWN.left);
expect(offsets.right).toBe(ROUNDED_DOWN.right);
expect(offsets.top).toBe(TOP);
expect(offsets.bottom).toBe(BOTTOM);
})
});

it('Math.rounds()s and subtracts 1 from left offset if both are odd in width', () => {
const offsets = getRoundedOffsets({
placement: 'bottom',
offsets: {
popper: { ...ODD_SIZE, ...ROUNDS_UP },
reference: ODD_SIZE,
},
}, true);
expect(offsets.left).toBe(ROUNDED_UP.left - 1);
expect(offsets.right).toBe(ROUNDED_UP.right);
expect(offsets.top).toBe(TOP);
expect(offsets.bottom).toBe(BOTTOM);
});

it('always Math.round()s variation placements', () => {
variationPlacements.forEach(placement => {
ALL_SIZE_COMBINATIONS.forEach(([popperSize, referenceSize]) => {
const offsets = getRoundedOffsets({
placement,
offsets: {
popper: { ...popperSize, ...ROUNDS_UP },
reference: referenceSize,
},
}, true);
expect(offsets.left).toBe(ROUNDED_UP.left);
expect(offsets.right).toBe(ROUNDED_UP.right);
expect(offsets.top).toBe(TOP);
expect(offsets.bottom).toBe(BOTTOM);
});
});
});

it('always Math.round()s vertical offsets', () => {
ALL_SIZE_COMBINATIONS.forEach(([popperSize, referenceSize]) => {
const offsets = getRoundedOffsets({
placement: 'bottom',
offsets: {
popper: { ...popperSize, ...ROUNDS_UP, top: 218.6, bottom: 318.6 },
reference: referenceSize,
},
}, true);
expect(offsets.top).toBe(219);
expect(offsets.bottom).toBe(319);
});
});

it('does not integerize the offsets if second argument is `false`', () => {
ALL_SIZE_COMBINATIONS.forEach(([popperSize, referenceSize]) => {
const offsets = getRoundedOffsets({
placement: 'bottom',
offsets: {
popper: { ...popperSize, ...ROUNDS_UP, top: 218.6, bottom: 318.6 },
reference: referenceSize,
},
}, false);
expect(offsets.left).toBe(ROUNDS_UP.left);
expect(offsets.right).toBe(ROUNDS_UP.right);
expect(offsets.top).toBe(218.6);
expect(offsets.bottom).toBe(318.6);
});
});
});

0 comments on commit 44be8de

Please sign in to comment.