Skip to content

fix: validate active item after GridView filtering#515

Merged
willeastcott merged 3 commits into
mainfrom
fix/gridview-validate-active-item-after-filter
Mar 2, 2026
Merged

fix: validate active item after GridView filtering#515
willeastcott merged 3 commits into
mainfrom
fix/gridview-validate-active-item-after-filter

Conversation

@willeastcott

@willeastcott willeastcott commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix Tab key navigation in GridView when filtering hides the active item (the one holding tabIndex=0)
  • Add _validateActiveItem() that reassigns the roving tabIndex to the first visible item when the current active item is hidden or destroyed
  • Call it at the end of filter(), and conditionally in filterAsync() only when the active item is hidden during the current batch
  • Reuse _validateActiveItem() in _onRemoveGridViewItem to eliminate duplicate scan logic

Background

GridView uses a roving tabIndex pattern where only one item has tabIndex=0 and all others have tabIndex=-1. When filterAsync() hides items (e.g. when navigating folders in the Editor's Assets panel), the active item can become hidden. Since display: none removes it from the tab order, no visible item is tabbable and Tab does nothing.

Test Plan

  • Open examples/accessibility/tabindex.html and verify roving tabIndex still works correctly (Tab into GridView selects the active item, arrow keys navigate, status bar shows 1 tab stop per widget)
  • In the Editor Assets panel (GridView mode), select the root folder and press Tab -- the first grid item should be selected and focused
  • Navigate to a subfolder and press Tab -- the first grid item in that folder should be selected and focused
  • Verify arrow key navigation works after Tab in both root and subfolders

When filtering hides the current active item (the one with tabIndex=0),
no visible item remains tabbable, breaking Tab key navigation.

Add _validateActiveItem() to reassign the roving tabIndex to the first
visible item after each filter batch, matching the Table's existing
_validateActiveRow() pattern.

Made-with: Cursor
@willeastcott willeastcott requested a review from Copilot March 2, 2026 19:08
@willeastcott willeastcott self-assigned this Mar 2, 2026
@willeastcott willeastcott added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Mar 2, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes GridView roving tabIndex behavior when filtering hides the currently active item, ensuring keyboard Tab navigation always has a visible tabbable item.

Changes:

  • Add _validateActiveItem() to reassign the active (tabbable) item when the current active item is hidden.
  • Invoke _validateActiveItem() after synchronous filter() runs.
  • Invoke _validateActiveItem() after each batch in filterAsync().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/GridView/index.ts Outdated
Comment thread src/components/GridView/index.ts
Comment thread src/components/GridView/index.ts
@willeastcott willeastcott requested a review from Copilot March 2, 2026 19:23
- Use _setActiveItem(null) instead of manual tabIndex/null mutation
- Add destroyed check to _validateActiveItem early return
- Reuse _validateActiveItem in _onRemoveGridViewItem
- Add unit tests for filter() and filterAsync() active item reassignment

Made-with: Cursor

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/GridView/index.ts Outdated
Track whether the active item was hidden during the current batch and
only call _validateActiveItem() when needed, avoiding a full DOM scan
on every frame for large grids.

Made-with: Cursor

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/components/GridView/index.ts:460

  • In filterAsync(), the cancel path (_filterCanceled) returns before _validateActiveItem() can run. Since child.hidden = true emits a synchronous hide event, external listeners could call filterAsyncCancel() mid-batch after the active item is hidden; that would trigger this early-return and leave tabIndex=0 on a now-hidden active item (the same a11y issue this PR is fixing). Consider validating the active item before emitting filter:cancel/returning (either unconditionally on cancel, or when the active item became invalid during the batch).
                if (this._filterCanceled) {
                    this._filterCanceled = false;
                    this.emit('filter:cancel');
                    return;
                }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/GridView/index.ts
@willeastcott willeastcott merged commit 230799b into main Mar 2, 2026
9 checks passed
@willeastcott willeastcott deleted the fix/gridview-validate-active-item-after-filter branch March 2, 2026 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants