Skip to content

NXT-7491 [2027TV] POC on 2way focus browsing#3412

Open
dan-ichim-lgp wants to merge 11 commits into
developfrom
feature/NXT-7491
Open

NXT-7491 [2027TV] POC on 2way focus browsing#3412
dan-ichim-lgp wants to merge 11 commits into
developfrom
feature/NXT-7491

Conversation

@dan-ichim-lgp

@dan-ichim-lgp dan-ichim-lgp commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Checklist

  • I have read and understand the contribution guide
  • A CHANGELOG entry is included
  • At least one test case is included for this feature or bug fix
  • Documentation was added or is not needed
  • This is an API breaking change

Issue Resolved / Feature Added

This change adds Tab / Shift+Tab keyboard navigation to Spotlight for Enact apps.

Before: Spotlight did not handle the Tab key. Tab used the browser’s default focus behavior, which does not understand Enact’s focus model (spottable controls, spotlight containers, portaled content). Keyboard users could not rely on Tab to move focus in a consistent, layout-based order across the UI.

After: Spotlight handles Tab and Shift+Tab and moves focus among spottable controls in visual order on screen (top to bottom, then left to right; mirrored for RTL).

This applies broadly—to any spottable UI, including lists, grids (e.g. VirtualGridList items), buttons, and form controls. The PR also adds extra behavior for open dropdown popups (portaled lists linked to triggers via aria-owns), which was the main motivation and test focus for this story.

Resolution

What this PR introduces

  1. General Tab navigation (all spottable controls)

When the user presses Tab or Shift+Tab, Spotlight:

  • finds focusable (spottable) controls in the current scope;
  • sorts them by where they appear on screen, not by DOM order alone;
  • moves focus to the next or previous control in that order.

This works for normal page content, grid layouts, lists, and any other component built with spottable children—including VirtualGridList, for rendered/visible items in the DOM.

Note: Virtualized lists only include items currently in the DOM, so Tab moves through visible spottable items, not the full virtual dataset.

  1. Additional behavior for open dropdown popups

Dropdown option lists use a special Enact pattern: a portaled popup list in a self-only spotlight container, linked to its trigger through aria-owns. For this case, Spotlight adds:

  • Tab / Shift+Tab through options inside an open list;
  • at the end or start of a list, handoff to the next control in layout order;
  • when the next control is another open dropdown, focus enters that list at the first (Tab) or last (Shift+Tab) option;
  • when a dropdown is closed, focus lands on its trigger button only.

Example (three open dropdowns):

last option in A → first option in B → first option in C → close button
(one Tab per step; Shift+Tab reverses)

Unchanged

5-way (arrow key) navigation — not modified
Public Spotlight API — no breaking changes; apps do not need code updates

How it works (simple overview)

  1. User presses Tab or Shift+Tab.
  2. Spotlight builds a list of spottable controls in screen order.
  3. If focus is inside an open dropdown list, Tab stays within that list until the last/first option.
  4. At a list boundary, Spotlight picks the next nearby control on the page.
  5. If that control opens another list, focus enters that list; otherwise focus moves to the control itself (button, grid item, etc.).

Other cases: Tab is managed while Spotlight is paused (modal); Tab is skipped if the pointer moved during the key press; RTL layouts are supported.

Additional Considerations

Links

NXT-7491

Comments

Enact-DCO-1.0-Signed-off-by: Dan Ichim (dan.ichim@lgepartner.com)

@dan-ichim-lgp dan-ichim-lgp self-assigned this Apr 23, 2026
@codecov

codecov Bot commented Apr 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.32%. Comparing base (ab463b9) to head (3d57553).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3412      +/-   ##
===========================================
+ Coverage    86.06%   87.32%   +1.26%     
===========================================
  Files          156      158       +2     
  Lines         7613     7971     +358     
  Branches      2015     2126     +111     
