Aman-Mittal opened a new issue, #570:
URL: https://github.com/apache/fineract-backoffice-ui/issues/570
## Business value
When a screen moves from `<ion-button>` to `<app-button>`, attributes
written on the call site land on the `<app-button>` host — not on the
`<ion-button>` the primitive renders inside it. For most attributes that is
invisible. For three it is not, because something else is looking for them in
the old place.
14 list screens were held back from the ADR 0005 migration batch for exactly
this reason. Each is a small change, but each needs a second edit alongside it,
and doing them without that second edit is how a green build ships a broken
selector.
## Why each one matters
**`data-testid` — the e2e suite selects on it.** Several specs name the
vendor tag directly:
```
e2e/teller-cash-management.spec.ts:159
page.locator('ion-button[data-testid="cashier-submit-btn"]')
e2e/teller-cash-management.spec.ts:152
tellerRow.locator('ion-button[data-testid^="manage-cashiers-btn-"]')
```
After migration the attribute is on `<app-button>`, so
`ion-button[data-testid=…]` matches nothing and the spec times out. The
selector has to move in the same change. Prefer `page.getByTestId(…)`, which
does not care which element carries it.
**`title` — it does not do what it looks like it does.**
`e2e/rbac-multi-permission.spec.ts:206` already records the reason: Ionic
forwards `aria-label` into the button it renders in its shadow root, but not
`title`. A `title` on the host names the outer element, not the control. So
these are worth converting rather than relocating — `app-button`'s `[label]`
puts an accessible name where a screen reader will actually find it, and
`[appTooltip]` covers the hover affordance if one is wanted.
**`class`** — styling written against the vendor element does not reach it
through the primitive. Same question as in #569: prefer an input naming the
intent over passing CSS down.
## The files
Run this to regenerate the list:
```bash
grep -rn '<ion-button' src/app --include=*.ts | grep -v spec | grep -E
'data-testid|title=|class='
```
| File | Blocking attribute |
| --- | --- |
| `features/groups/groups-list.component.ts` | `[attr.data-testid]`,
`[title]` |
| `features/organization/offices/offices-list.component.ts` | `data-testid`,
`[title]` |
| `features/system/report-definitions/report-definitions-list.component.ts`
| `data-testid`, `[title]` |
| `features/groups/tabs/group-notes-list.component.ts` | `data-testid` |
| `features/accounting/accounting-closures-list.component.ts` | `[title]` |
| `features/accounting/charges/charges-list.component.ts` | `[title]` |
| `features/accounting/chart-of-accounts.component.ts` | `[title]` |
| `features/centers/centers-list.component.ts` | `[title]` |
| `features/loans/collateral/collateral-list.component.ts` | `title` |
| `features/loans/rescheduling/reschedule-requests-list.component.ts` |
`title` |
| `features/products/fixed-deposits/fixed-deposits-list.component.ts` |
`title` |
|
`features/products/recurring-deposits/recurring-deposits-list.component.ts` |
`title` |
| `features/security/users/users-list.component.ts` | `title` |
| `features/tasks/checker-inbox/checker-inbox.component.ts` | `class` |
## Describing the change
The migration itself is the same substitution the batch already did —
`color` becomes `intent`, `fill` becomes `emphasis`, `[attr.aria-label]`
becomes `[label]`, `[routerLink]` becomes `[link]`, and a child `<ion-icon
name="x">` becomes an `icon="x"` input:
```ts
// before
<ion-button fill="clear" color="danger" data-testid="delete-btn"
[attr.aria-label]="'COMMON.DELETE' | translate"
(click)="onDelete(row)">
<ion-icon name="trash-outline"></ion-icon>
</ion-button>
// after
<app-button type="button" emphasis="quiet" intent="danger"
data-testid="delete-btn"
icon="trash-outline" [label]="'COMMON.DELETE' | translate"
(click)="onDelete(row)" />
```
What is different here is the second edit: before opening the PR, grep the
e2e suite for every test id the file uses and move any
`ion-button[data-testid=…]` selector to `getByTestId`.
## Scope
In scope: these 14 files, the e2e selectors that name them, and converting
`title` to `[label]`/`[appTooltip]`.
Out of scope: the 16 files in #569, which are blocked on `app-icon` rather
than on attribute position.
## Getting started
- Worked example:
`src/app/features/clients/tabs/client-notes-list.component.ts` and its spec,
which asserts the things a rewrite can silently drop — the link's `href`, the
accessible name, the click handler, the icon and the `*appHasPermission` guard.
- `npm run check:ui-primitives` enforces that every `<app-button>` declares
`type` and can be named.
- **Run the mocked e2e project** — `npx playwright test --project=mocked` —
it needs no backend, and it is what catches a moved selector. A migration that
only passes unit tests has not been checked, because almost no existing spec in
`src/app/features` asserts anything about a button.
- After each file, `npm run lint:prune` should remove its suppression entry.
- One pull request per feature directory.
--
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]
