Aman-Mittal opened a new pull request, #566:
URL: https://github.com/apache/fineract-backoffice-ui/pull/566

   ## What and why
   
   `eslint-suppressions.json` counts violations per rule id. ADR 0005 put the 
Ionic component boundary on `no-restricted-imports`, which already carried ADR 
0003's Material and `@ngx-translate` backlogs, and seeded ~280 Ionic imports 
into that shared counter — making the three boundaries fungible. This moves the 
component boundary onto a local rule of its own, and fixes three defects found 
in the same code.
   
   Closes #565
   
   ## The boundary split
   
   Verified against `main` before the change: in 
`accounting-closure-form.component.ts` (count `2`), adding a second 
`@ngx-translate/core` import and deleting the Ionic import block produces 
**no** `no-restricted-imports` error. Before ADR 0005, when that file's count 
was `1`, the same edit failed.
   
   | Rule | `main` | This branch |
   | --- | --- | --- |
   | `no-restricted-imports` | 588 | **305** |
   | `local/no-vendor-ui-import` | — | **291** |
   
   The 305 is the pre-ADR-0005 baseline exactly, checked per file: the only 
difference from that baseline is `entity-datatables.component.ts` going 1 → 0, 
the `TranslateModule` import ADR 0005 genuinely removed.
   
   **The old seeding also under-counted.** 291 is nine more than the 282 ADR 
0005 added. Eight of those are files importing Ionic components *and* 
`ModalController` in one statement — the shared counter charged them once, 
because the pre-existing controller violation already filled the slot, so their 
component imports were never on the books. The ninth is 
`entity-datatables.component.ts`, whose count was previously spent on i18n. 
`ignoreNames` keeps a controller-only import on the ADR 0003 boundary alone, so 
migrating it to OVERLAY decrements one counter, not two.
   
   `scripts/ui-boundary.test.mjs` now asserts *which* rule reports rather than 
that something did, so re-merging the counters fails instead of passing quietly.
   
   ## Three defects fixed alongside
   
   1. **Tab stop left a disabled selection.** `focusIndex` skipped a 
selected-but-disabled tab, so `aria-selected="true"` and `tabindex="0"` named 
different buttons and Tab entered the strip on a tab whose panel was not 
displayed. The selected tab keeps the tab stop; arrows still refuse to settle 
on it.
   
   2. **`idPrefix` was interpolated raw** while tab values were escaped, and 
the one caller builds it from the apptable name. A space or quote yields a 
malformed `id` with no error — `aria-controls`/`aria-labelledby` present, 
pointing at nothing. Both ids now derive from one escaped prefix.
   
   3. **`EntityDatatablesComponent` could show one table's rows under another's 
headers.** Nothing cancels the in-flight request, so switching custom-field 
tabs A → B → A leaves two running and the slower one writes `tableData` last, 
regardless of selection. Columns come from `activeTable()` and switch 
synchronously; rows do not. Guarded on the response still belonging to the 
selected table, and the previous rows are dropped as the request goes out. This 
predates ADR 0005 — the Ionic segment had the same structure.
   
   ## Verification
   
   - Full unit suite: **1493 tests across 242 files pass**. `npm run 
test:eslint-rules` (27) and `npm run test:scripts` (8, including the 5 boundary 
contract tests) pass.
   - `npm run lint` is clean, and `npm run lint:prune` produces no diff, so the 
new baseline is exact rather than padded.
   - The two new `EntityDatatablesComponent` cases were confirmed to **fail 
with the fix reverted** (`expected [ Array(1) ] to deeply equal []`), so they 
are not vacuous.
   - `npm run typecheck:e2e`, `check:a11y-names`, `check:responsive`, 
`check:icons`, `i18n:check`, `check:nav-ids`, `check:route-permissions` all 
pass. Prettier clean on every changed file.
   - Not run locally: e2e (needs browsers and a backend). The existing 
`group-detail.spec.ts` tab cases cover the ARIA contract; the request race is 
covered by unit tests instead, since it needs controlled response ordering.
   
   ## Screenshots
   
   Not applicable — no visual change. The tab-stop fix changes which button 
receives focus, which the unit tests assert directly.
   
   ## AI assistance (optional)
   
   - Tool / model: Claude Opus 5, via Claude Code.
   - Harness / workflow: assisted investigation, implementation and drafting. 
The suppression-counter claim was reproduced by editing a real source file and 
running ESLint; the per-file baseline comparison and the revert-and-rerun check 
on the new tests were run locally.
   
   ## 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. — No strings added; 
registered table names remain user data.
   - [x] I added or updated tests appropriate to this change, or explained why 
tests were not needed. — New rule unit tests, rewritten boundary contract 
tests, and rendered tests for each of the three defects.
   - [x] UI workflow changes include suitable e2e coverage, including 
real-backend testing where relevant. — The tab strip's browser coverage in 
`group-detail.spec.ts` is unchanged and still applies; the three fixes are 
covered by rendered unit tests, as noted under Verification.
   - [x] Commits are signed — see [Commit 
Signing](CONTRIBUTING.md#commit-signing) in CONTRIBUTING.md.
   - [x] I followed the [AI-assisted contributions 
guidance](CONTRIBUTING.md#ai-assisted-contributions).
   


-- 
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]

Reply via email to