From 506aeecdc255ac394f47304c67e9b16a7befd3b9 Mon Sep 17 00:00:00 2001 From: Anton Ramanovich <90370279+rmnvch@users.noreply.github.com> Date: Thu, 22 Jan 2026 07:18:33 +0100 Subject: [PATCH] [ACS-10280] Keyboard Navigation: Search: Unnecessary stops in the results table using the Tab key: (#11529) * [ACS-10280]: removes rows being focusable by default * [ACS-10280]: non fucusable chips by default * [ACS-10280]: datatable UTs * [ACS-10280]: updates focus management for datatable * [ACS-10280]: improves tabfocus for hidden action buttons --- .../datatable-row.component.spec.ts | 16 ++-- .../datatable-row/datatable-row.component.ts | 3 +- .../datatable/datatable.component.html | 26 +++--- .../datatable/datatable.component.scss | 35 +++++--- .../datatable/datatable.component.spec.ts | 87 +++++++------------ .../dynamic-chip-list.component.html | 1 - 6 files changed, 72 insertions(+), 96 deletions(-) diff --git a/lib/core/src/lib/datatable/components/datatable-row/datatable-row.component.spec.ts b/lib/core/src/lib/datatable/components/datatable-row/datatable-row.component.spec.ts index d2fc3f3c5f..06d6934bed 100644 --- a/lib/core/src/lib/datatable/components/datatable-row/datatable-row.component.spec.ts +++ b/lib/core/src/lib/datatable/components/datatable-row/datatable-row.component.spec.ts @@ -90,19 +90,21 @@ describe('DataTableRowComponent', () => { expect(fixture.debugElement.nativeElement.getAttribute('aria-label')).toBe('some-name'); }); - it('should set tabindex as focusable when row is not disabled', () => { + it('should set tabindex as non focusable by default (disabled propery is not passed)', () => { + fixture.detectChanges(); + expect(fixture.debugElement.nativeElement.getAttribute('tabindex')).toBeNull(); + }); + + it('should not set tabindex when row is disabled', () => { + component.disabled = false; fixture.detectChanges(); expect(fixture.debugElement.nativeElement.getAttribute('tabindex')).toBe('0'); }); - it('should not set tabindex when row is disabled', () => { - component.disabled = true; - fixture.detectChanges(); - expect(fixture.debugElement.nativeElement.getAttribute('tabindex')).toBe(null); - }); - it('should focus element', () => { expect(document.activeElement.classList.contains('adf-datatable-row')).toBe(false); + component.disabled = false; + fixture.detectChanges(); component.focus(); expect(document.activeElement.classList.contains('adf-datatable-row')).toBe(true); diff --git a/lib/core/src/lib/datatable/components/datatable-row/datatable-row.component.ts b/lib/core/src/lib/datatable/components/datatable-row/datatable-row.component.ts index b7207737a0..7130c3a81d 100644 --- a/lib/core/src/lib/datatable/components/datatable-row/datatable-row.component.ts +++ b/lib/core/src/lib/datatable/components/datatable-row/datatable-row.component.ts @@ -25,14 +25,13 @@ import { DataRow } from '../../data/data-row.model'; encapsulation: ViewEncapsulation.None, host: { class: 'adf-datatable-row', - tabindex: '0', role: 'row' } }) export class DataTableRowComponent implements FocusableOption { @Input() row: DataRow; - @Input() disabled = false; + @Input() disabled = true; @Output() select: EventEmitter = new EventEmitter(); diff --git a/lib/core/src/lib/datatable/components/datatable/datatable.component.html b/lib/core/src/lib/datatable/components/datatable/datatable.component.html index 0a495a703a..ea030262ed 100644 --- a/lib/core/src/lib/datatable/components/datatable/datatable.component.html +++ b/lib/core/src/lib/datatable/components/datatable/datatable.component.html @@ -12,7 +12,6 @@ cdkDropListOrientation="horizontal" [cdkDropListSortPredicate]="filterDisabledColumns" data-automation-id="datatable-row-header" - [disabled]="!isHeaderVisible()" class="adf-datatable-row" role="row"> @@ -58,7 +57,7 @@ (click)="onColumnHeaderClick(col, $event)" (keyup.enter)="onColumnHeaderClick(col, $event)" role="columnheader" - [attr.tabindex]="isHeaderVisible() ? 0 : null" + [attr.tabindex]="col.sortable ? 0 : null" [attr.aria-sort]="col.sortable ? (getAriaSort(col) | translate) : null" cdkDrag cdkDragLockAxis="x" @@ -214,7 +213,7 @@ [class.adf-datatable-row__dragging]="isDraggingRow" [attr.data-automation-id]="'datatable-row-' + idx" (contextmenu)="markRowAsContextMenuSource(row)" - [disabled]="!(data.allowFocusOnRows ?? true)" + [disabled]="!(data?.allowFocusOnRows ?? true)" >
+ [attr.tabindex]="null">
-
-
-
-
-
+
-
+
@@ -425,7 +422,6 @@ ((actions && actionsPosition === 'right') || (mainActionTemplate && showMainDatatableActions))" role="gridcell" - tabindex="0" class="adf-datatable-cell adf-datatable__actions-cell adf-datatable-center-actions-column-ie adf-datatable-actions-menu"> diff --git a/lib/core/src/lib/datatable/components/datatable/datatable.component.scss b/lib/core/src/lib/datatable/components/datatable/datatable.component.scss index 37aa44d062..b1733bc16f 100644 --- a/lib/core/src/lib/datatable/components/datatable/datatable.component.scss +++ b/lib/core/src/lib/datatable/components/datatable/datatable.component.scss @@ -342,23 +342,32 @@ $data-table-cell-min-width-file-size: $data-table-cell-min-width-1 !default; } } - .adf-datatable-row .adf-datatable-actions-menu { - margin-left: auto; - justify-content: end; - padding-right: 15px; - - &:focus-visible, - &:focus-within { - .adf-datatable-hide-actions-without-hover { + .adf-datatable-row { + &:focus-within, + &:hover { + .adf-datatable-actions-menu .adf-datatable-hide-actions-without-hover { visibility: visible; } } - &-provided { - /* stylelint-disable declaration-no-important */ - max-width: 100px !important; - justify-content: center; - padding-right: 0; + .adf-datatable-actions-menu { + margin-left: auto; + justify-content: end; + padding-right: 15px; + + &:focus-visible, + &:focus-within { + .adf-datatable-hide-actions-without-hover { + visibility: visible; + } + } + + &-provided { + /* stylelint-disable declaration-no-important */ + max-width: 100px !important; + justify-content: center; + padding-right: 0; + } } } diff --git a/lib/core/src/lib/datatable/components/datatable/datatable.component.spec.ts b/lib/core/src/lib/datatable/components/datatable/datatable.component.spec.ts index 0fd567eab3..8a1a0a2853 100644 --- a/lib/core/src/lib/datatable/components/datatable/datatable.component.spec.ts +++ b/lib/core/src/lib/datatable/components/datatable/datatable.component.spec.ts @@ -36,6 +36,7 @@ import { HarnessLoader } from '@angular/cdk/testing'; import { ConfigurableFocusTrapFactory } from '@angular/cdk/a11y'; import { provideRouter } from '@angular/router'; import { firstValueFrom } from 'rxjs'; +import { DataTableAdapter } from '@alfresco/adf-core'; @Component({ selector: 'adf-custom-column-template-component', @@ -1712,52 +1713,14 @@ describe('Accessibility', () => { expect(document.activeElement?.getAttribute('data-automation-id')).toBe('datatable-row-0'); }); - - it('should select header row when `showHeader` is `Always`', () => { - dataTable.showHeader = ShowHeaderMode.Always; - - dataTable.ngOnChanges({ - rows: new SimpleChange(null, dataRows, false) - }); - - fixture.detectChanges(); - dataTable.ngAfterViewInit(); - - const rowElement = testingUtils.getAllByCSS('.adf-datatable-body .adf-datatable-row')[0]; - testingUtils.setDebugElement(rowElement); - testingUtils.clickByCSS('.adf-datatable-cell'); - - fixture.debugElement.nativeElement.dispatchEvent(event); - - expect(document.activeElement?.getAttribute('data-automation-id')).toBe('datatable-row-header'); - }); - - it('should not select header row when `showHeader` is `Never`', () => { - dataTable.showHeader = ShowHeaderMode.Never; - - dataTable.ngOnChanges({ - rows: new SimpleChange(null, dataRows, false) - }); - - fixture.detectChanges(); - dataTable.ngAfterViewInit(); - - const rowElement = testingUtils.getAllByCSS('.adf-datatable-body .adf-datatable-row')[0]; - testingUtils.setDebugElement(rowElement); - testingUtils.clickByCSS('.adf-datatable-cell'); - - fixture.debugElement.nativeElement.dispatchEvent(event); - - expect(document.activeElement?.getAttribute('data-automation-id')).toBe('datatable-row-1'); - }); }); - describe('DataTable row focus management', () => { - const testFocus = (focus: boolean, selector: string, expectedTabindex: string | null) => { - dataTable.showHeader = ShowHeaderMode.Never; + describe('Row cells focus management', () => { + const testFocus = (sortable: boolean, selector: string, expectedTabindex: string | null) => { + dataTable.showHeader = ShowHeaderMode.Always; const dataRows = [{ name: 'name1' }]; - dataTable.data = new ObjectDataTableAdapter([], [new ObjectDataColumn({ key: 'name', template: columnCustomTemplate, focus })]); + dataTable.data = new ObjectDataTableAdapter([], [new ObjectDataColumn({ key: 'name', template: columnCustomTemplate, sortable })]); dataTable.ngOnChanges({ rows: new SimpleChange(null, dataRows, false) @@ -1770,23 +1733,24 @@ describe('Accessibility', () => { expect(element?.nativeElement.getAttribute('tabindex')).toEqual(expectedTabindex); }; - const cellaValSelector = '.adf-datatable-row[data-automation-id="datatable-row-0"] .adf-cell-value'; - const cellWrapperSelector = '.adf-datatable-cell'; + const headerCellSelector = '.adf-datatable-cell-header'; - it('should remove cell focus when [focus] is set to false', () => { - testFocus(false, cellaValSelector, null); + it('should remove header cell focus when cell is not sortable', () => { + testFocus(false, headerCellSelector, null); }); - it('should allow element focus when [focus] is set to true', () => { - testFocus(true, cellaValSelector, '0'); + it('should allow header cell focus when cell is sortable', () => { + testFocus(true, headerCellSelector, '0'); }); - it('should remove col focus when [focus] is set to false', () => { - testFocus(false, cellWrapperSelector, null); + it('should set tabindex equal to null on body cell by default (sortable = false)', () => { + const bodyCellSelector = '.adf-datatable-cell'; + testFocus(false, bodyCellSelector, null); }); - it('should allow col focus when [focus] is set to true', () => { - testFocus(true, cellWrapperSelector, '0'); + it('should set tabindex equal to null on body cell by default (sortable = true)', () => { + const bodyCellSelector = '.adf-datatable-cell'; + testFocus(true, bodyCellSelector, null); }); }); @@ -1802,12 +1766,10 @@ describe('Accessibility', () => { this.allowFocusOnRows = allow; } } - const testAllowFocusOnRows = (allowFocus: boolean, expectedTabindex: string | null) => { + const testAllowFocusOnRows = (expectedTabindex: string | null, adapter: DataTableAdapter) => { const fakeDataRows = [new FakeDataRow(), new FakeDataRow()]; - const adapter = new ShareAdapterMock([], []); adapter.setRows(fakeDataRows); - adapter.setAllowFocusOnTableRows(allowFocus); dataTable.data = adapter; fixture.detectChanges(); const rowElements = testingUtils.getAllByCSS('.adf-datatable-body adf-datatable-row'); @@ -1816,11 +1778,20 @@ describe('Accessibility', () => { }; it('should set tabindex to null (disabled === true) on datatable-body rows when allowFocusOnRows is set to false in ShareDatatable adapter', () => { - testAllowFocusOnRows(false, null); + const adapter = new ShareAdapterMock([], []); + adapter.setAllowFocusOnTableRows(false); + testAllowFocusOnRows(null, adapter); }); - it('should set tabindex to 0 (disabled === false) on datatable-body rows when allowFocusOnRows is set to true in ShareDatatable adapter (default case)', () => { - testAllowFocusOnRows(true, '0'); + it('should set tabindex to 0 (disabled === false) on datatable-body rows when allowFocusOnRows is not set explicitly in ShareDatatable adapter and falls back to default value ', () => { + const adapter = new ShareAdapterMock([], []); + adapter.setAllowFocusOnTableRows(true); + testAllowFocusOnRows('0', adapter); + }); + + it('should set tabindex to 0 (disabled === false) by default on datatable-body rows when allowFocusOnRows is not defined in Datatable adapter (fallback case)', () => { + const adapter = new ObjectDataTableAdapter([], []); + testAllowFocusOnRows('0', adapter); }); }); diff --git a/lib/core/src/lib/dynamic-chip-list/dynamic-chip-list.component.html b/lib/core/src/lib/dynamic-chip-list/dynamic-chip-list.component.html index 6c9d4ef6df..c79610b574 100644 --- a/lib/core/src/lib/dynamic-chip-list/dynamic-chip-list.component.html +++ b/lib/core/src/lib/dynamic-chip-list/dynamic-chip-list.component.html @@ -14,7 +14,6 @@ [style.border-radius]="roundUpChips ? '20px' : '10px'" [style.font-weight]="'bold'" role="listitem" - tabIndex="0" [attr.aria-label]="chip.name" (removed)="removedChip.emit(chip.id)"> {{ chip.name }}