Skip to content

Commit

Permalink
Compare offsetHeight/getComputedStyle at scale
Browse files Browse the repository at this point in the history
At very large heights based on many child elements, Chrome and even
Firefox begin to diverge the computed height values from what you see at
lower scales. This makes their comparison with offsetHeight less
reliable at high scales.

However, at large height values the exactness of the height measurement
also matters less. At the end, only a limited level of precision is
desired in the scale value and later code throws away noise at the lower
end.

So, use a more forgiving comparison methodology (% shift in the change)
vs requiring the two values to be only a single digit off.
  • Loading branch information
mixonic committed Dec 15, 2023
1 parent 6efb39c commit c96b90c
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 7 deletions.
27 changes: 20 additions & 7 deletions addon/-private/utils/element.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ export function closest(el, selector) {
return null;
}

function parseComputedStyleHeightToPixels(computedHeightStyleValue) {
return Number(computedHeightStyleValue.substring(0, computedHeightStyleValue.length - 2));
}

/*
* Calculate the scale of difference between an element's logical height
* (the offsetHeight or getComputedStyle height) compared to its rendered
Expand Down Expand Up @@ -61,18 +65,27 @@ export function getScale(element) {
}

let computedHeightStyleValue = window.getComputedStyle(element).height;
let computedHeightInPixels = Number(
computedHeightStyleValue.substring(0, computedHeightStyleValue.length - 2)
);
let computedHeightInPixels = parseComputedStyleHeightToPixels(computedHeightStyleValue);

let offsetHeight = element.offsetHeight;

if (isNaN(computedHeightInPixels)) {
computedHeightInPixels = offsetHeight;
} else if (Math.abs(computedHeightInPixels - offsetHeight) >= 1) {
throw new Error(
"EmberTable's getScale() utility can only work on elements where height as derived from getComputedStyle is reliable. Usually this means padding is present on the target element."
);
} else {
let [min, max] = [computedHeightInPixels, offsetHeight].sort();
let difference = max - min;

/*
* This assertion once used a static comparison of the two
* values to make sure they were within 1 number of each
* other. Chrome has a bug when height is a number 500k
* that seems to require a wider range.
*/
if ((max <= 500000 && difference >= 1) || (max > 500000 && (max - min) / max > 0.00001)) {
throw new Error(
"EmberTable's getScale() utility can only work on elements where height as derived from getComputedStyle is reliable. This error flags that offsetHeight and getComputedStyle disagree on the target element dimensions. This can be caused by padding, thead elements, and other cases."
);
}
}

if (computedHeightInPixels === renderedHeight) {
Expand Down
92 changes: 92 additions & 0 deletions tests/unit/-private/element-utils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,96 @@ module('Unit | Private | element', function(hooks) {
getScale(div);
});
});

test('can get the scale of element with table header', function(assert) {
let table = document.createElement('table');
table.style.borderSpacing = '0';

let thead = document.createElement('thead');
table.append(thead);

let row = document.createElement('tr');
row.style.lineHeight = '8.5px';
row.style.height = '8.5px';
let cell = document.createElement('td');
cell.style.padding = '0px';
cell.textContent = 'header cell';
row.append(cell);
thead.append(row);

let tbody = document.createElement('tbody');
tbody.style.borderTopWidth = '1px';
tbody.style.borderTopStyle = 'solid';
table.append(tbody);

for (let i = 0; i < 50; i++) {
let row = document.createElement('tr');
row.style.lineHeight = '40px';
row.style.height = '40px';
let cell = document.createElement('td');
cell.style.padding = '0px';
cell.textContent = `cell ${i}`;
row.append(cell);
tbody.append(row);
}

this.element.append(table);

assert.equal(getScale(table), 1, 'scale on a simple element is correct');

table.style.transform = 'scale(0.5)';

assert.equal(getScale(table), 2, 'scale on a scaled element is correct');
});

/*
* For very large numbers of items and rows, Chrome seems to start to
* yield internally inconsistent values between computed height and
* offset height. Assert that we've covered that case.
*/
test('can get the scale of element with table header and extraordinary item count and height', function(assert) {
let table = document.createElement('table');
table.style.borderSpacing = '0';

let thead = document.createElement('thead');
table.append(thead);

let row = document.createElement('tr');
row.style.lineHeight = '1.5px';
row.style.height = '1.5px';
let cell = document.createElement('td');
cell.style.padding = '0px';
cell.textContent = 'header cell';
row.append(cell);
thead.append(row);

let tbody = document.createElement('tbody');
tbody.style.borderTopWidth = '1px';
tbody.style.borderTopStyle = 'solid';
table.append(tbody);

/*
* This pushes the height of the box above 7 digits, which
* seems to reliably trigger the internal bug is triggered.
* It has been observed on macOS at a lower value of 500k.
*/
for (let i = 0; i < 250; i++) {
let row = document.createElement('tr');
row.style.lineHeight = '4000px';
row.style.height = '4000px';
let cell = document.createElement('td');
cell.style.padding = '0px';
cell.textContent = `cell ${i}`;
row.append(cell);
tbody.append(row);
}

this.element.append(table);

assert.equal(getScale(table), 1, 'scale on a simple element is correct');

table.style.transform = 'scale(0.5)';

assert.equal(getScale(table), 2, 'scale on a scaled element is correct');
});
});

0 comments on commit c96b90c

Please sign in to comment.