-
Notifications
You must be signed in to change notification settings - Fork 83
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
Changes from 6 commits
dd0ccce
a41da53
57f7036
f4e4c57
e174d3e
29857f5
0279e22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
||
|
@@ -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 { | ||
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: Debouncer + |
||
|
||
// 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); | ||
} | ||
} | ||
}, | ||
); | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -145,6 +145,10 @@ export const gridStyles = css` | |
white-space: nowrap; | ||
} | ||
|
||
[part~='cell'] { | ||
outline: none; | ||
} | ||
Comment on lines
+148
to
+150
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
[part~='cell'] > [tabindex] { | ||
display: flex; | ||
align-items: inherit; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: This is an optimization to avoid triggering the debouncer when there is no focus within the items container.