Aman-Mittal opened a new issue, #286:
URL: https://github.com/apache/fineract-backoffice-ui/issues/286
## Business value
`src/app/shared/components/data-table/data-table.component.ts:155-165`
renders every sortable column header like this:
```html
<th
cdk-header-cell
*cdkHeaderCellDef
[appTooltip]="col.tooltip || ''"
[attr.aria-sort]="ariaSortFor(col)"
[class.sortable]="col.sortable"
(click)="onSortHeaderClick(col)"
>
```
`aria-sort` is correct and it announces the current sort state properly. But
a `<th>` is not focusable, and there is no `tabindex`, no `role="button"`, and
no key handler. So the header **announces that it is sortable and then cannot
be sorted**, which is worse than not exposing the state at all.
This is one component used at **106 call sites across 103 files**. Sorting
is mouse-only on every list screen in the application — clients, loans,
savings, journal entries, offices, staff, all of them. Anyone working
keyboard-only, and anyone using voice control or a switch device, can read a
list but cannot reorder it. For a back-office application where finding one
record among thousands is the core task, sorting is not a convenience.
It also fails WCAG 2.1 SC 2.1.1 (Keyboard), a Level A criterion.
**Why the linter did not catch it.**
`@angular-eslint/template/click-events-have-key-events` is enabled, but the
element carries `*cdkHeaderCellDef`, and the rule does not fire on elements
inside a structural directive's template in this configuration. The a11y e2e
also misses it: `e2e/accessibility.spec.ts` scopes axe with `.include()` to
`ion-card` and runs on three screens, and axe cannot detect a missing key
handler in any case — that is a manual check.
## Reproducing it
Open any list screen with a sortable column. Press Tab repeatedly. Focus
never lands on a column header. There is no key that sorts.
```
grep -n "onSortHeaderClick\|tabindex\|keydown"
src/app/shared/components/data-table/data-table.component.ts
```
One click handler, no `tabindex`, no `keydown`.
## Describing the change
The header cell needs to be operable, and the accessible role has to match
what it does. Two shapes are reasonable:
**Preferred — a real button inside the `<th>`:**
```html
<th cdk-header-cell *cdkHeaderCellDef [attr.aria-sort]="ariaSortFor(col)">
@if (col.sortable) {
<button type="button" class="sort-button"
(click)="onSortHeaderClick(col)">
{{ col.label | translate }}
<!-- sort indicator -->
</button>
} @else {
{{ col.label | translate }}
}
</th>
```
This gets focusability, Enter and Space activation, and the correct role for
free, with no key handling to write and nothing to get wrong. `aria-sort` stays
on the `<th>`, which is where the spec requires it.
**Alternative — make the `<th>` itself operable** with
`[attr.tabindex]="col.sortable ? 0 : null"`, `[attr.role]="col.sortable ?
'button' : null"` and a `(keydown)` handler for Enter and Space. Fewer DOM
changes, but you must remember `event.preventDefault()` on Space or the page
scrolls, and you take on the role plumbing by hand.
Either is acceptable. Please take the first unless the styling fights you.
**Three things to be careful about.**
1. **Only sortable columns.** A non-sortable header must not be focusable —
putting every header in the tab order makes keyboard navigation of a wide table
considerably worse. Note the current template applies `(click)` unconditionally
and relies on `onSortHeaderClick` ignoring non-sortable columns; the
focusability must be conditional even though the click currently is not.
2. **Do not lose `aria-sort`.** It is already correct. Keep it on the
`<th>`, not on the button.
3. **Check the focus indicator is visible.** The header has its own
background; a default outline may disappear against it. This is a real part of
the fix, not polish — an invisible focus ring makes the feature unusable for
the people it is for.
## Testing
There is no platform acceptance test for this — it is a pure UI concern — so
the tests have to assert the behaviour directly.
**A unit spec** in `data-table.component.spec.ts` (the file exists and is
one of the better specs in the repo):
```ts
it('exposes sortable headers to the keyboard and not unsortable ones', () =>
{ ... });
it('sorts on Enter and on Space', () => { ... });
```
Assert on the rendered DOM, not on the component instance. Most specs in
this repo call a handler directly and check a signal, which would pass
throughout this bug — the entire defect is that nothing reaches the handler.
**A mocked e2e** is the test that genuinely proves it, because it drives
real focus:
```ts
await page.getByRole('button', { name: /office name/i }).focus();
await page.keyboard.press('Enter');
await expect(page.locator('th[aria-sort="ascending"]')).toBeVisible();
```
Reaching the header by keyboard alone (repeated `Tab` from a known starting
point) is a stronger assertion than `.focus()` if you can make it stable.
**Extending the axe coverage is welcome but will not catch this** — axe
cannot detect a missing key handler. If you do touch
`e2e/accessibility.spec.ts`, the more valuable change is widening the
`.include()` scope beyond `ion-card` so the surrounding page is audited at all.
## Scope
In scope: keyboard operability and focus visibility of sort headers in
`app-data-table`.
Out of scope: the clickable table rows in `client-search-v2.component.ts`
and `global-search.component.ts`, which have the same class of problem and
should get their own issue; and the wider accessibility gaps around dialog
names and icon `aria-hidden`.
## Getting started
- `src/app/shared/components/data-table/data-table.component.ts` — template
around `:155`, styles in the same file.
- Existing spec:
`src/app/shared/components/data-table/data-table.component.spec.ts`
- `npm test`, `npm run lint:prune` and `npm run build` must pass.
- This component has 106 call sites. Do not change its inputs or outputs —
the fix should be entirely internal, so no consumer needs touching. If you find
yourself editing a feature component, step back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
