You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "opensource-joe (via GitHub)" <gi...@apache.org> on 2026/08/15 17:56:49 UTC

[I] 44 icon-only buttons have no accessible name: appTooltip sets aria-describedby, which does not name a control [fineract-backoffice-ui]

opensource-joe opened a new issue, #354:
URL: https://github.com/apache/fineract-backoffice-ui/issues/354

   ## What is wrong
   
   44 icon-only buttons have no accessible name. They carry `appTooltip` and nothing else, and `appTooltip` does not name a control.
   
   This is the follow-up I offered in [#338](https://github.com/apache/fineract-backoffice-ui/pull/338), which fixed 34 of the buttons #233 identified and left these behind. On coming back to them I found the reason they were left is more interesting than "stragglers", so this issue re-states the problem rather than just carrying the number over.
   
   #233 says, correctly for the pattern as intended, that a tooltip "also gives them their accessible name". The directive does not do that. From `src/app/shared/directives/tooltip.directive.ts`:
   
   ```ts
   this.renderer.setAttribute(this.host.nativeElement, 'aria-describedby', this.id);
   ```
   
   Three things follow from `aria-describedby` rather than `aria-label`:
   
   1. **A description is not a name.** In the accessible name computation, `aria-describedby` is never consulted. It supplements a name, it does not supply one. A button with a description and no name is announced as "button", and the description is read after that, if the user's settings read descriptions at all.
   2. **The attribute does not exist at rest.** It is set in `show()`, 300ms after `mouseenter` or `focusin`, and removed again in `hide()`. So for anything inspecting the button in its resting state, including axe, there is no `aria-describedby` there to find.
   3. **Browse mode never triggers it.** `show()` is bound to `mouseenter` and `focusin`. A screen-reader user reading through a table in browse mode is not focusing these buttons and is not hovering them, so the tooltip never fires and the description never exists.
   
   The 77 icon-only buttons that also carry `[attr.aria-label]` are fine, and they are fine because of the `aria-label`, not the tooltip. The 44 in this issue rely on the tooltip alone.
   
   ```html
   <!-- one of the 44, src/app/features/clients/tabs/client-notes-list.component.ts -->
   <ion-button fill="clear" [appTooltip]="'COMMON.EDIT' | translate" (click)="onEdit(note)">
     <ion-icon name="create-outline"></ion-icon>
   </ion-button>
   ```
   
   ## Business value
   
   Same as #233, and the same rows: on a row of icon buttons the icon is the only thing separating edit from delete, and a screen-reader user gets "button, button, button" with nothing to choose from. Several of these sit on client documents, client identifiers, loan documents and delinquency management, where the wrong guess destroys a record.
   
   The difference from #233 is that these 44 look correct in review. They have a tooltip, the tooltip has a translated string, and the pattern matches the 246 uses elsewhere. Nothing about reading the template says the button is unnamed, which is why they survived the first pass. That makes this worth fixing as a stated rule rather than a one-off sweep: `appTooltip` describes, `aria-label` names, and an icon-only control needs both.
   
   ## Finding them
   
   Measured against `main` at 98770c4:
   
   ```
   elements with appTooltip:    256
     of which icon-only:        121
       already have a name:      77
       UNNAMED:                  44   (23 files)
   ```
   
   Element-level rather than line-level, since a tag's attributes and its content span several lines here:
   
   ```bash
   python3 - <<'PY'
   import pathlib, re
   TAG = re.compile(r"<([a-zA-Z][\w-]*)((?:[^<>]|\"[^\"]*\")*?)(/?)>", re.S)
   hits = 0
   for p in sorted(pathlib.Path('src').rglob('*.ts')):
       if p.name.endswith('.spec.ts'): continue
       s = p.read_text(errors='ignore')
       if 'appTooltip' not in s: continue
       for m in TAG.finditer(s):
           attrs = m.group(2)
           if 'appTooltip' not in attrs: continue
           if re.search(r'\baria-label(ledby)?\b', attrs): continue
           close = s.find('</%s>' % m.group(1), m.end())
           inner = s[m.end():close] if close > 0 else ''
           text = re.sub(r'<[^>]+>', '', re.sub(r'<!--.*?-->', '', inner, flags=re.S)).strip()
           if '<ion-icon' in inner and not text:
               hits += 1
               print('%s:%d' % (p, s[:m.start()].count('\n') + 1))
   print(hits, 'unnamed icon-only buttons')
   PY
   ```
   
   The 23 files, worst first: `client-view.component.ts` (9), `delinquency-management.component.ts` (4), then two each in `client-documents-list`, `client-family-members-list`, `client-identifiers-list`, `client-notes-list`, `loan-documents-tab`, `loan-form`, `fixed-deposit-form`, `recurring-deposit-form`, `savings-account-form`, `datatables-list`, and one each in eleven more.
   
   ## Scope
   
   Add `[attr.aria-label]` bound to a translation key on each of the 44, reusing the existing key wherever the tooltip already names the action, exactly as #338 did for the other 34. The tooltips stay: they are useful to sighted users and they are the correct use of `aria-describedby` once a name exists alongside them.
   
   Two questions worth settling here rather than in review:
   
   - **Should the directive do this itself?** It knows the string. It could set `aria-label` when the host has no name, which would fix all 44 and prevent the next one. I did not propose that in #338 because it changes shared behaviour for 256 call sites, some of which are not icon-only and already have text, and a directive that silently renames controls is harder to reason about than an explicit attribute. Happy to do it either way, and I would rather be told than choose.
   - **A lint rule or a test** so this does not regrow. `sonarjs` will not catch it, but a spec that walks the templates the way the script above does would.
   
   I am picking this up unless someone else already has it. I will hold off opening a PR for a few days in case the directive question changes the shape of the fix.
   


-- 
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: issues-unsubscribe@fineract.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] 44 icon-only buttons have no accessible name: appTooltip sets aria-describedby, which does not name a control [fineract-backoffice-ui]

