Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/beam-from-focused-leaf.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/claim-landing-not-last-focused.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/redirect-only-not-a-candidate.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/removed-claim-does-not-block.md
Original file line number Diff line number Diff line change
@@ -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.
95 changes: 95 additions & 0 deletions packages/react-lightning/src/focus/FocusKeyManager.spec.ts
Original file line number Diff line number Diff line change
@@ -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<MockElement>;
let km: FocusKeyManager<LightningElement>;

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<LightningElement>);
});

// 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);
});
});
59 changes: 38 additions & 21 deletions packages/react-lightning/src/focus/FocusKeyManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<LightningElement>[]): Iterable<LightningElement> {
function* childElements(
children: FocusNode<LightningElement>[],
exclude?: FocusNode<LightningElement>,
): Iterable<LightningElement> {
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.
Expand Down Expand Up @@ -112,30 +119,25 @@ export class FocusKeyManager<T extends LightningElement> {
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<LightningElement> = 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<LightningElement> = focusNode.focusedElement;

while (leafNode.focusedElement) {
leafNode = leafNode.focusedElement;
}

const target = resolveDirectionalTarget(
leafNode.element,
closestElement,
Expand Down Expand Up @@ -176,6 +178,21 @@ export class FocusKeyManager<T extends LightningElement> {
return true;
};

private _closestSibling(
source: LightningElement,
focusNode: FocusNode<LightningElement>,
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);

Expand Down
33 changes: 29 additions & 4 deletions packages/react-lightning/src/focus/FocusManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>): void {
const committed = best.focusCommitted;

this._focusNode(best);
best.focusCommitted = committed;
}

private _armClaimPatience(delay: number): void {
if (this._claimPatienceTimer !== null) {
return;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<T>): boolean {
return (
!!node.element.isFocusGroup &&
!node.hasFocusableChildren &&
!!(node.destinationKeys || node.destinations)
);
}

private _checkFocusableChildren(parentNode: FocusNode<T> | RootNode<T>) {
// The new items of a re-rendered list aren't focusable yet, so the settle checks later.
if (this._isRepickDeferred(parentNode)) {
Expand Down Expand Up @@ -1581,6 +1603,7 @@ export class FocusManager<
newChild &&
this._isEffectivelyFocusable(newChild) &&
!hasExternalRedirect(newChild) &&
!this._isRedirectOnly(newChild) &&
(includeRestorationExcluded || !newChild.focusRestorationExcluded) &&
newChild !== relativeNode
) {
Expand Down Expand Up @@ -1612,6 +1635,7 @@ export class FocusManager<
if (
!this._isEffectivelyFocusable(child) ||
hasExternalRedirect(child) ||
this._isRedirectOnly(child) ||
(!includeRestorationExcluded && child.focusRestorationExcluded)
) {
continue;
Expand Down Expand Up @@ -1680,6 +1704,7 @@ export class FocusManager<
child.focusRestorationExcluded ||
!this._isEffectivelyFocusable(child) ||
hasExternalRedirect(child) ||
this._isRedirectOnly(child) ||
!this._measure(child.element, scratch)
) {
continue;
Expand Down
71 changes: 71 additions & 0 deletions packages/react-lightning/src/focus/FocusRegions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});

Expand Down Expand Up @@ -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', () => {
Expand Down
Loading