| name | opal-frontend-common-ui-lib-review-guidelines |
| description | Automated review rules for opal-frontend-common-ui-lib PRs. Use when asked to review code, run @codex review, or provide PR feedback; apply severity levels and the required comment format. |
Opal Frontend Common UI Lib Review Guidelines
Overview
Apply these rules when reviewing changes in opal-frontend-common-ui-lib; focus on P0/P1 blockers and use the required comment format.
Related skills
- For changes that touch Vitest config, Angular test setup, or VS Code Vitest Explorer support, also use
opal-vitest-guard.
How to use
- Apply these rules to changes in this PR only.
- Prefer specific, line-anchored comments with a short rationale and a concrete fix.
- Treat P0 and P1 as blocking; P2 as advisory.
Scope of review
Automated review should only analyze and comment on project-owned code within this repository.
In particular:
- Focus on TypeScript, templates, and styles under
projects/opal-frontend-common/**, plus configuration that affects runtime behavior.
- Ignore generated files, build outputs (including
dist/ and coverage/), and node_modules.
Severity definitions
- P0 (blocker): Security risk, data loss, broken UX/AX, build/test failure, or architectural violation that will be hard to undo.
- P1 (high): Regressions in quality, maintainability, or performance with simple fixes.
- P2 (advice): Stylistic or non-critical improvements.
Repo scope
Angular v20+ standalone library using GOV.UK/HMCTS design system. Prefer modern Angular primitives (standalone components, template control flow, signals) and accessibility to WCAG 2.2 AA.
P0 rules (must block)
- Security & safety
- Unsanitised HTML or
bypassSecurityTrust* without justification and tests.
- Interpolating user data into
[innerHTML]/[srcdoc]/style, or unsafe URL handling.
- Credentials, tokens, secrets, or PII in code, logs, comments, or tests.
- Accessibility (AX)
- Interactive elements not reachable by keyboard,
click handlers on non-interactive elements without proper roles/tabindex.
- Missing visible label/
aria-label for form controls or buttons; images without meaningful alt.
- Architecture & build integrity
- New Angular modules where a standalone component/route/provider is appropriate.
- Mixing signals and imperative RxJS within the same component in a way that causes side-effects in template evaluation.
- Using barrel exports (
index.ts, export *) or importing via barrels. Prefer direct, specific imports from the declaring file. Keep features self-contained by default.
- CI/test failures, TypeScript errors, or missing required checks.
P1 rules (high priority)
- Angular correctness
- Prefer
@if, @for, @switch over legacy structural directives in new/changed templates.
- Use computed signals and pure functions for derived state; avoid calling methods with side effects in templates.
- Choose RxJS concurrency intentionally:
switchMap for latest-only, exhaustMap for form submit, concatMap when order matters.
- Code quality fundamentals
- Use clear, descriptive names for symbols, inputs/outputs, and tests; avoid abbreviations that obscure intent.
- Keep components and services small and cohesive; extract helpers to keep implementations readable.
- Prefer simple, readable code over cleverness; add comments that explain why decisions were made.
- Apply modern Angular features (standalone components, signals, control flow) in new/changed code.
- Performance
- Avoid heavy work in templates (no
.map()/.filter() or non-pure pipes inside bindings).
- Lazy-load routes and large feature areas; avoid broad shared providers when a standalone provider suffices.
- Guard against large third-party dependencies; if introduced, note size and reason.
- Testing
- Add/maintain tests for new logic and error/empty states.
- Prefer Angular Testing Library/Harnesses; avoid brittle DOM selectors/data-testids when a Harness exists.
- Function design
- Prefer small, single-purpose, pure functions.
- Keep cyclomatic complexity low.
- Pass explicit inputs and return data rather than performing side effects.
P2 rules (advisory)
- Prefer container (smart) vs. presentational component separation when complexity grows.
- Keep features self-contained by default. No barrels (
index.ts, export *) -- prefer direct file imports for tree-shaking.
- Provide brief inline docs when introducing patterns others should copy.
What to ignore (unless requested)
- Typos in comments/docs (treat as P2 unless critical).
- Pure formatting churn without semantic change.
Comment style for the reviewer (automated)
Use this shape to keep feedback crisp and useful:
Examples
-
P0: Unsanitised HTML
Problem: [innerHTML] used with dynamic user content in case-summary.component.html.
Why: XSS risk.
Fix: Remove innerHTML, render via bindings/DOM APIs, or use a trusted, sanitised subset with tests.
-
P1: Template work
Problem: Method call getItems() used in template; performs filtering.
Why: Re-runs each change detection; performance risk.
Fix: Move to computed signal itemsFiltered = computed(() => ...) and bind to that.
-
P1: Concurrency
Problem: Form submit uses mergeMap, causing double submits.
Why: Users can trigger parallel requests.
Fix: Use exhaustMap around submit stream and disable button while pending.
Design system references