Posted by "opensource-joe (via GitHub)" <gi...@apache.org>.
opensource-joe commented on issue #354:
URL: https://github.com/apache/fineract-backoffice-ui/issues/354#issuecomment-5303519493

   @Aman-Mittal flagging this one for you since it comes out of #233 and #338, which you reviewed and merged.
   
   The short version: the 44 buttons #338 left behind are not stragglers. `TooltipDirective` sets `aria-describedby` and never `aria-label`, and only while the tooltip is on screen, so the premise in #233 that the tooltip supplies the accessible name does not hold. That is written up above with the source and the re-derived counts.
   
   The only thing I need from you is the question at the end: **should the directive name the host itself, or should this be 44 more `[attr.aria-label]` attributes?**
   
   Being upfront so the answer is not skewed by workload: I already have the directive version written and passing (940 tests, `lint`, `lint:prune`, `format:check` all clean). I have deliberately not opened it as a PR, because #338 asked whether this follow-up was wanted at all and it would be poor form to answer my own question by just showing up with the code. So treat this purely as a preference call about shared behaviour across 256 call sites, not as an effort question. I am equally happy to throw that branch away and do the 44 attributes if you would rather keep the directive dumb.
   


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] 44 icon-only buttons have no accessible name: appTooltip sets aria-describedby, which does not name a control [fineract-backoffice-ui]

Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal closed issue #354: 44 icon-only buttons have no accessible name: appTooltip sets aria-describedby, which does not name a control
URL: https://github.com/apache/fineract-backoffice-ui/issues/354


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] 44 icon-only buttons have no accessible name: appTooltip sets aria-describedby, which does not name a control [fineract-backoffice-ui]

Posted by "Aman-Mittal (via GitHub)" <gi...@apache.org>.
Aman-Mittal commented on issue #354:
URL: https://github.com/apache/fineract-backoffice-ui/issues/354#issuecomment-5303648599

   Your analysis holds — `aria-describedby` describes, it does not name, and setting it only while the tooltip is on screen means the button has no accessible name at the moment a screen reader user is deciding whether to press it. The 44 are real.
   
   My answer is the 44 attributes, and it is not about your having the branch written. It is this: `appTooltip` is on 255 hosts, and only 132 are `ion-button`. 108 are `ion-item`, and the rest are `ion-icon`, `ion-chip`, `ion-card-title`, `th`, `span`, `ion-searchbar`. So a directive that names its host would be doing name-management across 255 elements to fix 44, and would need guards to be a no-op on the other 211 — including not overriding the visible text on the `ion-item`s, which would break Label in Name and speech input, and not putting an `aria-label` on `th`/`span`/`ion-card-title`, where it has no role to attach to. A directive whose correctness depends on that many exclusions is not shared behaviour, it is a heuristic with a selector.
   
   The other half: the tooltip text is a description by construction. On these 44 it happens to also be a good name (`COMMON.EDIT`, `LOANS.APPROVE`), but nothing keeps it that way — the moment someone puts a `HELP.*_DESC` on an icon-only button, the directive silently produces a sentence-long accessible name. `[attr.aria-label]` at the call site keeps name and description independently choosable, which is what they are.
   
   So: 44 `[attr.aria-label]` attributes, and please pair them with something that stops this regrowing — a unit test or lint rule that fails on an icon-only `ion-button` with no accessible name. That guard is the part that has lasting value, and it works regardless of what the directive does.
   
   Two notes for the PR:
   
   - `tooltip.directive.ts` has no spec, while `has-permission` and `has-institution-feature` both do. Not your issue to fix, but if you are in there anyway it would be welcome.
   - On these 44 the label and the description will be the same string, so a focused screen reader user hears it twice once the tooltip appears. I am fine with that; flagging it so it does not come up in review as a surprise.
   
   Sorry to send you back for the branch you already have. You asked the question the right way round.


-- 
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: issues-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org