Aman-Mittal opened a new pull request, #618:
URL: https://github.com/apache/fineract-backoffice-ui/pull/618
Fixes #611, fixes #612, fixes #613 — plus one defect the new contrast check
found on the way.
## #611 — a failed load rendered as "No records found."
Two screens threw *while mapping a successful response*. RxJS does not route
that to `error`, so both components' `error:` callbacks never ran and the
failure reached the operator as an ordinary empty state.
| Screen | Cause |
|---|---|
| `/system/entity-mapping` | `JSON.parse` on a body `HttpClient` had already
deserialised → `"[object Object]" is not valid JSON` |
| `/system/report-mailing-jobs` | paged envelope assigned into an
array-typed signal → `t is not iterable` in a downstream `computed` |
Entity mapping is the worse of the two: the sandbox endpoint returns **five
real rows**, and every one was hidden behind "No records found." The generated
client types that response `string` because the spec does, but the request asks
for `application/json` and the endpoint sends an array.
Report mailing jobs is a shape mismatch — the endpoint returns `{
totalFilteredRecords, pageItems }` while the generated type says `Array`.
`loadRunHistory()` in the same file already guarded exactly this; `load()`
never got the same treatment.
Both now normalise through a helper that accepts the array, the JSON string
and the envelope, and throws on anything else, so the caller can render
`app-load-error` with a retry. That component's own doc comment already
described this failure mode; it just was not reachable, because it hangs off
the `error:` path.
The same bare `JSON.parse(raw)` appears at six more call sites listed in
#611. None threw during the sweep, so they are left alone rather than changed
speculatively.
## #612 — idle logout explained nothing
`IdleService` sends `reason=inactivity`. `login.component.ts` compared only
against `session-expired`. So the **common** path — a 15-minute timer during
normal work — dropped the user at a silent login screen, while the rarer 401
path was explained. I hit this by accident mid-session before finding it in the
code.
Both reasons now live in `core/auth/logout-reason.ts` with a message apiece,
and the screen reads that map rather than testing one value, so adding a reason
without copy is a type error. `SESSION_EXPIRED_REASON` is re-exported so
existing importers keep working.
## #613 — primary buttons back under AA
`--ion-color-primary` was pointed at `--primary-strong` (#2471a3, 5.30:1) by
the #587 fix, then back at `--primary-color` (#3498db, **3.15:1**) by a later
commit on the same branch. Ionic pins `--ion-color-primary-contrast` to white;
`primary-strong` is the primary token held to a contrast floor by
`REQUIRES_WHITE_TEXT`, and `primary-color` is *deliberately* exempt because its
label is derived per-colour. Pairing a hardcoded white with the exempt token is
the defect. Restored, along with the `--ion-color-tertiary` block deleted
beside it, which two progressive-loan badges still use.
**Branded deployments were never affected** — `BrandingService` derives a
matching label, which is why a pale accent legitimately gets black text. That
behaviour is preserved: the service now emits the fill next to the companions
derived from the same hex, so a deployment overrides the new default instead of
inheriting it. Without that, a branded pale accent would have kept the default
dark fill and turned its label black.
Two latent traps surfaced while measuring:
- `var(--primary-dark, #1b4f72)` **never reached its accessible fallback** —
`--primary-dark` is defined as `#2980b9`, which is 4.30:1, under AA. Restoring
that line verbatim would have shipped another failure.
- Routing `-shade` through `--primary-text` would have put white on
`#5dade2` in dark mode, since that token flips light.
Both are literals now, with the reasoning in the stylesheet.
## Found by the new check: dark-mode secondary at 1.15:1
`--secondary-color` turns near-white (`#ecf0f1`) under
`[data-theme='dark']`, and `--ion-color-secondary-contrast` stays white — so
every `color="secondary"` button rendered a white label on near-white. Verified
on the deployed build: the Submit control on Survey Responses measures
**1.15:1**, an effectively invisible label. Same defect class as #613, so it is
fixed here rather than the check being weakened to let it through.
## The regression guard
`scripts/check-theme-contrast.mjs` (wired into `package.json` and CI, beside
`check:responsive`) asserts that every Ionic slot pinning a literal contrast
clears 4.5:1, in both themes. It resolves `var()` to the token's real value
rather than its fallback, which is what makes the `--primary-dark` trap visible.
Run against the pre-fix stylesheet it reports all seven failures rather than
crashing:
```
--ion-color-primary (light): #3498db on #ffffff is 3.15:1, below the 4.5:1
AA floor.
--ion-color-primary (dark): #3498db on #ffffff is 3.15:1, below the 4.5:1 AA
floor.
--ion-color-secondary (dark): #ecf0f1 on #ffffff is 1.15:1, below the 4.5:1
AA floor.
--ion-color-tertiary (light): --ion-color-tertiary is not declared
--ion-color-tertiary (dark): --ion-color-tertiary is not declared
--ion-color-primary-shade (light): #2e86c1 is 3.97:1 against the button
label, below the 4.5:1 AA floor.
--ion-color-primary-shade (dark): #2e86c1 is 3.97:1 against the button
label, below the 4.5:1 AA floor.
```
This is the check whose absence let #587's fix be reverted and shipped.
## Tests
**Unit — 1562 passing, up from 1535.** New coverage: both
payload-normalising helpers across array / JSON-string / envelope / null /
unreadable shapes; the failure and recovery transitions on both screens; the
reason→message mapping including an unknown reason; the branding fill emitted
beside its derived label, and the pale-accent pairing.
**e2e — 37 passing** across four mocked specs. Two are new:
- `e2e/list-load-failure.spec.ts` — rows render when the endpoint returns
its real shape; a 200 carrying an unreadable payload shows the error state and
*not* "No records found."; retry recovers.
- `e2e/logout-reason-notice.spec.ts` — each reason gets its own wording, and
an absent or unrecognised reason shows nothing rather than a blank notice.
`e2e/branded-deployment.spec.ts` and `e2e/deployment-customization.spec.ts`
are updated where they asserted the pre-#613 contract, and otherwise pass
unchanged — including the pale-accent test, which is what caught my first
attempt at this fix.
Also green: `npm run build`, `lint`, `format:check`, `typecheck:e2e`,
`test:scripts`, `test:eslint-rules` and all 12 `check:*` scripts.
**One pre-existing failure, unrelated:** `accessibility.spec.ts:344` fails
identically on unmodified `main` and on this branch. I verified it against a
clean checkout rather than assume; it is not touched here.
--
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]
