VIBE-236 - Update accessibility statement content #146
Conversation
WalkthroughBack-to-top selection changed to find an anchor inside a container. Accessibility statement data (EN/CY) expanded with many structured fields (contacts, links, assistive options). Template and tests updated to render and assert new fields. Footer accessibility link now opens in a new tab with security attributes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/web-core/src/pages/accessibility-statement/en.ts (1)
19-23: Minor inconsistency:abilityNetPrefixmissing in English but present in Welsh.The English locale is missing
abilityNetPrefixwhich exists incy.ts(Line 20:abilityNetPrefix: "Mae gan "). The template at Line 18 conditionally renders this prefix only if it exists, so this works, but the structural inconsistency may cause confusion.Consider adding an empty string or the appropriate English prefix for structural parity:
simpleLanguage: "We've also made the website text as simple as possible to understand.", + abilityNetPrefix: "", abilityNetLink: "AbilityNet (opens in a new window)",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/web-core/src/assets/js/back-to-top.test.ts(7 hunks)libs/web-core/src/assets/js/back-to-top.ts(1 hunks)libs/web-core/src/pages/accessibility-statement/cy.ts(1 hunks)libs/web-core/src/pages/accessibility-statement/en.ts(1 hunks)libs/web-core/src/pages/accessibility-statement/index.njk(2 hunks)libs/web-core/src/pages/accessibility-statement/index.njk.test.ts(4 hunks)libs/web-core/src/pages/accessibility-statement/index.test.ts(1 hunks)libs/web-core/src/views/components/site-footer.njk(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and Interfaces must use PascalCase (e.g.,UserService,CaseRepository). Do NOT useIprefix for interfaces (useUserRepositorynotIUserRepository).
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: consts outside function scope at top, exported functions next, other functions ordered by usage, interfaces and types at bottom.
TypeScript must use strict mode enabled with noanywithout justification. Use workspace aliases (@hmcts/*) for imports.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
All input endpoints must validate inputs. Use parameterized database queries with Prisma. Never include sensitive data in logs.
Only export functions that are intended to be used outside the module. Don't export functions solely for testing purposes.
Only add comments when they are meaningful. Explain why something is done, not what is done.
Favor functional style with simple functional approaches. Don't use a class unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.
Use Express version 5.x with proper async/await error handling in middleware and route handlers.
Files:
libs/web-core/src/pages/accessibility-statement/index.njk.test.tslibs/web-core/src/assets/js/back-to-top.test.tslibs/web-core/src/pages/accessibility-statement/index.test.tslibs/web-core/src/assets/js/back-to-top.tslibs/web-core/src/pages/accessibility-statement/en.tslibs/web-core/src/pages/accessibility-statement/cy.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
API endpoints must use plural for collections (
/api/cases,/api/users), singular for specific resources (/api/case/:id), and singular for creation (POST /api/case).
Files:
libs/web-core/src/pages/accessibility-statement/index.njk.test.tslibs/web-core/src/assets/js/back-to-top.test.tslibs/web-core/src/pages/accessibility-statement/index.test.tslibs/web-core/src/assets/js/back-to-top.tslibs/web-core/src/pages/accessibility-statement/en.tslibs/web-core/src/pages/accessibility-statement/cy.ts
**/{pages,locales}/**/*.{ts,njk}
📄 CodeRabbit inference engine (CLAUDE.md)
Every page must support both English and Welsh by providing
enandcyobjects in controllers and maintaining matching structure in locale files (libs/[module]/src/locales/en.ts and cy.ts).
Files:
libs/web-core/src/pages/accessibility-statement/index.njk.test.tslibs/web-core/src/pages/accessibility-statement/index.test.tslibs/web-core/src/pages/accessibility-statement/index.njklibs/web-core/src/pages/accessibility-statement/en.tslibs/web-core/src/pages/accessibility-statement/cy.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All packages must use
"test": "vitest run"as the test script. Unit and integration tests must be co-located with source code as*.test.tsfiles.
Files:
libs/web-core/src/pages/accessibility-statement/index.njk.test.tslibs/web-core/src/assets/js/back-to-top.test.tslibs/web-core/src/pages/accessibility-statement/index.test.ts
**/pages/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Page routes are created based on file names within the
pages/directory. Nested routes are created using subdirectories (e.g.,pages/admin/my-page.tsbecomes/admin/my-page).
Files:
libs/web-core/src/pages/accessibility-statement/index.njk.test.tslibs/web-core/src/pages/accessibility-statement/index.test.tslibs/web-core/src/pages/accessibility-statement/en.tslibs/web-core/src/pages/accessibility-statement/cy.ts
**/{locales,pages}/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Shared/common content (button text, phase banner, service name, common errors) should be in locale files (libs/[module]/src/locales/en.ts and cy.ts). Page-specific content should be in controllers.
Files:
libs/web-core/src/pages/accessibility-statement/index.njk.test.tslibs/web-core/src/pages/accessibility-statement/index.test.tslibs/web-core/src/pages/accessibility-statement/en.tslibs/web-core/src/pages/accessibility-statement/cy.ts
🧠 Learnings (4)
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/{pages,locales}/**/*.{ts,njk} : Every page must support both English and Welsh by providing `en` and `cy` objects in controllers and maintaining matching structure in locale files (libs/[module]/src/locales/en.ts and cy.ts).
Applied to files:
libs/web-core/src/pages/accessibility-statement/index.njk.test.tslibs/web-core/src/pages/accessibility-statement/index.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/{locales,pages}/**/*.ts : Shared/common content (button text, phase banner, service name, common errors) should be in locale files (libs/[module]/src/locales/en.ts and cy.ts). Page-specific content should be in controllers.
Applied to files:
libs/web-core/src/pages/accessibility-statement/index.njk.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/pages/*.ts : Page controllers must export GET and/or POST functions with signature `(req: Request, res: Response) => Promise<void>`. Content (titles, descriptions) should be organized as `en` and `cy` objects.
Applied to files:
libs/web-core/src/pages/accessibility-statement/index.test.ts
📚 Learning: 2025-11-25T06:40:24.099Z
Learnt from: CR
Repo: hmcts/cath-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T06:40:24.099Z
Learning: Applies to **/pages/*.njk : Nunjucks templates must extend `layouts/default.njk` and use GOV.UK Design System macros. Every page must support both English and Welsh content.
Applied to files:
libs/web-core/src/pages/accessibility-statement/index.njk
🧬 Code graph analysis (2)
libs/web-core/src/pages/accessibility-statement/index.njk.test.ts (2)
libs/web-core/src/pages/accessibility-statement/en.ts (1)
en(1-104)libs/web-core/src/pages/accessibility-statement/cy.ts (1)
cy(1-105)
libs/web-core/src/assets/js/back-to-top.test.ts (1)
libs/web-core/src/assets/js/back-to-top.ts (1)
initBackToTop(1-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarQube Analysis
- GitHub Check: E2E Tests
🔇 Additional comments (19)
libs/web-core/src/assets/js/back-to-top.test.ts (6)
19-36: LGTM!The test correctly reflects the new DOM structure where the anchor is nested inside a paragraph container. The dynamic import pattern ensures proper module isolation.
38-59: LGTM!Good coverage of the preventDefault behavior with the updated selector.
61-75: LGTM!Edge case coverage for missing elements is maintained.
77-96: LGTM!Good test for nested content within the anchor element.
98-122: LGTM!Important edge case: verifies that only the first back-to-top link receives the handler when multiple exist. This matches the
querySelectorbehavior in the implementation.
124-141: LGTM!Remaining tests properly verify scroll behavior details with the updated DOM structure.
Also applies to: 143-161, 163-181
libs/web-core/src/pages/accessibility-statement/index.test.ts (1)
16-33: LGTM!The test correctly validates both English and Welsh locale objects include the new
backToTopfield alongside the existingtitle. This ensures bilingual consistency as required by the coding guidelines.libs/web-core/src/views/components/site-footer.njk (1)
19-26: Verify: Opening internal link in new tab is intentional?The
/accessibility-statementis an internal route, but it's configured to open in a new tab (target="_blank"). Other footer links (including external gov.uk links) don't use this pattern.If this is intentional (e.g., to preserve user context during a flow), the security attributes are correctly applied. However, opening internal pages in new tabs can be unexpected for users and may cause accessibility/usability concerns (users may not realize they've opened a new tab).
Please confirm this behavior is intentional per HMCTS UX guidelines.
libs/web-core/src/pages/accessibility-statement/index.njk.test.ts (4)
24-26: LGTM!Good addition of test coverage for the new
backToTopfield in English locale.
35-41: LGTM!Test coverage appropriately extended to verify the new feedback section fields including
contact.name,textRelay, andaudioLoops.
54-56: LGTM!Welsh locale test for
backToTopensures bilingual parity.
65-71: LGTM!Welsh locale test mirrors the English feedback field tests, maintaining structural consistency.
libs/web-core/src/assets/js/back-to-top.ts (1)
1-14: LGTM!Clean implementation with two-step lookup pattern. Optional chaining (
backToTopContainer?.querySelector("a")) safely handles the case where the container doesn't exist. The approach is more flexible and accommodates the new DOM structure.libs/web-core/src/pages/accessibility-statement/en.ts (1)
1-104: LGTM - Well-structured accessibility statement content.The expanded data structure with separate URL and display text fields (
serviceUrl/serviceUrlFull,wcagUrl/wcagLinkText, etc.) is a good pattern for maintainability. The content properly supports the template's rendering requirements.libs/web-core/src/pages/accessibility-statement/index.njk (4)
4-7: LGTM!Good use of the new structured intro fields with proper link rendering.
17-19: Good conditional handling for language differences.The conditional check for
abilityNetPrefixgracefully handles the structural difference between English and Welsh locales.
80-87: LGTM - Back-to-top component properly implemented.The structure matches the expected DOM structure in
back-to-top.tsand tests. The SVG icon is appropriately marked withfocusable="false"to prevent keyboard focus on the decorative element.
21-42: I'll help you verify the accessibility concern about heading hierarchy. Let me start by exploring the file structure and understanding the context.
<function_calls>
#!/bin/bashFirst, let's find the accessibility-statement directory and explore its structure
find . -type d -name "accessibility-statement" | head -20
</function_calls>
<function_calls>
#!/bin/bashLet's read the complete template file to see the full heading hierarchy
cat -n libs/web-core/src/pages/accessibility-statement/index.njk
</function_calls>
<function_calls>
#!/bin/bashLet's find the controller and locale files for accessibility-statement
find libs/web-core/src -type f -name "accessibility" | grep -E ".(ts|js|njk)$"
</function_calls>libs/web-core/src/pages/accessibility-statement/cy.ts (1)
1-105: LGTM - Welsh locale properly structured.The Welsh content mirrors the English structure, maintaining bilingual consistency as required by the coding guidelines. The additional
abilityNetPrefixfield ("Mae gan ") is appropriately included to handle Welsh sentence structure differences.
🎭 Playwright E2E Test Results419 tests 419 ✅ 32m 44s ⏱️ Results for commit 17ac774. ♻️ This comment has been updated with latest results. |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
e2e-tests/tests/page-structure.spec.ts (1)
77-104: Tidy up footer link test naming and make full use of test dataThe logic for counting 8 footer links and splitting same‑tab vs new‑tab behavior looks sound, but there are a couple of small cleanups you could make:
sameTablLinkshas a spelling typo and is a bit hard to read; consider renaming tosameTabLinksfor clarity and consistency with camelCase naming, as per coding guidelines.- Each object in
sameTablLinkscarries atextfield that is currently unused. If you want the test to guard against content regressions as well as URLs, you could also assert the link text (e.g.await expect(footerLink).toHaveText(link.text)), or drop thetextproperty to avoid confusion.These are minor, but they make the test intent and data shape clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
e2e-tests/tests/page-structure.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript variables must use camelCase (e.g.,userId,caseDetails,documentId). Booleans should useis/has/canprefix (e.g.,isActive,hasAccess,canEdit).
Classes and Interfaces must use PascalCase (e.g.,UserService,CaseRepository). Do NOT useIprefix for interfaces (useUserRepositorynotIUserRepository).
Constants must use SCREAMING_SNAKE_CASE (e.g.,MAX_FILE_SIZE,DEFAULT_TIMEOUT).
Module ordering: consts outside function scope at top, exported functions next, other functions ordered by usage, interfaces and types at bottom.
TypeScript must use strict mode enabled with noanywithout justification. Use workspace aliases (@hmcts/*) for imports.
Always add.jsextension to relative imports (e.g.,import { foo } from "./bar.js"). This is required for ESM with Node.js "nodenext" module resolution, even when importing TypeScript files.
All input endpoints must validate inputs. Use parameterized database queries with Prisma. Never include sensitive data in logs.
Only export functions that are intended to be used outside the module. Don't export functions solely for testing purposes.
Only add comments when they are meaningful. Explain why something is done, not what is done.
Favor functional style with simple functional approaches. Don't use a class unless you have shared state.
Data should be immutable by default. Use const and avoid mutations to ensure predictable state.
Functions should have no side effects. Avoid modifying external state or relying on mutable data.
Use Express version 5.x with proper async/await error handling in middleware and route handlers.
Files:
e2e-tests/tests/page-structure.spec.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
API endpoints must use plural for collections (
/api/cases,/api/users), singular for specific resources (/api/case/:id), and singular for creation (POST /api/case).
Files:
e2e-tests/tests/page-structure.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarQube Analysis
- GitHub Check: E2E Tests
2 participants
Jira link
https://tools.hmcts.net/jira/browse/VIBE-236
Change description
Testing done
Security Vulnerability Assessment
CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?
Checklist
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.