===========================================
+ Hits          6552     6961     +409     
+ Misses         845      807      -38     
+ Partials       216      203      -13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@daniel-stoian-lgp daniel-stoian-lgp changed the base branch from feature/2027TV to develop April 23, 2026 10:21
@dan-ichim-lgp dan-ichim-lgp changed the title Feature/nxt 7491 NXT-7491 [2027TV] POC on 2way focus browsing Apr 23, 2026

@daniel-stoian-lgp daniel-stoian-lgp 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.

Before reading my comments, please do a self-review of the entire PR.
It was really hard to go through the PR as the PR description speaks very thin about navigation between some dropdowns, while de DoD of the user story mentions a broader scope. The PR Description should explain in technical details what all these changes in the PR do, how the new flow works etc.
The PR description should be oriented towards an outside user, so it can be easily understood how it works and what it does

Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-specs.js Outdated

@daniel-stoian-lgp daniel-stoian-lgp 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.

check my tests comments.

Comment thread packages/spotlight/src/tests/spotlight-specs.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-specs.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-specs.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-specs.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-keydown-specs.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-keydown-specs.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-keydown-specs.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-keydown-specs.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-keydown-specs.js Outdated
Comment thread packages/spotlight/src/tests/spotlight-keydown-specs.js Outdated
@dan-ichim-lgp dan-ichim-lgp force-pushed the feature/NXT-7491 branch 3 times, most recently from 5038303 to e5bfacb Compare May 20, 2026 12:32
@dan-ichim-lgp dan-ichim-lgp force-pushed the feature/NXT-7491 branch 2 times, most recently from 3010141 to 69ca424 Compare May 21, 2026 11:54

@daniel-stoian-lgp daniel-stoian-lgp 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.

check my comments

Comment thread packages/spotlight/src/tests/spotlight-keydown-specs.js
Comment thread packages/spotlight/CHANGELOG.md Outdated
Comment thread packages/spotlight/src/spotlight.js Outdated
@daniel-stoian-lgp

Copy link
Copy Markdown
Contributor

The function extraction looks good.
However, the tests still reach into internals the same way they did with _tabNavTestHooks. They just import the new tabTraversal mutable export instead. So you kept a test-only export, and it now relies on import-order side effects (tabNavTestHooks is null until spotlight.js's immediately invoked function expression runs createTabTraversal).

Could you do the following:

  1. Export the pure helpers as plain named exports from tabTraversal.js (comesBeforeInTabOrder, getLinearTabSearchContainerId, getLinearTargetContainerId, getLinearTargetsInContainer, isTargetInSelfOnlyContainer, resolveTargetToOpenPopupItem, findLinearTabExitTarget, findLinearTabExitTargetInTargets).
    Then have the helper tests import {…} from '../tabTraversal' directly and drop the export let tabTraversal / tabTraversal = api singleton entirely. createTabTraversal should return only what production consumes (spotLinear, plus the cache wrapper if we keep it).
  2. In the tests, rename the two mislabeled describe blocks. 'Spotlight spotLinear (integration)' and 'Spotlight Tab helpers (integration)' aren't integration tests. They call internal functions directly. They should read as unit tests (example 'tabTraversal — spotLinear', 'tabTraversal — pure helpers'). The genuine integration coverage already lives in 'Spotlight Tab key dispatch (integration)' and 'Tab keydown (integration …)', which dispatch real keydown events.
  3. Drop the redundant spotLinear unit tests that duplicate the real dispatch tests. Specifically:
    - 'should advance focus to the next spottable element' (calls spotLinear(true) → second) duplicates 'should move focus forward on Tab'.
    - 'should return false at the end of the linear list' duplicates 'should not call preventDefault when Tab reaches end of a non-self-only linear list'.
    For the remaining spotLinear cases that are real edge paths (restoreFocus bootstrap, nearest-target fallback, self-only-no-exit), I prefer using them through dispatchTab(handlers.keydown) so we're testing the path users actually do, rather than the internal entry point.

@daniel-stoian-lgp daniel-stoian-lgp 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.

LGTM. thank you for your effort

@alexandrumorariu alexandrumorariu 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.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants