Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve rounding #715

Merged
merged 6 commits into from Nov 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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);
});
});
});