diff --git a/.changeset/beam-from-focused-leaf.md b/.changeset/beam-from-focused-leaf.md new file mode 100644 index 00000000..c48dfbcb --- /dev/null +++ b/.changeset/beam-from-focused-leaf.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +Directional moves pick the next sibling by beaming from the focused leaf instead of the group's focused child, as on tvOS and Android. Up from a tile at the left of a full-width row now goes to what sits above that tile, not to whatever sits above the row's centre. When nothing lines up with the leaf, the row is used as before. diff --git a/.changeset/claim-landing-not-last-focused.md b/.changeset/claim-landing-not-last-focused.md new file mode 100644 index 00000000..66d42279 --- /dev/null +++ b/.changeset/claim-landing-not-last-focused.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +Where a claim lands on its own no longer counts as the region's last-focused item. A claim like `['last-focused', episodeKey]` that falls back to its first child while the episode key is still mounting now moves to the episode when it registers, instead of resolving 'last-focused' to its own fallback and staying put. A side effect: until the user moves inside the claimed region, a later `autoFocus` child there can still take its preferred slot. diff --git a/.changeset/redirect-only-not-a-candidate.md b/.changeset/redirect-only-not-a-candidate.md new file mode 100644 index 00000000..b1db0a65 --- /dev/null +++ b/.changeset/redirect-only-not-a-candidate.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +An empty group that only forwards focus (`destinationKeys` or `destinations` with no focusable children, like a claim region) is no longer picked when a focused item unmounts or when focus enters its parent. Before, a remounting button could leave focus stuck on the invisible claim region next to it. diff --git a/.changeset/removed-claim-does-not-block.md b/.changeset/removed-claim-does-not-block.md new file mode 100644 index 00000000..a6999cf8 --- /dev/null +++ b/.changeset/removed-claim-does-not-block.md @@ -0,0 +1,5 @@ +--- +'@plextv/react-lightning': patch +--- + +A claim that was removed no longer blocks the next one. Before, once the target of a lower priority claim unmounted, a later higher priority claim in the same scope never took focus. diff --git a/packages/react-lightning/src/focus/FocusKeyManager.spec.ts b/packages/react-lightning/src/focus/FocusKeyManager.spec.ts new file mode 100644 index 00000000..32f57f1d --- /dev/null +++ b/packages/react-lightning/src/focus/FocusKeyManager.spec.ts @@ -0,0 +1,95 @@ +import { beforeEach, describe, expect, it } from 'vitest'; + +import { Keys } from '../input/Keys'; +import { createMockElement, type MockElement } from '../mocks/createMockElement'; +import type { KeyEvent, LightningElement, Rect } from '../types'; +import { FocusKeyManager } from './FocusKeyManager'; +import { FocusManager } from './FocusManager'; + +type PlacedElement = MockElement & { + node: { w: number; h: number }; + getRelativePosition: () => { x: number; y: number }; +}; + +let nextId = 1; + +function place(name: string, rect: Rect, isGroup = false): PlacedElement { + const element = createMockElement(nextId++, name) as PlacedElement; + element.isFocusGroup = isGroup; + element.node = { w: rect.w, h: rect.h }; + element.getRelativePosition = () => ({ x: rect.x, y: rect.y }); + + return element; +} + +describe('directional moves', () => { + let fm: FocusManager; + let km: FocusKeyManager; + + const press = (group: MockElement, remoteKey: Keys) => + km.handleKeyDown(group as unknown as LightningElement, { remoteKey } as KeyEvent); + + beforeEach(() => { + nextId = 1; + fm = new FocusManager(); + km = new FocusKeyManager(fm as unknown as FocusManager); + }); + + // A full-width activity row whose focused tile sits at the left, under a + // narrow action bar, with a summary off to the right of the action bar. + const buildDetails = () => { + const page = place('page', { x: 0, y: 0, w: 1952, h: 600 }, true); + const summary = place('summary', { x: 500, y: 0, w: 1400, h: 250 }); + const actions = place('actions', { x: 0, y: 200, w: 448, h: 60 }, true); + const play = place('play', { x: 0, y: 200, w: 200, h: 60 }); + const activity = place('activity', { x: 0, y: 300, w: 1952, h: 100 }, true); + const tile = place('tile', { x: 0, y: 300, w: 300, h: 100 }); + fm.addElement(page); + fm.addElement(summary, page); + fm.addElement(actions, page); + fm.addElement(play, actions); + fm.addElement(activity, page); + fm.addElement(tile, activity); + fm.focus(tile); + + return { page, summary, play, tile }; + }; + + it('beams from the focused leaf, not its full-width row', () => { + const { page, summary, play } = buildDetails(); + + expect(press(page, Keys.Up)).toBe(false); + expect(play.focused).toBe(true); + expect(summary.focused).toBe(false); + }); + + it('does not pick the row the focused leaf sits in', () => { + const page = place('page', { x: 0, y: 0, w: 1000, h: 600 }, true); + const top = place('top', { x: 0, y: 0, w: 1000, h: 100 }); + const row = place('row', { x: 0, y: 200, w: 1000, h: 300 }, true); + const tile = place('tile', { x: 0, y: 200, w: 200, h: 100 }); + fm.addElement(page); + fm.addElement(top, page); + fm.addElement(row, page); + fm.addElement(tile, row); + fm.focus(tile); + + expect(press(page, Keys.Down)).toBe(true); + expect(tile.focused).toBe(true); + }); + + it('falls back to the row when nothing lines up with the leaf', () => { + const page = place('page', { x: 0, y: 0, w: 1000, h: 600 }, true); + const banner = place('banner', { x: 500, y: 0, w: 300, h: 100 }); + const row = place('row', { x: 0, y: 200, w: 1000, h: 100 }, true); + const tile = place('tile', { x: 0, y: 200, w: 100, h: 100 }); + fm.addElement(page); + fm.addElement(banner, page); + fm.addElement(row, page); + fm.addElement(tile, row); + fm.focus(tile); + + expect(press(page, Keys.Up)).toBe(false); + expect(banner.focused).toBe(true); + }); +}); diff --git a/packages/react-lightning/src/focus/FocusKeyManager.ts b/packages/react-lightning/src/focus/FocusKeyManager.ts index 945bf496..53323746 100644 --- a/packages/react-lightning/src/focus/FocusKeyManager.ts +++ b/packages/react-lightning/src/focus/FocusKeyManager.ts @@ -5,11 +5,18 @@ import { Direction } from './Direction'; import type { FocusExits, FocusManager, FocusNode } from './FocusManager'; /** Lazily maps FocusNode children to their elements without allocating an array */ -function* childElements(children: FocusNode[]): Iterable { +function* childElements( + children: FocusNode[], + exclude?: FocusNode, +): Iterable { for (let i = 0; i < children.length; i++) { // oxlint-disable-next-line typescript/no-non-null-assertion -- bounds-checked loop const child = children[i]!; + if (child === exclude) { + continue; + } + // A focus group with no focusable descendant only wraps non-interactive // content (a list header); skip it so nav lands on a real target. // A redirect with destinations is a target even when empty, it forwards on arrival. @@ -112,30 +119,25 @@ export class FocusKeyManager { return true; } - // Pick the next sibling from the immediate focused child, so the current - // subtree (and its ancestors) are excluded as candidates. - const closestElement = findClosestElement( - focusNode.focusedElement.element, - childElements(focusNode.children), - focusNode.parent.element, - direction, - focusNode.allowOffscreen, - this._hasFocusableChildren, - ); + const focusedChild = focusNode.focusedElement; + let leafNode: FocusNode = focusedChild; + + while (leafNode.focusedElement) { + leafNode = leafNode.focusedElement; + } + + // Beam from the deepest focused leaf, as UIKit and Android do, so a + // full-width row moves from where the user is in it. The row itself is the + // fallback when nothing lines up with the leaf. + const closestElement = + this._closestSibling(leafNode.element, focusNode, direction) ?? + (leafNode === focusedChild + ? null + : this._closestSibling(focusedChild.element, focusNode, direction)); if (closestElement) { - // Descend into the chosen sibling from the deepest focused leaf, not the - // group's immediate child, so a nested layout keeps the real cross-axis - // position of what the user is on (an EPG airing, not its full-width row) - // and lands on the overlapping child instead of the group's first child. // A redirect node (e.g. a row's airings guide) is returned for the focus // manager to forward to its anchored destination. - let leafNode: FocusNode = focusNode.focusedElement; - - while (leafNode.focusedElement) { - leafNode = leafNode.focusedElement; - } - const target = resolveDirectionalTarget( leafNode.element, closestElement, @@ -176,6 +178,21 @@ export class FocusKeyManager { return true; }; + private _closestSibling( + source: LightningElement, + focusNode: FocusNode, + direction: Direction, + ): LightningElement | null { + return findClosestElement( + source, + childElements(focusNode.children, focusNode.focusedElement ?? undefined), + focusNode.parent.element, + direction, + focusNode.allowOffscreen, + this._hasFocusableChildren, + ); + } + private _hasFocusableChildren = (element: LightningElement): boolean => { const node = this._focusManager.getFocusNode(element); diff --git a/packages/react-lightning/src/focus/FocusManager.ts b/packages/react-lightning/src/focus/FocusManager.ts index 2d9a7604..a2de0b09 100644 --- a/packages/react-lightning/src/focus/FocusManager.ts +++ b/packages/react-lightning/src/focus/FocusManager.ts @@ -648,10 +648,18 @@ export class FocusManager< const { focusPath } = this.activeLayer; if (destination && focusPath.includes(best.element) && !focusPath.includes(destination)) { - this._focusNode(best); + this._focusClaim(best); } } + /** A claim's own landing is a mount-time default, so it isn't what 'last-focused' remembers. */ + private _focusClaim(best: FocusNode): void { + const committed = best.focusCommitted; + + this._focusNode(best); + best.focusCommitted = committed; + } + private _armClaimPatience(delay: number): void { if (this._claimPatienceTimer !== null) { return; @@ -728,13 +736,18 @@ export class FocusManager< const previous = scope.scopeClaimTarget; - // A claim that gave up only hands over focus it still holds. - if (previous && !this._claimHasEntry(previous) && !this._holdsClaimFocus(previous)) { + // A claim that gave up only hands over focus it still holds. A removed one holds nothing. + if ( + previous && + this.activeLayer.elements.get(previous.element) === previous && + !this._claimHasEntry(previous) && + !this._holdsClaimFocus(previous) + ) { return; } scope.scopeClaimTarget = best; - this._focusNode(best); + this._focusClaim(best); } public get hasInitialClaim(): boolean { @@ -1470,6 +1483,15 @@ export class FocusManager< ); } + /** An empty group that only forwards (a claim region) shows nothing, so a repick or an entry never lands on it. */ + private _isRedirectOnly(node: FocusNode): boolean { + return ( + !!node.element.isFocusGroup && + !node.hasFocusableChildren && + !!(node.destinationKeys || node.destinations) + ); + } + private _checkFocusableChildren(parentNode: FocusNode | RootNode) { // The new items of a re-rendered list aren't focusable yet, so the settle checks later. if (this._isRepickDeferred(parentNode)) { @@ -1581,6 +1603,7 @@ export class FocusManager< newChild && this._isEffectivelyFocusable(newChild) && !hasExternalRedirect(newChild) && + !this._isRedirectOnly(newChild) && (includeRestorationExcluded || !newChild.focusRestorationExcluded) && newChild !== relativeNode ) { @@ -1612,6 +1635,7 @@ export class FocusManager< if ( !this._isEffectivelyFocusable(child) || hasExternalRedirect(child) || + this._isRedirectOnly(child) || (!includeRestorationExcluded && child.focusRestorationExcluded) ) { continue; @@ -1680,6 +1704,7 @@ export class FocusManager< child.focusRestorationExcluded || !this._isEffectivelyFocusable(child) || hasExternalRedirect(child) || + this._isRedirectOnly(child) || !this._measure(child.element, scratch) ) { continue; diff --git a/packages/react-lightning/src/focus/FocusRegions.spec.ts b/packages/react-lightning/src/focus/FocusRegions.spec.ts index 9f8f379e..1dfdb06c 100644 --- a/packages/react-lightning/src/focus/FocusRegions.spec.ts +++ b/packages/react-lightning/src/focus/FocusRegions.spec.ts @@ -498,6 +498,39 @@ describe('focus regions', () => { expect(outside.focused).toBe(true); expect(first.focused).toBe(false); }); + + it('lets a later claim take over after the previous one was removed', () => { + const scope = grp('scope'); + fm.addElement(scope, null, { scope: true }); + const row = grp('row'); + fm.addElement(row, scope); + const target = keyed(row, 'target'); + fm.addElement(leaf('other'), row); + const low = claim(scope, 5, ['target']); + + expect(target.focused).toBe(true); + + fm.removeElement(target); + fm.removeElement(low); + const high = region(scope, 10); + + expect(high.child.focused).toBe(true); + }); + + it('does not treat its own fallback landing as last-focused', () => { + const scope = grp('scope'); + fm.addElement(scope, null, { scope: true }); + const r = claim(scope, 2, ['last-focused', 'episode']); + const first = leaf('first'); + fm.addElement(first, r); + + expect(first.focused).toBe(true); + + vi.advanceTimersByTime(11); + const episode = keyed(r, 'episode'); + + expect(episode.focused).toBe(true); + }); }); }); @@ -570,6 +603,44 @@ describe('focus regions', () => { expect(c.focused).toBe(true); }); + + it('skips an empty redirect-only group', () => { + const row = grp('row'); + const claimRegion = grp('claim'); + const play = leaf('play'); + rects.set(claimRegion, { x: 0, y: 0, w: 0, h: 0 }); + rects.set(play, { x: 0, y: 0, w: 100, h: 40 }); + geo.addElement(row); + geo.addElement(claimRegion, row, { destinationKeys: ['play'], focusRedirect: true }); + geo.addElement(play, row, { focusKey: 'play' }); + geo.focus(play); + geo.removeElement(play); + + expect(geo.focusPath).not.toContain(claimRegion); + + const remounted = leaf('play'); + rects.set(remounted, { x: 0, y: 0, w: 100, h: 40 }); + geo.addElement(remounted, row, { focusKey: 'play' }); + + expect(remounted.focused).toBe(true); + }); + + it('enters a group past an empty redirect-only child', () => { + const outside = leaf('outside'); + const row = grp('row'); + const claimRegion = grp('claim'); + const play = leaf('play'); + rects.set(claimRegion, { x: 0, y: 0, w: 0, h: 0 }); + rects.set(play, { x: 10, y: 0, w: 100, h: 40 }); + geo.addElement(outside); + geo.addElement(row, null, { focusEntry: 'first' }); + geo.addElement(claimRegion, row, { destinationKeys: ['missing'], focusRedirect: true }); + geo.addElement(play, row); + geo.focus(outside); + geo.focus(row); + + expect(play.focused).toBe(true); + }); }); describe('enter and leave', () => {