From 76aabe2d3d4355f09179f1d9b8aa788b5283d930 Mon Sep 17 00:00:00 2001 From: Peter Ashwood Date: Mon, 17 Aug 2026 16:36:42 +0000 Subject: [PATCH 1/2] fix(design): RTI breaks in incremental hydration --- .../roving-tab-index-boundary.directive.ts | 15 +--- .../roving-tab-index-group.service.ts | 22 ------ .../roving-tab-index.directive.spec.ts | 77 ++++++++++++++++++- .../roving-tab-index.directive.ts | 29 ++++--- 4 files changed, 97 insertions(+), 46 deletions(-) diff --git a/libs/design/src/core/roving-tab-index/roving-tab-index-boundary.directive.ts b/libs/design/src/core/roving-tab-index/roving-tab-index-boundary.directive.ts index 76238a6dbc..90409015d9 100644 --- a/libs/design/src/core/roving-tab-index/roving-tab-index-boundary.directive.ts +++ b/libs/design/src/core/roving-tab-index/roving-tab-index-boundary.directive.ts @@ -1,9 +1,8 @@ -import { CdkTrapFocus } from '@angular/cdk/a11y'; import { computed, Directive, - effect, forwardRef, + inject, input, } from '@angular/core'; @@ -25,7 +24,6 @@ import { DaffRovingTabIndexDirective } from './roving-tab-index.directive'; '(keydown.space)': 'enterGroup($event)', }, hostDirectives: [ - CdkTrapFocus, DaffRovingTabIndexDirective, ], providers: [ @@ -52,6 +50,8 @@ export class DaffRovingTabIndexBoundaryDirective implements DaffRovingTabIndexBo return this._cachedUniqueId; } + private readonly groupService = inject(DaffRovingTabIndexService); + /** * The name of the group for which that this element will act as boundary. * Optional, will be autogenerated to a unique name if omitted. @@ -62,15 +62,6 @@ export class DaffRovingTabIndexBoundaryDirective implements DaffRovingTabIndexBo */ readonly effectiveBoundary = computed(() => this.rtiBoundary() || this._uniqueId); - constructor( - private groupService: DaffRovingTabIndexService, - private focusTrap: CdkTrapFocus, - ) { - effect(() => { - this.focusTrap.enabled = this.effectiveBoundary() === this.groupService.group(); - }); - } - /** * @docs-private */ diff --git a/libs/design/src/core/roving-tab-index/roving-tab-index-group.service.ts b/libs/design/src/core/roving-tab-index/roving-tab-index-group.service.ts index 0226930683..b59c49fa22 100644 --- a/libs/design/src/core/roving-tab-index/roving-tab-index-group.service.ts +++ b/libs/design/src/core/roving-tab-index/roving-tab-index-group.service.ts @@ -63,26 +63,4 @@ export class DaffRovingTabIndexService { : ary[index === ary.length - 1 ? 0 : index + 1]).focus(); } } - - onKeydown(evt: Event) { - if ('key' in evt) { - switch ((evt).key) { - case 'ArrowUp': - case 'ArrowDown': - if (this._group()) { - evt.preventDefault(); - const ary = Array.from(this.document.querySelectorAll(`[data-rti="${this._group()}"]`)); - const index = ary.findIndex((el) => el === this.document.activeElement); - (this.document.activeElement).blur(); - ((evt).key === 'ArrowUp' - ? ary[index === 0 ? ary.length - 1 : index - 1] - : ary[index === ary.length - 1 ? 0 : index + 1]).focus(); - } - break; - - default: - break; - } - } - } } diff --git a/libs/design/src/core/roving-tab-index/roving-tab-index.directive.spec.ts b/libs/design/src/core/roving-tab-index/roving-tab-index.directive.spec.ts index 87e910b4b5..9312e4357d 100644 --- a/libs/design/src/core/roving-tab-index/roving-tab-index.directive.spec.ts +++ b/libs/design/src/core/roving-tab-index/roving-tab-index.directive.spec.ts @@ -78,8 +78,9 @@ describe('@daffodil/design | DaffRovingTabIndexDirective', () => { expect(directive.group()).toEqual(''); }); - describe('when the escape key is pressed', () => { + describe('when the escape key is pressed when inside a group', () => { beforeEach(() => { + groupSpy.set('test'); (de.nativeElement).dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); fixture.detectChanges(); }); @@ -89,8 +90,9 @@ describe('@daffodil/design | DaffRovingTabIndexDirective', () => { }); }); - describe('when the up arrow is pressed', () => { + describe('when the up arrow is pressed when inside a group', () => { beforeEach(() => { + groupSpy.set('test'); (de.nativeElement).dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowUp' })); fixture.detectChanges(); }); @@ -100,8 +102,9 @@ describe('@daffodil/design | DaffRovingTabIndexDirective', () => { }); }); - describe('when the down arrow is pressed', () => { + describe('when the down arrow is pressed when inside a group', () => { beforeEach(() => { + groupSpy.set('test'); (de.nativeElement).dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown' })); fixture.detectChanges(); }); @@ -110,6 +113,74 @@ describe('@daffodil/design | DaffRovingTabIndexDirective', () => { expect(serviceSpy.next).toHaveBeenCalledWith(); }); }); + + describe('when the up arrow is pressed when not inside a group', () => { + beforeEach(() => { + (de.nativeElement).dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowUp' })); + fixture.detectChanges(); + }); + + it('should not navigate to the previous target', () => { + expect(serviceSpy.previous).not.toHaveBeenCalled(); + }); + }); + + describe('when the down arrow is pressed when not inside a group', () => { + beforeEach(() => { + (de.nativeElement).dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown' })); + fixture.detectChanges(); + }); + + it('should not navigate to the next target', () => { + expect(serviceSpy.next).not.toHaveBeenCalled(); + }); + }); + + describe('when the tab key is pressed when inside a group', () => { + beforeEach(() => { + groupSpy.set('test'); + (de.nativeElement).dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab' })); + fixture.detectChanges(); + }); + + it('should navigate to the next target', () => { + expect(serviceSpy.next).toHaveBeenCalledWith(); + }); + }); + + describe('when the tab plus shift key is pressed when inside a group', () => { + beforeEach(() => { + groupSpy.set('test'); + (de.nativeElement).dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', shiftKey: true })); + fixture.detectChanges(); + }); + + it('should navigate to the previous target', () => { + expect(serviceSpy.previous).toHaveBeenCalledWith(); + }); + }); + + describe('when the tab key is pressed when not inside a group', () => { + beforeEach(() => { + (de.nativeElement).dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab' })); + fixture.detectChanges(); + }); + + it('should not navigate to the previous target', () => { + expect(serviceSpy.previous).not.toHaveBeenCalled(); + }); + }); + + describe('when the tab plus shift key is pressed when not inside a group', () => { + beforeEach(() => { + (de.nativeElement).dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', shiftKey: true })); + fixture.detectChanges(); + }); + + it('should not navigate to the next target', () => { + expect(serviceSpy.next).not.toHaveBeenCalled(); + }); + }); }); @Component({ diff --git a/libs/design/src/core/roving-tab-index/roving-tab-index.directive.ts b/libs/design/src/core/roving-tab-index/roving-tab-index.directive.ts index b5feb39286..8c77fc7785 100644 --- a/libs/design/src/core/roving-tab-index/roving-tab-index.directive.ts +++ b/libs/design/src/core/roving-tab-index/roving-tab-index.directive.ts @@ -26,6 +26,8 @@ import { DaffRovingTabIndexService } from './roving-tab-index-group.service'; '(keydown.escape)': 'leaveGroup($event)', '(keydown.arrowup)': 'previous($event)', '(keydown.arrowdown)': 'next($event)', + '(keydown.tab)': 'next($event)', + '(keydown.shift.tab)': 'previous($event)', }, }) export class DaffRovingTabIndexDirective { @@ -57,24 +59,33 @@ export class DaffRovingTabIndexDirective { /** * @docs-private */ - leaveGroup(evt: Event) { - evt.stopPropagation(); - this.service.leave(); + protected leaveGroup(evt: Event) { + if (this.service.group()) { + evt.preventDefault(); + evt.stopPropagation(); + this.service.leave(); + } } /** * @docs-private */ - next(evt: Event) { - evt.stopPropagation(); - this.service.next(); + protected next(evt: Event) { + if (this.service.group()) { + evt.preventDefault(); + evt.stopPropagation(); + this.service.next(); + } } /** * @docs-private */ - previous(evt: Event) { - evt.stopPropagation(); - this.service.previous(); + protected previous(evt: Event) { + if (this.service.group()) { + evt.preventDefault(); + evt.stopPropagation(); + this.service.previous(); + } } } From 59602627b9b66c30d6f4cb44a55a6113f688febc Mon Sep 17 00:00:00 2001 From: Peter Ashwood Date: Tue, 18 Aug 2026 17:08:01 +0000 Subject: [PATCH 2/2] force group recompute after hydration --- .../incremental-hydration.integration.spec.ts | 64 +++++++++++++++++++ .../roving-tab-index-boundary.directive.ts | 7 +- .../roving-tab-index.directive.ts | 16 ++++- 3 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 libs/design/src/core/roving-tab-index/incremental-hydration.integration.spec.ts diff --git a/libs/design/src/core/roving-tab-index/incremental-hydration.integration.spec.ts b/libs/design/src/core/roving-tab-index/incremental-hydration.integration.spec.ts new file mode 100644 index 0000000000..6194394f8b --- /dev/null +++ b/libs/design/src/core/roving-tab-index/incremental-hydration.integration.spec.ts @@ -0,0 +1,64 @@ +import { + Component, + signal, +} from '@angular/core'; +import { + ComponentFixture, + DeferBlockBehavior, + DeferBlockFixture, + DeferBlockState, + TestBed, +} from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; + +import { DaffRovingTabIndexBoundaryDirective } from './roving-tab-index-boundary.directive'; +import { DaffRovingTabIndexDirective } from './roving-tab-index.directive'; + +@Component({ + template: ` +
+ @defer (hydrate on interaction) { +
+ } +
+ `, + imports: [ + DaffRovingTabIndexBoundaryDirective, + DaffRovingTabIndexDirective, + ], +}) +class WrapperComponent { + groupValue = signal('group'); +} + +describe('@daffodil/design/core | Roving Tab Index | Incremental Hydration', () => { + let wrapper: WrapperComponent; + let fixture: ComponentFixture; + let deferBlockFixture: DeferBlockFixture; + + beforeEach(async () => { + TestBed.configureTestingModule({ + imports: [ + WrapperComponent, + ], + deferBlockBehavior: DeferBlockBehavior.Manual, + }); + + fixture = TestBed.createComponent(WrapperComponent); + wrapper = fixture.componentInstance; + fixture.detectChanges(); + deferBlockFixture = (await fixture.getDeferBlocks())[0]; + }); + + // not functional as a mocked test, needs e2e to properly test + xit('should update the child to match the parent after deferred hydration', async () => { + await deferBlockFixture.render(DeferBlockState.Complete); + + wrapper.groupValue.set('updated-group'); + await fixture.whenStable(); + + const child = fixture.debugElement.query(By.css('.child')); + + expect(child.attributes['data-rti']).toEqual('updated-group'); + }); +}); diff --git a/libs/design/src/core/roving-tab-index/roving-tab-index-boundary.directive.ts b/libs/design/src/core/roving-tab-index/roving-tab-index-boundary.directive.ts index 90409015d9..8b56618ffe 100644 --- a/libs/design/src/core/roving-tab-index/roving-tab-index-boundary.directive.ts +++ b/libs/design/src/core/roving-tab-index/roving-tab-index-boundary.directive.ts @@ -34,18 +34,13 @@ import { DaffRovingTabIndexDirective } from './roving-tab-index.directive'; ], }) export class DaffRovingTabIndexBoundaryDirective implements DaffRovingTabIndexBoundary { - /** - * Don't touch this directly. Use `_uniqueId`. - */ - private static _uniqueIdCounter = 0; - /** * Don't touch this directly. Use `_uniqueId`. */ private _cachedUniqueId: string | undefined; private get _uniqueId(): string { if (!this._cachedUniqueId) { - this._cachedUniqueId = `ε-rtiBoundary-${DaffRovingTabIndexBoundaryDirective._uniqueIdCounter++}`; + this._cachedUniqueId = `ε-rtiBoundary-${crypto.randomUUID()}`; } return this._cachedUniqueId; } diff --git a/libs/design/src/core/roving-tab-index/roving-tab-index.directive.ts b/libs/design/src/core/roving-tab-index/roving-tab-index.directive.ts index 8c77fc7785..e80da9539f 100644 --- a/libs/design/src/core/roving-tab-index/roving-tab-index.directive.ts +++ b/libs/design/src/core/roving-tab-index/roving-tab-index.directive.ts @@ -1,9 +1,11 @@ import { + afterNextRender, computed, Directive, Inject, input, Optional, + signal, SkipSelf, } from '@angular/core'; @@ -31,6 +33,8 @@ import { DaffRovingTabIndexService } from './roving-tab-index-group.service'; }, }) export class DaffRovingTabIndexDirective { + private readonly _hydrationWorkaround = signal(false); + /** * Allows the RTI group to be overriden. * By default it will be the nearest ancestor or the default root group if no boundary ancestor exists. @@ -41,7 +45,11 @@ export class DaffRovingTabIndexDirective { * The group in which this RTI target resides. * See {@link DaffRovingTabIndexBoundaryDirective} to make an element act as the boundary of an RTI group. */ - readonly group = computed(() => this.rti() || this.parent?.effectiveBoundary() || ''); + readonly group = computed(() => + this._hydrationWorkaround() + ? this.rti() || this.parent?.effectiveBoundary() || '' + : this.rti() || this.parent?.effectiveBoundary() || '', + ); /** * @docs-private */ @@ -54,7 +62,11 @@ export class DaffRovingTabIndexDirective { constructor( private service: DaffRovingTabIndexService, @Optional() @SkipSelf() @Inject(DAFF_ROVING_TAB_INDEX_BOUNDARY) private parent: DaffRovingTabIndexBoundary, - ) {} + ) { + afterNextRender({ + read: () => this._hydrationWorkaround.set(true), + }); + } /** * @docs-private