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

Merged
merged 7 commits into from
Oct 19, 2024

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Oct 16, 2024

Description

Fixes the issue where the focus outline repeatedly appeared on random items as the user scrolled when the actual focused item was out of view. This specifically happened if the item was focused by the mouse and a custom CSS was applied to show the focus outline on clicks:

vaadin-grid::part(focused-cell)::before {
  content: '';
  position: absolute;
  top: 0;
  right: 0;
  bottom: 0;
  left: 0;
  pointer-events: none;
  box-shadow: inset 0 0 0 2px var(--lumo-primary-color-50pct);
}

Depends on

Fixes #7614

Type of change

  • Bugfix

@vursen vursen changed the title fix: ensure no focus outline when scrolled out of view fix: ensure no focus outline when item is scrolled out of view Oct 16, 2024
@vursen vursen force-pushed the hide-focus-outline-when-element-is-out-of-view branch from d585d3a to e174d3e Compare October 16, 2024 13:32
if (isFocusedItemRendered) {
// Ensure the correct element is focused, as the virtualizer
// may use different elements when re-rendering visible items.
this.__updateItemsFocusable();
Copy link
Contributor Author

@vursen vursen Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Debouncer + __updateItemsFocusable fixes #7659 (comment). It also fixes cases where scrolling back to the focused item is triggered by Tab or an arrow key.

@vursen
Copy link
Contributor Author

vursen commented Oct 17, 2024

Here is a snippet that I used for testing:

<style>
  vaadin-grid::part(focused-cell)::before {
    content: '';
    position: absolute;
    top: 0;
    right: 0;
    bottom: 0;
    left: 0;
    pointer-events: none;
    box-shadow: inset 0 0 0 2px var(--lumo-primary-color-50pct);
  }
</style>

<script type="module">
  import '@vaadin/grid';

  const grid = document.querySelector('vaadin-grid');

  grid.items = Array.from({ length: 500 }, (_, i) => `Item-${i}`);

  grid.querySelectorAll('vaadin-grid-column').forEach((column) => {
    column.headerRenderer = (root, column, model) => {
      root.textContent = 'Header';
    }

    column.renderer = (root, column, model) => {
      root.textContent = model.item;
    }

    column.footerRenderer = (root, column, model) => {
      root.textContent = 'Footer';
    }
  })
</script>

<vaadin-grid>
  <vaadin-grid-column></vaadin-grid-column>
  <vaadin-grid-column></vaadin-grid-column>
</vaadin-grid>

Before:

Screen.Recording.2024-10-17.at.10.38.31.mov

After:

Screen.Recording.2024-10-17.at.10.39.06.mov

Comment on lines +148 to +150
[part~='cell'] {
outline: none;
}
Copy link
Contributor Author

@vursen vursen Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: This hides the default browser outline that otherwise appears when the focused-cell part is removed. It appears because the virtualizer element remains focused even after the focused item that was previously rendered in that element is no longer visible.

image

@vursen vursen marked this pull request as ready for review October 17, 2024 06:33
this.toggleAttribute('navigating', true);
/** @protected */
_preventScrollerRotatingCellFocus() {
if (this._activeRowGroup !== this.$.items) {
Copy link
Contributor Author

@vursen vursen Oct 17, 2024

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.

@vursen vursen marked this pull request as draft October 17, 2024 08:33
@vursen
Copy link
Contributor Author

vursen commented Oct 17, 2024

One test fails in Webkit. I'm investigating... Fixed.

Copy link

@vursen vursen marked this pull request as ready for review October 17, 2024 09:27
@vursen vursen merged commit 0cb7c09 into main Oct 19, 2024
9 checks passed
@vursen vursen deleted the hide-focus-outline-when-element-is-out-of-view branch October 19, 2024 12:15
vursen added a commit that referenced this pull request Oct 19, 2024
vursen added a commit that referenced this pull request Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recurring focussed cell when scrolling in tables with many entries
4 participants