From 0b27c08aab8829f288e2e915b9020ff098efe7d3 Mon Sep 17 00:00:00 2001 From: Alex MacLeod Date: Thu, 18 Mar 2021 18:18:04 -0400 Subject: [PATCH 1/4] Re-fill when scrollbar appears --- addon/-private/column-tree.js | 16 +++++++++++++++- addon/-private/utils/element.js | 11 +++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/addon/-private/column-tree.js b/addon/-private/column-tree.js index 91961e099..fe428681b 100644 --- a/addon/-private/column-tree.js +++ b/addon/-private/column-tree.js @@ -676,6 +676,9 @@ export default EmberObject.extend({ return; } + // cache right scrollbar width so we can detect if it appears or disappears + this._scrollbarWidth = this.getScrollbarWidth(); + let leaves = get(this, 'root.leaves'); // ensures that min and max widths are respected _before_ `applyFillMode()` @@ -750,12 +753,19 @@ export default EmberObject.extend({ let contentWidth = get(this, 'root.contentWidth'); let delta = containerWidth - contentWidth; + // force re-fill when right scrollbar appears; this prevents the bottom + // scrollbar from unnecessarily showing when table becomes scrollable + let scrollbarWidth = this.getScrollbarWidth(); + let didScrollbarShow = scrollbarWidth > this._scrollbarWidth; + this._scrollbarWidth = scrollbarWidth; + if ( (widthConstraint === WIDTH_CONSTRAINT.EQ_CONTAINER && delta !== 0) || (widthConstraint === WIDTH_CONSTRAINT.EQ_CONTAINER_SLACK && delta !== 0) || (widthConstraint === WIDTH_CONSTRAINT.LTE_CONTAINER && delta < 0) || (widthConstraint === WIDTH_CONSTRAINT.GTE_CONTAINER && delta > 0) || - (widthConstraint === WIDTH_CONSTRAINT.GTE_CONTAINER_SLACK && delta > 0) + (widthConstraint === WIDTH_CONSTRAINT.GTE_CONTAINER_SLACK && delta > 0) || + didScrollbarShow ) { if (fillMode === FILL_MODE.EQUAL_COLUMN) { set(this, 'root.width', containerWidth); @@ -797,6 +807,10 @@ export default EmberObject.extend({ return getInnerClientRect(this.container).width * this.scale + containerWidthAdjustment; }, + getScrollbarWidth() { + return this.container.offsetWidth - this.container.clientWidth; + }, + getReorderBounds(node) { let parent = get(node, 'parent'); let { scale } = this; diff --git a/addon/-private/utils/element.js b/addon/-private/utils/element.js index cd5dce4cd..585e3dfc1 100644 --- a/addon/-private/utils/element.js +++ b/addon/-private/utils/element.js @@ -51,14 +51,17 @@ export function getInnerClientRect(element) { let borderLeftWidth = parseFloat(computedStyle.getPropertyValue('border-left-width')) / scale; let borderRightWidth = parseFloat(computedStyle.getPropertyValue('border-right-width')) / scale; + let scrollbarBottomHeight = element.offsetHeight - element.clientHeight; + let scrollbarRightWidth = element.offsetWidth - element.clientWidth; + return { top: outerClientRect.top + borderTopWidth, - bottom: outerClientRect.bottom - borderBottomWidth, + bottom: outerClientRect.bottom - borderBottomWidth - scrollbarBottomHeight, left: outerClientRect.left + borderLeftWidth, - right: outerClientRect.right - borderRightWidth, + right: outerClientRect.right - borderRightWidth - scrollbarRightWidth, - height: outerClientRect.height - borderTopWidth - borderBottomWidth, - width: outerClientRect.width - borderLeftWidth - borderRightWidth, + height: outerClientRect.height - borderTopWidth - borderBottomWidth - scrollbarBottomHeight, + width: outerClientRect.width - borderLeftWidth - borderRightWidth - scrollbarRightWidth, }; } From 238cfa7ba80b335195e71f793535415fdd59ef97 Mon Sep 17 00:00:00 2001 From: Alex MacLeod Date: Fri, 19 Mar 2021 12:34:58 -0400 Subject: [PATCH 2/4] Adjust content based on scrollbar delta --- addon/-private/column-tree.js | 99 +++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 46 deletions(-) diff --git a/addon/-private/column-tree.js b/addon/-private/column-tree.js index fe428681b..fd1eea0c6 100644 --- a/addon/-private/column-tree.js +++ b/addon/-private/column-tree.js @@ -695,7 +695,8 @@ export default EmberObject.extend({ let initialFillMode = get(this, 'initialFillMode'); if (isSlackModeEnabled && initialFillMode) { - this.applyFillMode(initialFillMode); + let containerWidth = this.getContainerWidth(); + this.applyFillMode(initialFillMode, containerWidth); } this.ensureWidthConstraint(); @@ -710,13 +711,38 @@ export default EmberObject.extend({ return; } - let isSlackModeEnabled = get(this, 'isSlackModeEnabled'); + let fillMode = get(this, 'fillMode'); + + // resize columns when right scrollbar appears or vanishes; this prevents + // the bottom scrollbar from showing unnecessarily + let scrollbarDelta = this.getScrollbarWidth() - this._scrollbarWidth; + if (scrollbarDelta !== 0) { + let targetWidth = get(this, 'root.contentWidth') - scrollbarDelta; + this.applyFillMode(fillMode, targetWidth); + } + this._scrollbarWidth += scrollbarDelta; + // if `widthConstraint` is set to a slack variety, fill excess space + // with a slack column before applying `fillMode` + let isSlackModeEnabled = get(this, 'isSlackModeEnabled'); if (isSlackModeEnabled) { this.updateSlackColumn(); } - this.applyFillMode(); + // only trigger fill mode if `widthConstraint` has been violated + let widthConstraint = get(this, 'widthConstraint'); + let contentWidth = get(this, 'root.contentWidth'); + let containerWidth = this.getContainerWidth(); + let delta = containerWidth - contentWidth; + if ( + (widthConstraint === WIDTH_CONSTRAINT.EQ_CONTAINER && delta !== 0) || + (widthConstraint === WIDTH_CONSTRAINT.EQ_CONTAINER_SLACK && delta !== 0) || + (widthConstraint === WIDTH_CONSTRAINT.LTE_CONTAINER && delta < 0) || + (widthConstraint === WIDTH_CONSTRAINT.GTE_CONTAINER && delta > 0) || + (widthConstraint === WIDTH_CONSTRAINT.GTE_CONTAINER_SLACK && delta > 0) + ) { + this.applyFillMode(fillMode, containerWidth); + } }, /** @@ -739,53 +765,34 @@ export default EmberObject.extend({ }, /** - Attempts to satisfy tree's width constraint by resizing columns according - to the specifid `fillMode`. If no `fillMode` is specified, the tree's - own `fillMode` property will be used. + Attempts to fit columns to container size by resizing columns using the + specified `fillMode` to fit in the specified target width. @param {String} fillMode + @param {Number} targetWidth */ - applyFillMode(fillMode) { - fillMode = fillMode || get(this, 'fillMode'); - - let widthConstraint = get(this, 'widthConstraint'); - let containerWidth = this.getContainerWidth(); + applyFillMode(fillMode, targetWidth) { let contentWidth = get(this, 'root.contentWidth'); - let delta = containerWidth - contentWidth; - - // force re-fill when right scrollbar appears; this prevents the bottom - // scrollbar from unnecessarily showing when table becomes scrollable - let scrollbarWidth = this.getScrollbarWidth(); - let didScrollbarShow = scrollbarWidth > this._scrollbarWidth; - this._scrollbarWidth = scrollbarWidth; - - if ( - (widthConstraint === WIDTH_CONSTRAINT.EQ_CONTAINER && delta !== 0) || - (widthConstraint === WIDTH_CONSTRAINT.EQ_CONTAINER_SLACK && delta !== 0) || - (widthConstraint === WIDTH_CONSTRAINT.LTE_CONTAINER && delta < 0) || - (widthConstraint === WIDTH_CONSTRAINT.GTE_CONTAINER && delta > 0) || - (widthConstraint === WIDTH_CONSTRAINT.GTE_CONTAINER_SLACK && delta > 0) || - didScrollbarShow - ) { - if (fillMode === FILL_MODE.EQUAL_COLUMN) { - set(this, 'root.width', containerWidth); - } else if (fillMode === FILL_MODE.FIRST_COLUMN) { - this.resizeColumn(0, delta); - } else if (fillMode === FILL_MODE.LAST_COLUMN) { - let isSlackModeEnabled = get(this, 'isSlackModeEnabled'); - let columns = get(this, 'root.subcolumnNodes'); - let lastColumnIndex = isSlackModeEnabled ? columns.length - 2 : columns.length - 1; - this.resizeColumn(lastColumnIndex, delta); - } else if (fillMode === FILL_MODE.NTH_COLUMN) { - let fillColumnIndex = get(this, 'fillColumnIndex'); - - assert( - "fillMode 'nth-column' must have a fillColumnIndex defined", - !isEmpty(fillColumnIndex) - ); - - this.resizeColumn(fillColumnIndex, delta); - } + let delta = targetWidth - contentWidth; + + if (fillMode === FILL_MODE.EQUAL_COLUMN) { + set(this, 'root.width', targetWidth); + } else if (fillMode === FILL_MODE.FIRST_COLUMN) { + this.resizeColumn(0, delta); + } else if (fillMode === FILL_MODE.LAST_COLUMN) { + let isSlackModeEnabled = get(this, 'isSlackModeEnabled'); + let columns = get(this, 'root.subcolumnNodes'); + let lastColumnIndex = isSlackModeEnabled ? columns.length - 2 : columns.length - 1; + this.resizeColumn(lastColumnIndex, delta); + } else if (fillMode === FILL_MODE.NTH_COLUMN) { + let fillColumnIndex = get(this, 'fillColumnIndex'); + + assert( + "fillMode 'nth-column' must have a fillColumnIndex defined", + !isEmpty(fillColumnIndex) + ); + + this.resizeColumn(fillColumnIndex, delta); } }, From 2422604d75accaa619b166984c58a322e09daa49 Mon Sep 17 00:00:00 2001 From: Alex MacLeod Date: Fri, 19 Mar 2021 13:45:37 -0400 Subject: [PATCH 3/4] Fix test --- .../components/scroll-indicators-test.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/integration/components/scroll-indicators-test.js b/tests/integration/components/scroll-indicators-test.js index d66420e45..3809bc592 100644 --- a/tests/integration/components/scroll-indicators-test.js +++ b/tests/integration/components/scroll-indicators-test.js @@ -134,12 +134,17 @@ module('Integration | scroll indicators', function() { }, }); + let firstHeader = table.headers.objectAt(0); + + // not necessarily 100px wide because of integer division + let lastHeader = table.headers.objectAt(table.headers.length - 1); + assert.equal( table.isScrollIndicatorRendered('right'), true, 'right scroll indicator is initially shown' ); - assert.ok(isOffset('right', 100), 'right scroll indicator is offset'); + assert.ok(isOffset('right', lastHeader.width), 'right scroll indicator is offset'); assert.equal( table.isScrollIndicatorRendered('left'), false, @@ -154,13 +159,13 @@ module('Integration | scroll indicators', function() { true, 'right scroll indicator is shown during partial scroll' ); - assert.ok(isOffset('right', 100), 'right scroll indicator is offset'); + assert.ok(isOffset('right', lastHeader.width), 'right scroll indicator is offset'); assert.equal( table.isScrollIndicatorRendered('left'), true, 'left scroll indicator is shown during partial scroll' ); - assert.ok(isOffset('left', 100), 'left scroll indicator is offset'); + assert.ok(isOffset('left', firstHeader.width), 'left scroll indicator is offset'); // scroll horizontally to the end await scrollTo('[data-test-ember-table-overflow]', SCROLL_MAX, 0); @@ -175,7 +180,7 @@ module('Integration | scroll indicators', function() { true, 'left scroll indicator is still shown at end of scroll' ); - assert.ok(isOffset('left', 100), 'left scroll indicator is offset'); + assert.ok(isOffset('left', firstHeader.width), 'left scroll indicator is offset'); }); test('top scroll indicator positioned below header', async function(assert) { From 1bc875f867da5d742b8670f2063fb20f30a19622 Mon Sep 17 00:00:00 2001 From: Alex MacLeod Date: Fri, 19 Mar 2021 18:27:47 -0400 Subject: [PATCH 4/4] Resize only when needed --- addon/-private/column-tree.js | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/addon/-private/column-tree.js b/addon/-private/column-tree.js index fd1eea0c6..923d3045a 100644 --- a/addon/-private/column-tree.js +++ b/addon/-private/column-tree.js @@ -713,13 +713,33 @@ export default EmberObject.extend({ let fillMode = get(this, 'fillMode'); - // resize columns when right scrollbar appears or vanishes; this prevents - // the bottom scrollbar from showing unnecessarily + // accommodate scrollbar as it appears/vanishes let scrollbarDelta = this.getScrollbarWidth() - this._scrollbarWidth; if (scrollbarDelta !== 0) { - let targetWidth = get(this, 'root.contentWidth') - scrollbarDelta; - this.applyFillMode(fillMode, targetWidth); + let treeWidth = get(this, 'root.width'); + let contentWidth = get(this, 'root.contentWidth'); + let containerWidth = this.getContainerWidth(); + let extra = containerWidth - contentWidth; + + // If `scrollbarDelta` is positive, it means the right scrollbar just + // appeared; in this case, we shrink the columns if they are within a + // scrollbar width of the container. This prevents a bottom scrollbar + // from appearing when it is not necessary. + + // If `scrollbarDelta` is negative, it means the right scrollbar just + // vanished; in this case, we enlarge if the rightmost column's edge is + // between one and two scrollbar widths of the container's edge. This + // ensures the inverse action to the above. + + if ( + (scrollbarDelta > 0 && (0 < -extra && -extra <= scrollbarDelta)) || + (scrollbarDelta < 0 && (-scrollbarDelta <= extra && extra < -2 * scrollbarDelta)) + ) { + let targetWidth = treeWidth - scrollbarDelta; + this.applyFillMode(fillMode, targetWidth); + } } + this._scrollbarWidth += scrollbarDelta; // if `widthConstraint` is set to a slack variety, fill excess space