Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not scroll when clicking cells (#7717) (CP: 24.4) #7722

Merged
merged 2 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/grid/src/vaadin-grid-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -600,8 +600,8 @@ export const GridMixin = (superClass) =>

// Patch `focus()` to use the button
cell._focusButton = div;
cell.focus = function () {
cell._focusButton.focus();
cell.focus = function (options) {
cell._focusButton.focus(options);
};

div.appendChild(slot);
Expand All @@ -624,7 +624,7 @@ export const GridMixin = (superClass) =>
// Only focus if mouse is released on cell content itself
const mouseUpWithinCell = event.composedPath().includes(cellContent);
if (!contentContainsFocusedElement && mouseUpWithinCell) {
cell.focus();
cell.focus({ preventScroll: true });
}
document.removeEventListener('mouseup', mouseUpListener, true);
};
Expand All @@ -634,7 +634,7 @@ export const GridMixin = (superClass) =>
// Watch out sync focus removal issue, only async focus works here.
setTimeout(() => {
if (!cellContent.contains(this.getRootNode().activeElement)) {
cell.focus();
cell.focus({ preventScroll: true });
}
});
}
Expand Down
5 changes: 4 additions & 1 deletion packages/grid/src/vaadin-grid-scroll-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ export const ScrollMixin = (superClass) =>

if (this._rowWithFocusedElement) {
// Make sure the row with the focused element is fully inside the visible viewport
this.__scrollIntoViewport(this._rowWithFocusedElement.index);
// Don't change scroll position if the user is interacting with the mouse
if (!this._isMousedown) {
this.__scrollIntoViewport(this._rowWithFocusedElement.index);
}

if (!this.$.table.contains(e.relatedTarget)) {
// Virtualizer can't catch the event because if orginates from the light DOM.
Expand Down
3 changes: 3 additions & 0 deletions packages/grid/test/scroll-into-view-lit.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import '../theme/lumo/lit-all-imports.js';
import '../src/lit-all-imports.js';
import './scroll-into-view.common.js';
2 changes: 2 additions & 0 deletions packages/grid/test/scroll-into-view-polymer.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import '../vaadin-grid.js';
import './scroll-into-view.common.js';
90 changes: 90 additions & 0 deletions packages/grid/test/scroll-into-view.common.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import { expect } from '@esm-bundle/chai';
import { fixtureSync, nextFrame } from '@vaadin/testing-helpers';
import { sendKeys, sendMouse } from '@web/test-runner-commands';
import { flushGrid, getContainerCell, getLastVisibleItem, getPhysicalItems } from './helpers.js';

describe('scroll into view', () => {
let grid, firstCell, secondCell, secondDetailsCell;

function verifyRowFullyVisible(grid, rowIndex) {
const scrollerBounds = grid.$.scroller.getBoundingClientRect();
const row = getPhysicalItems(grid)[rowIndex];
const rowBounds = row.getBoundingClientRect();
expect(rowBounds.top).to.be.greaterThanOrEqual(scrollerBounds.top);
expect(rowBounds.bottom).to.be.lessThanOrEqual(scrollerBounds.bottom);
}

function verifyRowNotFullyVisible(grid, rowIndex) {
const scrollerBounds = grid.$.scroller.getBoundingClientRect();
const row = getPhysicalItems(grid)[rowIndex];
const rowBounds = row.getBoundingClientRect();
const isTopInvisible = rowBounds.top < scrollerBounds.top;
const isBottomInvisible = rowBounds.bottom > scrollerBounds.bottom;
expect(isTopInvisible || isBottomInvisible).to.be.true;
}

beforeEach(async () => {
grid = fixtureSync(`
<vaadin-grid style="height: 150px">
<vaadin-grid-column></vaadin-grid-column>
</vaadin-grid>
`);
const column = grid.querySelector('vaadin-grid-column');
column.renderer = (root, _, model) => {
root.innerHTML = `<div style="height: 100px">${model.item}</div>`;
};
grid.rowDetailsRenderer = (root, _, model) => {
root.innerHTML = `<div style="height: 30px">${model.item} details</div>`;
};
grid.items = [1, 2];
grid.detailsOpenedItems = [2];

flushGrid(grid);
await nextFrame();

firstCell = getContainerCell(grid.$.items, 0, 0);
secondCell = getContainerCell(grid.$.items, 1, 0);
secondDetailsCell = getLastVisibleItem(grid).querySelector('[part~="details-cell"]');
});

it('should scroll row into view when focusing programmatically', () => {
verifyRowNotFullyVisible(grid, 1);

secondCell.focus();

expect(grid.$.table.scrollTop).to.be.above(0);
verifyRowFullyVisible(grid, 1);
});

it('should scroll row into view when focusing with keyboard navigation', async () => {
verifyRowNotFullyVisible(grid, 1);

firstCell.focus();
await sendKeys({ press: 'ArrowDown' });

expect(grid.$.table.scrollTop).to.be.above(0);
verifyRowFullyVisible(grid, 1);
});

it('should not change scroll position when focusing row cell with click', async () => {
verifyRowNotFullyVisible(grid, 1);

const bounds = secondCell.getBoundingClientRect();
await sendMouse({ type: 'click', position: [bounds.x + 5, bounds.y + 5] });

expect(grid.$.table.scrollTop).to.equal(0);
});

it('should not change scroll position when focusing details cell with click', async () => {
// Make details cell partially visible for clicking
grid.style.height = '240px';
await nextFrame();

verifyRowNotFullyVisible(grid, 1);

const bounds = secondDetailsCell.getBoundingClientRect();
await sendMouse({ type: 'click', position: [bounds.x + 5, bounds.y + 5] });

expect(grid.$.table.scrollTop).to.equal(0);
});
});
Loading