Skip to content

14 list screens cannot migrate because their test id or tooltip sits on the ion-button itself #570

Description

@Aman-Mittal

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:

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:

// 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 projectnpx 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions