From a1590ed03fcdb36ed528032b3b0ade230e5c9e76 Mon Sep 17 00:00:00 2001 From: Austin Joyner Date: Wed, 6 May 2020 15:19:45 -0400 Subject: [PATCH 1/2] Tests for scrollbar offset when working with scrollbar indicators. Additional test-support for calculated scrollbar width. --- addon-test-support/pages/ember-table.js | 8 ++ .../components/scroll-indicators-test.js | 125 ++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/addon-test-support/pages/ember-table.js b/addon-test-support/pages/ember-table.js index a5aa40a38..16fce2752 100644 --- a/addon-test-support/pages/ember-table.js +++ b/addon-test-support/pages/ember-table.js @@ -40,6 +40,14 @@ export default PageObject.extend({ return findElement(this, 'table').offsetWidth; }, + /** + * Gets the width of overflow scrollbar-y in pixels + */ + get scrollbarWidth() { + let scrollContainer = findElement(this, '[data-test-ember-table-overflow]'); + return scrollContainer.offsetWidth - scrollContainer.clientWidth; + }, + /** * Returns the table container width. */ diff --git a/tests/integration/components/scroll-indicators-test.js b/tests/integration/components/scroll-indicators-test.js index b6143fba4..377ac61db 100644 --- a/tests/integration/components/scroll-indicators-test.js +++ b/tests/integration/components/scroll-indicators-test.js @@ -118,5 +118,130 @@ module('Integration | scroll indicators', function() { ); assert.ok(isOffset('left', 100), 'left scroll indicator is offset'); }); + + test('right scroll indicators accounts for scrollbar when determining positioning', async function(assert) { + this.set('enableScrollIndicators', true); + await generateTable(this, { + columnCount: 30, + rowCount: 100, // add rows to get overflow-y + columnOptions: { + width: 100, + }, + }); + + /** + * Use a threshold instead of direct equality + * based on the test environment the window scrollbar size will vary due to scale/zoom + */ + function isOffsetAbove(side, minDistance) { + let element = table.scrollIndicator(side); + let calculatedDistance = Number(getComputedStyle(element)[side].replace('px', '')); + return calculatedDistance > minDistance; + } + + assert.equal( + table.isScrollIndicatorRendered('right'), + true, + 'right scroll indicator is initially shown' + ); + + assert.equal( + table.isScrollIndicatorRendered('left'), + false, + 'left scroll indicator is not initially shown' + ); + + // a little scroll + await scrollTo('[data-test-ember-table-overflow]', 150, 0); + + assert.equal( + table.isScrollIndicatorRendered('left'), + true, + 'left scroll indicator is shown after scrolling' + ); + + assert.ok( + isOffsetAbove('right', 0), + 'right scrollbar is offset by the width of the scrollbar' + ); + + // scroll to the end + await scrollTo('[data-test-ember-table-overflow]', 9999999, 0); + + assert.equal( + table.isScrollIndicatorRendered('right'), + false, + 'right scroll indicator is not shown at end of scroll' + ); + + assert.equal( + table.isScrollIndicatorRendered('left'), + true, + 'left scroll indicator is shown at end of scroll' + ); + }); + + test('right scroll indicator account for scrollbar in conjunction with frozen rows', async function(assert) { + this.set('enableScrollIndicators', true); + await generateTable(this, { + columnCount: 30, + rowCount: 357, + columnOptions: { + fixedLeftCount: 1, + fixedRightCount: 1, + width: 100, + }, + }); + + /** + * Use a threshold instead of direct equality + * based on the test environment the window scrollbar size will vary due to scale/zoom + */ + function isOffsetAbove(side, minDistance) { + let element = table.scrollIndicator(side); + let calculatedDistance = Number(getComputedStyle(element)[side].replace('px', '')); + return calculatedDistance > minDistance; + } + + assert.equal( + table.isScrollIndicatorRendered('right'), + true, + 'right scroll indicator is initially shown' + ); + + assert.equal( + table.isScrollIndicatorRendered('left'), + false, + 'left scroll indicator is not initially shown' + ); + + // a little scroll + await scrollTo('[data-test-ember-table-overflow]', 150, 0); + + assert.equal( + table.isScrollIndicatorRendered('left'), + true, + 'left scroll indicator is shown after scrolling' + ); + + assert.ok( + isOffsetAbove('right', 100), + 'right scrollbar is offset by the width of the scrollbar' + ); + + // scroll to the end + await scrollTo('[data-test-ember-table-overflow]', 9999999, 0); + + assert.equal( + table.isScrollIndicatorRendered('right'), + false, + 'right scroll indicator is not shown at end of scroll' + ); + assert.equal( + table.isScrollIndicatorRendered('left'), + true, + 'left scroll indicator is shown at end of scroll' + ); + }); }); }); From 4a9f5d3338d41479ac5db90213af2fcae52c9bdf Mon Sep 17 00:00:00 2001 From: Austin Joyner Date: Wed, 6 May 2020 15:20:25 -0400 Subject: [PATCH 2/2] Updates to ember-table to account for scrollbar-y offset when applying box shadow for scroll indicators. --- .../-private/scroll-indicators/component.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/addon/components/-private/scroll-indicators/component.js b/addon/components/-private/scroll-indicators/component.js index 418265301..d82defaec 100644 --- a/addon/components/-private/scroll-indicators/component.js +++ b/addon/components/-private/scroll-indicators/component.js @@ -7,6 +7,7 @@ import { htmlSafe } from '@ember/template'; import { isEmpty } from '@ember/utils'; import { addObserver } from 'ember-table/-private/utils/observer'; import layout from './template'; +import { getScale } from 'ember-table/-private/utils/element'; const indicatorStyle = side => { return computed( @@ -18,9 +19,12 @@ const indicatorStyle = side => { // left/right position let fixedNodes = this.get(`columnTree.${side}FixedNodes`); + let scrollbarOffsets = { left: 0, right: this.get('scrollbarOffset') }; if (!isEmpty(fixedNodes)) { let fixedWidth = fixedNodes.reduce((acc, node) => acc + node.get('width'), 0); - style.push(`${side}:${fixedWidth}px;`); + style.push(`${side}:${fixedWidth + scrollbarOffsets[side]}px;`); + } else { + style.push(`${side}:${scrollbarOffsets[side]}px;`); } // height @@ -86,18 +90,24 @@ export default Component.extend({ let scrollElement = this._getScrollElement(); let scrollRect = scrollElement.getBoundingClientRect(); let tableRect = scrollElement.querySelector('table').getBoundingClientRect(); + let tableScale = 1 / getScale(scrollElement); + let scrollbarOffset = (scrollElement.offsetWidth - scrollElement.clientWidth) * tableScale; + this.set('scale', tableScale); this.set('scrollRect', scrollRect); this.set('tableRect', tableRect); + this.set('scrollbarOffset', scrollbarOffset); }, _updateIndicatorShow() { this._setRects(); let scrollRect = this.get('scrollRect'); let tableRect = this.get('tableRect'); + let scrollbarOffset = this.get('scrollbarOffset'); + let xDiff = scrollRect.x - tableRect.x; let widthDiff = tableRect.width - scrollRect.width; this.set('showLeft', xDiff !== 0); - this.set('showRight', widthDiff > 0 && xDiff !== widthDiff); + this.set('showRight', widthDiff > 0 && xDiff - scrollbarOffset !== widthDiff); }, _updateListeners() {