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: ensure no focus outline when item is scrolled out of view (#7980) (CP: 24.4) #7999

Merged
merged 1 commit into from
Oct 19, 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
63 changes: 42 additions & 21 deletions packages/grid/src/vaadin-grid-keyboard-navigation-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
import { animationFrame } from '@vaadin/component-base/src/async.js';
import { Debouncer } from '@vaadin/component-base/src/debounce.js';
import { addValueToAttribute, removeValueFromAttribute } from '@vaadin/component-base/src/dom-utils.js';
import { get } from '@vaadin/component-base/src/path-utils.js';

Expand Down Expand Up @@ -504,8 +506,8 @@ export const KeyboardNavigationMixin = (superClass) =>
// listener invocation gets updated _focusedItemIndex value.
this._focusedItemIndex = dstRowIndex;

// This has to be set after scrolling, otherwise it can be removed by
// `_preventScrollerRotatingCellFocus(row, index)` during scrolling.
// Reapply navigating state in case it was removed due to previous item
// being focused with the mouse.
this.toggleAttribute('navigating', true);

return {
Expand Down Expand Up @@ -920,26 +922,45 @@ export const KeyboardNavigationMixin = (superClass) =>
focusTarget.tabIndex = isInteractingWithinActiveSection ? -1 : 0;
}

/**
* @param {!HTMLTableRowElement} row
* @param {number} index
* @protected
*/
_preventScrollerRotatingCellFocus(row, index) {
if (
row.index === this._focusedItemIndex &&
this.hasAttribute('navigating') &&
this._activeRowGroup === this.$.items
) {
// Focused item has went, hide navigation mode
this._navigatingIsHidden = true;
this.toggleAttribute('navigating', false);
}
if (index === this._focusedItemIndex && this._navigatingIsHidden) {
// Focused item is back, restore navigation mode
this._navigatingIsHidden = false;
this.toggleAttribute('navigating', true);
/** @protected */
_preventScrollerRotatingCellFocus() {
if (this._activeRowGroup !== this.$.items) {
return;
}

this.__preventScrollerRotatingCellFocusDebouncer = Debouncer.debounce(
this.__preventScrollerRotatingCellFocusDebouncer,
animationFrame,
() => {
const isItemsRowGroupActive = this._activeRowGroup === this.$.items;
const isFocusedItemRendered = this._getRenderedRows().some((row) => row.index === this._focusedItemIndex);
if (isFocusedItemRendered) {
// Ensure the correct element is focused, as the virtualizer
// may use different elements when re-rendering visible items.
this.__updateItemsFocusable();

// The focused item is visible, so restore the cell focus outline
// and navigation mode.
if (isItemsRowGroupActive && !this.__rowFocusMode) {
this._focusedCell = this._itemsFocusable;
}

if (this._navigatingIsHidden) {
this.toggleAttribute('navigating', true);
this._navigatingIsHidden = false;
}
} else if (isItemsRowGroupActive) {
// The focused item was scrolled out of view and focus is still inside body,
// so remove the cell focus outline and hide navigation mode.
this._focusedCell = null;

if (this.hasAttribute('navigating')) {
this._navigatingIsHidden = true;
this.toggleAttribute('navigating', false);
}
}
},
);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/grid/src/vaadin-grid-styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ export const gridStyles = css`
white-space: nowrap;
}

[part~='cell'] {
outline: none;
}

[part~='cell'] > [tabindex] {
display: flex;
align-items: inherit;
Expand Down
1 change: 1 addition & 0 deletions packages/grid/test/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const flushGrid = (grid) => {
].forEach((debouncer) => debouncer?.flush());

grid.__virtualizer.flush();
grid.__preventScrollerRotatingCellFocusDebouncer?.flush();
grid.performUpdate?.();
};

Expand Down
75 changes: 68 additions & 7 deletions packages/grid/test/keyboard-navigation.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
getContainerCell,
getFirstVisibleItem,
getLastVisibleItem,
getPhysicalItems,
getRowCells,
getRows,
infiniteDataProvider,
Expand Down Expand Up @@ -1323,6 +1324,7 @@ describe('keyboard navigation', () => {
expect(grid.hasAttribute('navigating')).to.be.true;

grid.scrollToIndex(100);
flushGrid(grid);

expect(grid.hasAttribute('navigating')).to.be.false;
});
Expand All @@ -1331,8 +1333,10 @@ describe('keyboard navigation', () => {
focusItem(0);
right();
grid.scrollToIndex(100);
flushGrid(grid);

grid.scrollToIndex(0);
flushGrid(grid);

expect(grid.hasAttribute('navigating')).to.be.true;
});
Expand All @@ -1344,6 +1348,7 @@ describe('keyboard navigation', () => {
expect(grid.hasAttribute('navigating')).to.be.true;

grid.scrollToIndex(100);
flushGrid(grid);

expect(grid.hasAttribute('navigating')).to.be.true;
});
Expand Down Expand Up @@ -1567,18 +1572,74 @@ describe('keyboard navigation', () => {
});

describe('focused-cell part', () => {
it('should add focused-cell to cell part when focused', () => {
focusFirstHeaderCell();

expect(getFirstHeaderCell().getAttribute('part')).to.contain('focused-cell');
beforeEach(() => {
grid.items = undefined;
grid.size = 200;
grid.dataProvider = infiniteDataProvider;
flushGrid(grid);
});

it('should remove focused-cell from cell part when blurred', () => {
focusFirstHeaderCell();
it('should add the part to cell when focused', () => {
focusItem(5);
const cell = getPhysicalItems(grid)[5].firstChild;
expect(cell.getAttribute('part')).to.contain('focused-cell');
});

it('should remove the part from cell when blurred', () => {
focusItem(5);
focusable.focus();
const cell = getPhysicalItems(grid)[5].firstChild;
expect(cell.getAttribute('part')).to.not.contain('focused-cell');
});

it('should keep the part when focused item is scrolled but still visible', () => {
focusItem(5);
grid.scrollToIndex(2);
flushGrid(grid);
const cell = getPhysicalItems(grid)[5].firstChild;
expect(cell.getAttribute('part')).to.contain('focused-cell');
});

it('should remove the part when focused item is scrolled out of view', () => {
focusItem(5);
grid.scrollToIndex(100);
flushGrid(grid);
expect(grid.$.items.querySelector(':not([hidden]) [part~="focused-cell"')).to.be.null;
});

expect(getFirstHeaderCell().getAttribute('part')).to.not.contain('focused-cell');
it('should restore the part when focused item is scrolled back to view', () => {
focusItem(5);
grid.scrollToIndex(100);
flushGrid(grid);

// Simulate real scrolling to get the virtualizer to render
// the focused item in a different element.
grid.$.table.scrollTop = 0;
flushGrid(grid);

const cell = getPhysicalItems(grid)[5].firstChild;
expect(cell.getAttribute('part')).to.contain('focused-cell');
});

it('should not add the part to any element when focused item is scrolled back to view - row focus mode', () => {
focusItem(5);
left();
grid.scrollToIndex(100);
flushGrid(grid);
grid.scrollToIndex(0);
flushGrid(grid);
expect(grid.$.items.querySelector(':not([hidden]) [part~="focused-cell"')).to.be.null;
});

it('should not remove the part from header cell when scrolling items', () => {
focusFirstHeaderCell();
grid.scrollToIndex(100);
flushGrid(grid);
expect(getFirstHeaderCell().getAttribute('part')).to.contain('focused-cell');

grid.scrollToIndex(0);
flushGrid(grid);
expect(getFirstHeaderCell().getAttribute('part')).to.contain('focused-cell');
});
});

Expand Down