Refactor scale via offsetHeight use precise API

offsetHeight returns an integer, making it a poor API to base the
determination of element scale on. Instead, use an alternative and
precise API. Only allow a reasonable amount of precision by decimal
place, and for cases where the scale number is reasonably rounded do so.

The offsetHeight method includes padding in returned sizes, where
getComputedStyle does not. Check them against each other, and if they
diverge signifcantly (>1) throw an exception.

To make ember-table itself compatible with this appraoch, pass the
`scale` value around the system a bit more instead of trying to derive
it in many places.

addon-test-support/helpers/element.js

@@ -1,9 +1,3 @@
-export function getScale(element) {
-  let rect = element.getBoundingClientRect();
+import { getScale } from 'ember-table/-private/utils/element';
 
-  if (element.offsetHeight === rect.height || rect.height === 0) {
-    return 1;
-  } else {
-    return element.offsetHeight / rect.height;
-  }
-}
+export { getScale };

addon-test-support/pages/-private/ember-table-header.js

@@ -139,7 +139,7 @@ const Header = PageObject.extend({
     /**
      * Below a certain number of steps, Hammer (the gesture library
      * which recognizes panning) will not pick up the pointermove
-     * events emitted by `mouseMove` before the gestrure completes.
+     * events emitted by `mouseMove` before the gesture completes.
      *
      * 5 seems a reasonable number.
      */

addon-test-support/pages/ember-table.js

@@ -65,6 +65,7 @@ export default PageObject.extend({
    * offsetWidth returns a rounded integer, and so can
    * result in unreliable tests.
    *
+   * @deprecated
    * @returns {number}
    */
   get width() {
@@ -73,17 +74,29 @@ export default PageObject.extend({
 
   /**
    * Retrieves the logical width of the table.
+   *
+   * @returns {number}
    */
   get logicalWidth() {
     return computedStyleInPixels(findElement(this, 'table'), 'width');
   },
 
+  /**
+   * Retrieves the rendered width of the table.
+   *
+   * @returns {number}
+   */
+  get renderedWidth() {
+    return findElement(this, 'table').getBoundingClientRect().width;
+  },
+
   /**
    * Returns the table container width.
    *
    * offsetWidth returns a rounded integer, and so can
    * result in unreliable tests.
    *
+   * @deprecated
    * @returns {number}
    */
   get containerWidth() {
@@ -92,11 +105,22 @@ export default PageObject.extend({
 
   /**
    * Retrieves the logical width of the container.
+   *
+   * @returns {number}
    */
   get logicalContainerWidth() {
     return computedStyleInPixels(findElement(this), 'width');
   },
 
+  /**
+   * Retrieves the rendered width of the container.
+   *
+   * @returns {number}
+   */
+  get renderedContainerWidth() {
+    return findElement(this).getBoundingClientRect().width;
+  },
+
   /**
    * Returns the specified scroll indicator element
    */

addon/-private/column-tree.js

@@ -665,7 +665,10 @@ export default EmberObject.extend({
   }),
 
   scrollBounds: computed('leftFixedNodes.@each.width', 'rightFixedNodes.@each.width', function() {
-    let { left: containerLeft, right: containerRight } = getInnerClientRect(this.container);
+    let { left: containerLeft, right: containerRight } = getInnerClientRect(
+      this.container,
+      this.scale
+    );
 
     containerLeft += get(this, 'leftFixedNodes').reduce((sum, n) => sum + get(n, 'width'), 0);
     containerRight -= get(this, 'rightFixedNodes').reduce((sum, n) => sum + get(n, 'width'), 0);
@@ -847,14 +850,16 @@ export default EmberObject.extend({
 
   getContainerWidth() {
     let containerWidthAdjustment = get(this, 'containerWidthAdjustment') || 0;
-    return getInnerClientRect(this.container).width * this.scale + containerWidthAdjustment;
+    return (
+      getInnerClientRect(this.container, this.scale).width * this.scale + containerWidthAdjustment
+    );
   },
 
   getReorderBounds(node) {
     let parent = get(node, 'parent');
     let { scale } = this;
     let { scrollLeft } = this.container;
-    let { left: containerLeft } = getInnerClientRect(this.container);
+    let { left: containerLeft } = getInnerClientRect(this.container, this.scale);
 
     let leftBound, rightBound, nodes;
 
@@ -903,7 +908,7 @@ export default EmberObject.extend({
 
   registerContainer(container) {
     this.container = container;
-    this.scale = getScale(container);
+    this.scale = getScale(container.querySelector('table'));
 
     get(this, 'root').registerElement(container);
 
@@ -918,7 +923,7 @@ export default EmberObject.extend({
     } else if (isFixed === 'right') {
       left += this.container.scrollWidth;
       left -= this.container.scrollLeft;
-      left -= getInnerClientRect(this.container).width * this.scale;
+      left -= getInnerClientRect(this.container, this.scale).width * this.scale;
     }
 
     let subcolumns = get(column.parent, 'subcolumnNodes');
@@ -945,7 +950,7 @@ export default EmberObject.extend({
     } else if (isFixed === 'right') {
       offsetLeft -= this.container.scrollWidth;
       offsetLeft += this.container.scrollLeft;
-      offsetLeft += getInnerClientRect(this.container).width * this.scale;
+      offsetLeft += getInnerClientRect(this.container, this.scale).width * this.scale;
     }
 
     return offsetLeft;
@@ -973,8 +978,18 @@ export default EmberObject.extend({
 
     let bounds = this.getReorderBounds(node);
 
-    this._reorderMainIndicator = new MainIndicator(this.container, node.element, bounds);
-    this._reorderDropIndicator = new DropIndicator(this.container, node.element, bounds);
+    this._reorderMainIndicator = new MainIndicator(
+      this.container,
+      this.scale,
+      node.element,
+      bounds
+    );
+    this._reorderDropIndicator = new DropIndicator(
+      this.container,
+      this.scale,
+      node.element,
+      bounds
+    );
 
     this.container.classList.add('is-reordering');
   },
@@ -991,7 +1006,7 @@ export default EmberObject.extend({
 
   _updateReorder(node) {
     let { scrollLeft } = this.container;
-    let realContainerLeft = getInnerClientRect(this.container).left * this.scale;
+    let realContainerLeft = getInnerClientRect(this.container, this.scale).left * this.scale;
     let offset = this.clientX * this.scale - realContainerLeft + scrollLeft;
 
     let width = get(node, 'width');
@@ -1013,7 +1028,7 @@ export default EmberObject.extend({
 
   endReorder(node) {
     let { scrollLeft } = this.container;
-    let realContainerLeft = getInnerClientRect(this.container).left * this.scale;
+    let realContainerLeft = getInnerClientRect(this.container, this.scale).left * this.scale;
     let offset = this.clientX * this.scale - realContainerLeft + scrollLeft;
 
     let { leftBound, rightBound } = this.getReorderBounds(node);

addon/-private/sticky/table-sticky-polyfill.js

@@ -1,3 +1,5 @@
+import { getScale } from '../utils/element';
+
 /* global ResizeSensor */
 /* eslint-disable ember/no-observers */
 
@@ -171,7 +173,7 @@ class TableStickyPolyfill {
    */
   repositionStickyElements = () => {
     let table = this.element.parentNode;
-    let scale = table.offsetHeight / table.getBoundingClientRect().height;
+    let scale = getScale(table);
     let containerHeight = table.parentNode.offsetHeight;
 
     // exclude ResizeSensor divs

addon/-private/utils/element.js

@@ -30,18 +30,72 @@ export function closest(el, selector) {
   return null;
 }
 
+/*
+ * Calculate the scale of difference between an element's logical height
+ * (the offsetHeight or getComputedStyle height) compared to its rendered
+ * height (via getBoundingClientRect).
+ *
+ * Note that there are some interesting edge cases to consider around
+ * these APIs.
+ *
+ *   - `getComputedStyle` returns a string from styles in pixels. For
+ *     example `48.23px`. The precision of this API in Chrome seems to
+ *     be 3 decimal places. Additionally, there can be unexpected values
+ *     such as `auto`. Finally, this API may be slow compared to other
+ *     DOM API calls.
+ *   - `offsetHeight` always returns in integer. If the height of an
+ *     element is at float precision, then the value is rounded. This
+ *     makes it at best useful as a fallback.
+ *   - `getComputedStyle` may return a smaller value than expected when
+ *     used with exotic kinds of styling (likely due to passing/broder
+ *     and block model or overflow issues?). In the ember table codebase
+ *     today, only the `table` tag itself is passed in for measurement.
+ *
+ */
 export function getScale(element) {
   let rect = element.getBoundingClientRect();
+  let renderedHeight = rect.height;
 
-  if (element.offsetHeight === rect.height || rect.height === 0) {
+  if (renderedHeight === 0) {
+    return 1;
+  }
+
+  let computedHeightStyleValue = window.getComputedStyle(element).height;
+  let computedHeightInPixels = Number(
+    computedHeightStyleValue.substring(0, computedHeightStyleValue.length - 2)
+  );
+
+  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."
+    );
+  }
+
+  if (computedHeightInPixels === renderedHeight) {
     return 1;
   } else {
-    return element.offsetHeight / rect.height;
+    let scale = computedHeightInPixels / renderedHeight;
+
+    /*
+     * If a scale is very close to an integer value, it should return an
+     * integer value instead of a float with deep precision.
+     *
+     * Allowing only 4 digits of precision is arbitrary.
+     */
+    let roundedScale = Math.round(scale);
+    if (Math.abs(roundedScale - scale) < 0.00001) {
+      return roundedScale;
+    }
+
+    return scale;
   }
 }
 
-export function getInnerClientRect(element) {
-  let scale = getScale(element);
+export function getInnerClientRect(element, scale) {
   let outerClientRect = element.getBoundingClientRect();
 
   let computedStyle = window.getComputedStyle(element);

addon/-private/utils/reorder-indicators.js

@@ -1,4 +1,4 @@
-import { getScale, getOuterClientRect, getInnerClientRect } from './element';
+import { getOuterClientRect, getInnerClientRect } from './element';
 
 function createElement(mainClass, dimensions) {
   let element = document.createElement('div');
@@ -13,25 +13,24 @@ function createElement(mainClass, dimensions) {
 }
 
 class ReorderIndicator {
-  constructor(container, element, bounds, mainClass, child) {
+  constructor(container, scale, element, bounds, mainClass, child) {
     this.container = container;
     this.element = element;
     this.bounds = bounds;
     this.child = child;
-    this.scale = getScale(container);
 
     let scrollTop = this.container.scrollTop;
     let scrollLeft = this.container.scrollLeft;
 
-    let { top: containerTop, left: containerLeft } = getInnerClientRect(this.container);
+    let { top: containerTop, left: containerLeft } = getInnerClientRect(this.container, scale);
 
     let { top: elementTop, left: elementLeft, width: elementWidth } = getOuterClientRect(
       this.element
     );
 
-    let top = (elementTop - containerTop) * this.scale + scrollTop;
-    let left = (elementLeft - containerLeft) * this.scale + scrollLeft;
-    let width = elementWidth * this.scale;
+    let top = (elementTop - containerTop) * scale + scrollTop;
+    let left = (elementLeft - containerLeft) * scale + scrollLeft;
+    let width = elementWidth * scale;
 
     this.originLeft = left;
     this.indicatorElement = createElement(mainClass, { top, left, width });
@@ -81,16 +80,15 @@ class ReorderIndicator {
 }
 
 export class MainIndicator extends ReorderIndicator {
-  constructor(container, element, bounds) {
-    // let width = getOuterClientRect(element).width * getScale(element);
+  constructor(container, scale, element, bounds) {
     let child = element.cloneNode(true);
 
-    super(container, element, bounds, 'et-reorder-main-indicator', child);
+    super(container, scale, element, bounds, 'et-reorder-main-indicator', child);
   }
 }
 
 export class DropIndicator extends ReorderIndicator {
-  constructor(container, element, bounds) {
-    super(container, element, bounds, 'et-reorder-drop-indicator');
+  constructor(container, scale, element, bounds) {
+    super(container, scale, element, bounds, 'et-reorder-drop-indicator');
   }
 }

tests/integration/components/headers/reorder-test.js

@@ -51,7 +51,7 @@ export async function scrollToEdge(targetElement, edgeOffset, direction) {
   /*
    * Below a certain number of steps, Hammer (the gesture library
    * which recognizes panning) will not pick up the pointermove
-   * events emitted by `mouseMove` before the gestrure completes.
+   * events emitted by `mouseMove` before the gesture completes.
    *
    * 5 seems a reasonable number.
    */

tests/unit/-private/element-utils-test.js

@@ -0,0 +1,60 @@
+import { module, test } from 'qunit';
+import { getScale } from 'ember-table/-private/utils/element';
+
+module('Unit | Private | element', function(hooks) {
+  hooks.beforeEach(function() {
+    /*
+     * Use an element outside the normal test harness as that harness
+     * uses scale() on the test container.
+     */
+    this.element = document.createElement('div');
+    document.body.append(this.element);
+  });
+
+  hooks.afterEach(function() {
+    this.element.remove();
+  });
+
+  test('can get the scale of a transformed element', function(assert) {
+    let div = document.createElement('div');
+    div.style.height = '4px';
+    this.element.append(div);
+
+    assert.equal(getScale(div), 1, 'scale on a simple element is correct');
+
+    div.style.transform = 'scale(0.5)';
+
+    assert.equal(getScale(div), 2, 'scale on a scaled element is correct');
+
+    div.style.height = '1.5px';
+
+    assert.equal(getScale(div), 2, 'scale on a scaled element is correct');
+  });
+
+  test('gets an integer when a scale is very close to its rounded integer value', function(assert) {
+    let div = document.createElement('div');
+    div.style.height = '4px';
+    this.element.append(div);
+
+    assert.equal(getScale(div), 1, 'scale on a simple element is correct');
+
+    div.style.transform = 'scale(1.000001)';
+
+    assert.equal(getScale(div), 1, 'scale on a scaled element is correct');
+
+    div.style.height = '1.5px';
+
+    assert.equal(getScale(div), 1, 'scale on a scaled element is correct');
+  });
+
+  test('throws if the height from getComputedStyle is diverged from offsetHeight', function(assert) {
+    let div = document.createElement('div');
+    div.textContent = 'aBc';
+    div.style.padding = '10px';
+    this.element.append(div);
+
+    assert.throws(() => {
+      getScale(div);
+    });
+  });
+});