opensource-joe opened a new pull request, #338: URL: https://github.com/apache/fineract-backoffice-ui/pull/338
## What and why 34 icon-only `ion-button`s across 19 files had no accessible name, so a screen reader announced each one as just "button". On a row of icon buttons the icon is the only thing separating edit from delete, and several of these actions are destructive: deleting an accounting rule, removing a datatable column, re-opening a closed accounting period. Each button now carries `[attr.aria-label]` bound to a translation key, which is the pattern the 100 already-labelled buttons in the app use. Existing keys are reused wherever one fits (`COMMON.EDIT`, `COMMON.DELETE`, `ACTIONS.APPROVE`, `ACCOUNTING_CLOSURES.REOPEN` and so on). 9 new keys are added for actions that had none. Two smaller things came with it: * Eight buttons carried a native `title` (five hardcoded English, three translated). Those are removed. As `tooltip.directive.ts` documents in its own header, the native `title` does not surface from an Ionic component's shadow host, so it was neither naming the button nor reliably showing help text. * `accounting-rules-list` and `financial-activity-mappings-list` had no translation pipe at all. They now use the I18N adapter's `appTranslate` pipe per `DOCS/adr/0003-adapter-boundary.md`, rather than adding two new direct `@ngx-translate/core` imports. Closes #233 ## One thing worth flagging before review The issue says the 246 `appTooltip` uses are what give those buttons their accessible name. That does not appear to be the case, and it changes what "done" means here. `TooltipDirective` sets `aria-describedby`, and only while the tooltip is visible (`src/app/shared/directives/tooltip.directive.ts`, `show()` and `hide()`). `aria-describedby` supplies a description, not a name, so a button whose only annotation is `appTooltip` still has no accessible name and still fails WCAG 4.1.2. Measured across `src/app`, counting `ion-button`s whose only content is an `ion-icon`: | | count | |---|---| | icon-only buttons total | 187 | | with `aria-label` (real accessible name) | 109 | | without | 78 | This PR fixes 34 of those 78. The other 44 have an `appTooltip` and no `aria-label`, and the detection script in the issue skips them for exactly that reason. Happy to follow up with a second PR covering those 44 if you agree with the reading, or to fold them into this one if you would rather have it in a single change. Two smaller notes on counting. The issue says 38 in 19 files; I get 34 strictly icon-only in 19 files. The other 4 (`journal-entry-form` "Add Debit" and "Add Credit", `clients-list`, `asset-owner-view`) do have visible text, so they already have an accessible name. Their text is hardcoded English rather than a translation key, which is a real bug but an i18n one, so I left them out of an accessibility fix. Say the word and I will raise it separately. ## Verification All run against this branch in a clean container (node 22.23.2): * `npm run lint:prune` passes, and `eslint-suppressions.json` is unchanged, so no suppressed violation was disturbed. * `npm run format:check`, `npx eslint "src/**/*.html"`, `npm run i18n:check`, `npm run check:icons`, `./scripts/check-license.sh` all pass. * `npm run build` passes. It was the build that caught the two components missing a translation pipe, which lint alone did not. * `npm test` passes: 897 of 897, up from 896. * `./scripts/verify-signed-commits.sh` reports 0 unsigned commits. On the added test, I checked it is not vacuous: stripping the two `aria-label`s back out of `business-steps.component.ts` turns it red (1 FAILED, 896 SUCCESS), and restoring them turns it green again. I also confirmed the mechanism rather than assuming it. `@ionic/core`'s button calls `inheritAriaAttributes(this.el)` and spreads the result onto the inner `button.button-native`, so an `aria-label` on the `<ion-button>` host does reach the native button and does become its accessible name. ## Screenshots No visual change. `aria-label` renders nothing on screen, and the elements are unchanged in size and position. The one user-visible difference is that the eight buttons that had a native `title` no longer show a browser tooltip on hover. For five of them that tooltip was untranslated English. If you want hover help preserved on any of these, the house pattern is `appTooltip`, and I am glad to add it alongside the label. ## Checklist - [x] I did not hand-edit generated files under `src/app/api/`. - [x] New component or service code uses the adapter boundary in `src/app/core/adapters/` instead of direct browser globals or imperative third-party APIs. - [x] User-facing strings use translation keys. - [x] I added or updated tests appropriate to this change, or explained why tests were not needed. - [x] UI workflow changes include suitable e2e coverage, including real-backend testing where relevant. No workflow changes here: this adds an attribute and removes a `title`, with no change to what any button does. Existing e2e specs select on `data-testid` and `id`, neither of which moved. - [x] Commits are signed. -- 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]
