Skip to content

Commit

Permalink
fix: ensure no focus outline when item is scrolled out of view (#7980)
Browse files Browse the repository at this point in the history
  • Loading branch information
vursen authored Oct 19, 2024
1 parent d5f95aa commit 0cb7c09
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 28 deletions.
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 @@ -145,6 +145,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 @@ -25,6 +25,7 @@ import {
getFocusedCellIndex,
getFocusedRowIndex,
getLastVisibleItem,
getPhysicalItems,
getRowCells,
getRows,
infiniteDataProvider,
Expand Down Expand Up @@ -1313,6 +1314,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 @@ -1321,8 +1323,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 @@ -1334,6 +1338,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 @@ -1557,18 +1562,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

0 comments on commit 0cb7c09

Please sign in to comment.