Skip to content

Commit

Permalink
Ensure that the bottom/right cell borders are visible in the viewport…
Browse files Browse the repository at this point in the history
… after scrolling from the absolute top/absolute left table position. (#10887)

* - Compensate for the header border when API-scrolling from the absolute top/absolute left position of the table
- Modify the test cases to match the new scrollLeft/scrollTop values
- Add a test case ensuring the cell borders are visible after scrolling

* Add the changelog entry.

* Simplify and clean up some of the conditions.

* Correct the fresh test cases to the scroll values from this PR
  • Loading branch information
jansiegel committed Apr 3, 2024
1 parent bfb0e3a commit 53a190b
Show file tree
Hide file tree
Showing 18 changed files with 235 additions and 63 deletions.
8 changes: 8 additions & 0 deletions .changelogs/10887.json
@@ -0,0 +1,8 @@
{
"issuesOrigin": "private",
"title": "Fixed a problem where the bottom/right cell borders were not visible in the viewport after scrolling from the absolute top/left positions of the table using the API.",
"type": "fixed",
"issueOrPR": 10887,
"breaking": false,
"framework": "none"
}
23 changes: 21 additions & 2 deletions handsontable/src/3rdparty/walkontable/src/overlay/inlineStart.js
Expand Up @@ -2,6 +2,7 @@ import {
addClass,
getScrollbarWidth,
getScrollLeft,
getMaximumScrollLeft,
getWindowScrollTop,
hasClass,
outerWidth,
Expand Down Expand Up @@ -247,9 +248,17 @@ export class InlineStartOverlay extends Overlay {
* @returns {boolean}
*/
scrollTo(sourceCol, beyondRendered) {
let newX = this.getTableParentOffset();
const { wtSettings } = this;
const rowHeaders = wtSettings.getSetting('rowHeaders');
const fixedColumnsStart = wtSettings.getSetting('fixedColumnsStart');
const sourceInstance = this.wot.cloneSource ? this.wot.cloneSource : this.wot;
const mainHolder = sourceInstance.wtTable.holder;
const rowHeaderBorderCompensation = (
fixedColumnsStart === 0 &&
rowHeaders.length > 0 &&
!hasClass(mainHolder.parentNode, 'innerBorderInlineStart')
) ? 1 : 0;
let newX = this.getTableParentOffset();
let scrollbarCompensation = 0;

if (beyondRendered) {
Expand All @@ -264,17 +273,27 @@ export class InlineStartOverlay extends Overlay {
if (beyondRendered && mainHolder.offsetWidth !== mainHolder.clientWidth) {
scrollbarCompensation = getScrollbarWidth(this.domBindings.rootDocument);
}

if (beyondRendered) {
newX += this.sumCellSizes(0, sourceCol + 1);
newX -= this.wot.wtViewport.getViewportWidth();
// Compensate for the right header border if scrolled from the absolute left.
newX += rowHeaderBorderCompensation;

} else {
newX += this.sumCellSizes(this.wtSettings.getSetting('fixedColumnsStart'), sourceCol);
}

newX += scrollbarCompensation;

// If the table is scrolled all the way left when starting the scroll and going to be scrolled to the far right,
// we need to compensate for the potential header border width.
if (
getMaximumScrollLeft(this.mainTableScrollableElement) === newX - rowHeaderBorderCompensation &&
rowHeaderBorderCompensation > 0
) {
this.wot.wtOverlays.expandHiderHorizontallyBy(rowHeaderBorderCompensation);
}

return this.setScrollPosition(newX);
}

Expand Down
20 changes: 20 additions & 0 deletions handsontable/src/3rdparty/walkontable/src/overlay/top.js
@@ -1,5 +1,6 @@
import {
addClass,
getMaximumScrollTop,
getScrollbarWidth,
getScrollTop,
getWindowScrollLeft,
Expand Down Expand Up @@ -275,6 +276,13 @@ export class TopOverlay extends Overlay {
const { wot, wtSettings } = this;
const sourceInstance = wot.cloneSource ? wot.cloneSource : wot;
const mainHolder = sourceInstance.wtTable.holder;
const columnHeaders = wtSettings.getSetting('columnHeaders');
const fixedRowsTop = wtSettings.getSetting('fixedRowsTop');
const columnHeaderBorderCompensation = (
fixedRowsTop === 0 &&
columnHeaders.length > 0 &&
!hasClass(mainHolder.parentNode, 'innerBorderTop')
) ? 1 : 0;
let newY = this.getTableParentOffset();
let scrollbarCompensation = 0;

Expand All @@ -299,12 +307,24 @@ export class TopOverlay extends Overlay {
newY -= wot.wtViewport.getViewportHeight() - this.sumCellSizes(totalRows - fixedRowsBottom, totalRows);
// Fix 1 pixel offset when cell is selected
newY += 1;
// Compensate for the bottom header border if scrolled from the absolute top.
newY += columnHeaderBorderCompensation;

} else {
newY += this.sumCellSizes(wtSettings.getSetting('fixedRowsTop'), sourceRow);
}

newY += scrollbarCompensation;

// If the table is scrolled all the way up when starting the scroll and going to be scrolled to the bottom,
// we need to compensate for the potential header bottom border height.
if (
getMaximumScrollTop(this.mainTableScrollableElement) === newY - columnHeaderBorderCompensation &&
columnHeaderBorderCompensation > 0
) {
this.wot.wtOverlays.expandHiderVerticallyBy(columnHeaderBorderCompensation);
}

return this.setScrollPosition(newY);
}

Expand Down
47 changes: 44 additions & 3 deletions handsontable/src/3rdparty/walkontable/src/overlays.js
Expand Up @@ -632,14 +632,33 @@ class Overlays {
adjustElementsSize(force = false) {
const { wtViewport } = this.wot;
const { wtTable } = this;
const { rootWindow } = this.domBindings;
const isWindowScrolled = this.scrollableElement === rootWindow;
const totalColumns = this.wtSettings.getSetting('totalColumns');
const totalRows = this.wtSettings.getSetting('totalRows');
const headerRowSize = wtViewport.getRowHeaderWidth();
const headerColumnSize = wtViewport.getColumnHeaderHeight();
const hiderStyle = wtTable.hider.style;
const proposedHiderHeight = headerColumnSize + this.topOverlay.sumCellSizes(0, totalRows) + 1;
const proposedHiderWidth = headerRowSize + this.inlineStartOverlay.sumCellSizes(0, totalColumns);
const hiderElement = wtTable.hider;
const hiderStyle = hiderElement.style;
const isScrolledBeyondHiderHeight = () => {
return isWindowScrolled ?
false :
(this.scrollableElement.scrollTop > Math.max(0, proposedHiderHeight - wtTable.holder.clientHeight));
};
const isScrolledBeyondHiderWidth = () => {
return isWindowScrolled ?
false :
(this.scrollableElement.scrollLeft > Math.max(0, proposedHiderWidth - wtTable.holder.clientWidth));
};
const columnHeaderBorderCompensation = isScrolledBeyondHiderHeight() ? 1 : 0;
const rowHeaderBorderCompensation = isScrolledBeyondHiderWidth() ? 1 : 0;

hiderStyle.width = `${headerRowSize + this.inlineStartOverlay.sumCellSizes(0, totalColumns)}px`;
hiderStyle.height = `${headerColumnSize + this.topOverlay.sumCellSizes(0, totalRows) + 1}px`;
// If the elements are being adjusted after scrolling the table from the very beginning to the very end,
// we need to adjust the hider dimensions by the header border size. (https://github.com/handsontable/dev-handsontable/issues/1772)
hiderStyle.width = `${proposedHiderWidth + rowHeaderBorderCompensation}px`;
hiderStyle.height = `${proposedHiderHeight + columnHeaderBorderCompensation}px`;

if (this.scrollbarSize > 0) { // todo refactoring, looking as a part of logic which should be moved outside the class
const {
Expand All @@ -666,6 +685,28 @@ class Overlays {
this.bottomOverlay.adjustElementsSize(force);
}

/**
* Expand the hider vertically element by the provided delta value.
*
* @param {number} heightDelta The delta value to expand the hider element by.
*/
expandHiderVerticallyBy(heightDelta) {
const { wtTable } = this;

wtTable.hider.style.height = `${parseInt(wtTable.hider.style.height, 10) + heightDelta}px`;
}

/**
* Expand the hider horizontally element by the provided delta value.
*
* @param {number} widthDelta The delta value to expand the hider element by.
*/
expandHiderHorizontallyBy(widthDelta) {
const { wtTable } = this;

wtTable.hider.style.width = `${parseInt(wtTable.hider.style.width, 10) + widthDelta}px`;
}

/**
*
*/
Expand Down
Expand Up @@ -728,8 +728,8 @@ describe('WalkontableOverlay', () => {
const baseRect = getTableRect(wt.wtTable);

expect(baseRect).toEqual(jasmine.objectContaining({
top: documentClientHeight - totalRowsHeight,
bottom: documentClientHeight + 1, // +1 innerBorderTop
top: documentClientHeight - totalRowsHeight - 1, // 1px header border compensation
bottom: documentClientHeight,
left: documentClientWidth - totalColumnsWidth,
}));
expect(getTableRect(wt.wtOverlays.topOverlay.clone.wtTable)).toEqual(jasmine.objectContaining({
Expand All @@ -743,8 +743,8 @@ describe('WalkontableOverlay', () => {
left: 0,
}));
expect(getTableRect(wt.wtOverlays.inlineStartOverlay.clone.wtTable)).toEqual(jasmine.objectContaining({
top: documentClientHeight - totalRowsHeight,
bottom: documentClientHeight + 1, // +1 innerBorderTop
top: documentClientHeight - totalRowsHeight - 1, // 1px header border compensation
bottom: documentClientHeight,
left: 0,
}));
});
Expand Down
Expand Up @@ -180,9 +180,9 @@ describe('WalkontableScroll', () => {
const firstRow = getTableMaster().find('tbody tr:first');
const lastRow = getTableMaster().find('tbody tr:last');

expect(firstRow.find('td:first').text()).toBe('H1');
expect(firstRow.find('td:first').text()).toBe('I1');
expect(firstRow.find('td:last').text()).toBe('K1');
expect(lastRow.find('td:first').text()).toBe('H8');
expect(lastRow.find('td:first').text()).toBe('I8');
expect(lastRow.find('td:last').text()).toBe('K8');
});

Expand Down
Expand Up @@ -29,7 +29,8 @@ describe('Focus selection scroll', () => {
keyDownUp('enter');
keyDownUp('enter'); // B4

expect(topOverlay().getScrollPosition()).toBe(4);
// 92 row heights - 104 viewport width + 15 scrollbart offset + + 1 selection offset + 1 header border offset
expect(topOverlay().getScrollPosition()).toBe(5);

keyDownUp('enter'); // B5

Expand Down Expand Up @@ -58,7 +59,8 @@ describe('Focus selection scroll', () => {

keyDownUp(['shift', 'enter']); // B50

expect(topOverlay().getScrollPosition()).toBe(1062);
// 1150 row heights - 104 viewport width + 15 scrollbart offset + 1 header border offset
expect(topOverlay().getScrollPosition()).toBe(1063);
});

it('should scroll the viewport horizontally', () => {
Expand All @@ -77,7 +79,8 @@ describe('Focus selection scroll', () => {
keyDownUp('tab');
keyDownUp('tab'); // C2

expect(inlineStartOverlay().getScrollPosition()).toBe(15);
// 150 column widths - 150 viewport width + 15 scrollbart offset + 1 header border offset
expect(inlineStartOverlay().getScrollPosition()).toBe(16);

keyDownUp('tab'); // D2

Expand All @@ -102,6 +105,7 @@ describe('Focus selection scroll', () => {

keyDownUp(['shift', 'tab']); // E2

expect(inlineStartOverlay().getScrollPosition()).toBe(115);
// 250 column widths - 150 viewport width + 15 scrollbart offset + 1 header border offset
expect(inlineStartOverlay().getScrollPosition()).toBe(116);
});
});
20 changes: 20 additions & 0 deletions handsontable/src/helpers/dom/element.js
Expand Up @@ -639,6 +639,26 @@ export function getScrollableElement(element) {
return rootWindow;
}

/**
* Get the maximum available `scrollTop` value for the provided element.
*
* @param {HTMLElement} element The element to get the maximum scroll top value from.
* @returns {number} The maximum scroll top value.
*/
export function getMaximumScrollTop(element) {
return element.scrollHeight - element.clientHeight;
}

/**
* Get the maximum available `scrollLeft` value for the provided element.
*
* @param {HTMLElement} element The element to get the maximum scroll left value from.
* @returns {number} The maximum scroll left value.
*/
export function getMaximumScrollLeft(element) {
return element.scrollWidth - element.clientWidth;
}

/**
* Returns a DOM element responsible for trimming the provided element.
*
Expand Down
Expand Up @@ -283,8 +283,8 @@ describe('AutoRowSize', () => {

keyDownUp('enter');

expect(getInlineStartClone().find('.wtHolder').scrollTop()).toBe(89);
expect(getMaster().find('.wtHolder').scrollTop()).toBe(89);
expect(getInlineStartClone().find('.wtHolder').scrollTop()).toBe(90);
expect(getMaster().find('.wtHolder').scrollTop()).toBe(90);
});

it('should consider CSS style of each instance separately', () => {
Expand Down
Expand Up @@ -45,6 +45,7 @@ describe('Comments keyboard shortcut', () => {
data: createSpreadsheetData(500, 50),
width: 300,
height: 300,
colWidths: 50,
rowHeaders: true,
colHeaders: true,
comments: true,
Expand All @@ -71,8 +72,10 @@ describe('Comments keyboard shortcut', () => {
expect(editor.value).toBe('');
expect(document.activeElement).toBe(editor);
expect(plugin.range).toEqualCellRange('highlight: 400,40 from: 400,40 to: 400,40');

// 2050 column width - 250 viewport width + 15 scrollbar compensation + 1 header border compensation
expect(hot.view._wt.wtOverlays.inlineStartOverlay.getScrollPosition()).toBe(1816);
expect(hot.view._wt.wtOverlays.topOverlay.getScrollPosition()).toBe(8965);
expect(hot.view._wt.wtOverlays.topOverlay.getScrollPosition()).toBe(8966);
});

it('should open and edit a comment, make it active, and ready for typing', async() => {
Expand Down
Expand Up @@ -958,10 +958,11 @@ describe('DropdownMenu', () => {

await sleep(10);

expect(inlineStartOverlay().getScrollPosition()).toBe(650);
// 900 column width - 250 viewport width + 1 header border compensation
expect(inlineStartOverlay().getScrollPosition()).toBe(651);

dropdownMenu(6); // click on the column `G` header button

expect(inlineStartOverlay().getScrollPosition()).toBe(650);
expect(inlineStartOverlay().getScrollPosition()).toBe(651);
});
});
Expand Up @@ -1535,7 +1535,8 @@ describe('NestedHeaders', () => {
keyDownUp('arrowright'); // "C"

expect(topOverlay().getScrollPosition()).toBe(0);
expect(inlineStartOverlay().getScrollPosition()).toBe(65);
// 300 column width - 250 viewport width + 15 scrollbar compensation + 1 header border compensation
expect(inlineStartOverlay().getScrollPosition()).toBe(66);

keyDownUp('arrowright'); // "D"

Expand Down Expand Up @@ -1648,7 +1649,7 @@ describe('NestedHeaders', () => {
keyDownUp('arrowup'); // "C"

expect(topOverlay().getScrollPosition()).toBe(0);
expect(inlineStartOverlay().getScrollPosition()).toBe(50);
expect(inlineStartOverlay().getScrollPosition()).toBe(51);
});

it('should scroll the viewport correctly while navigating vertically using arrows ' +
Expand Down Expand Up @@ -1706,12 +1707,12 @@ describe('NestedHeaders', () => {
selectCell(-1, 20);

expect(topOverlay().getScrollPosition()).toBe(0);
expect(inlineStartOverlay().getScrollPosition()).toBe(800);
expect(inlineStartOverlay().getScrollPosition()).toBe(801);

keyDownUp('arrowup'); // "F"

expect(topOverlay().getScrollPosition()).toBe(0);
expect(inlineStartOverlay().getScrollPosition()).toBe(800);
expect(inlineStartOverlay().getScrollPosition()).toBe(801);
});
});

Expand Down
3 changes: 2 additions & 1 deletion handsontable/test/e2e/core/scrollToFocusedCell.spec.js
Expand Up @@ -146,7 +146,8 @@ describe('Core.scrollToFocusedCell', () => {

await sleep(10);

expect(inlineStartOverlay().getScrollPosition()).toBe(2265);
// 2500 column width - 250 viewport width + 15 scrollbar compensation + 1 header border compensation
expect(inlineStartOverlay().getScrollPosition()).toBe(2266);
expect(topOverlay().getScrollPosition()).toBe(5750);
});

Expand Down

0 comments on commit 53a190b

Please sign in to comment.