Fix announcement of soft-validations for screen reader #3925
Conversation
📝 WalkthroughWalkthroughAdds two CSS classes for soft validations, refactors validation rendering to use Alert components with container IDs/data attributes, and updates Input and TextArea components to include composite aria-describedby that references validation containers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Magnusrm
added
backport-ignore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/layout/TextArea/TextAreaComponent.tsx (1)
67-73: Major: Validations not linked when description is absent.Same issue as InputComponent.tsx. The
ariaDescribedByis only set when bothtitleanddescriptionexist, meaning validation messages won't be announced by screen readers when there's no description but validations are present.The validations ID should be included in
ariaDescribedByeven when there's no description, as long as there's a title.
🤖 Fix all issues with AI agents
In @src/features/validation/ComponentValidations.tsx:
- Around line 67-71: The parent div with id template using baseId currently has
aria-live='assertive' while child SoftValidations components render
role='alert', causing nested live regions and double announcements; pick one
strategy and make changes consistently: either remove aria-live='assertive' from
the parent container (the div using baseId and data-validation/indexedId) and
ensure SoftValidations (and any Alert-bearing children) keep role='alert' to own
announcements, or remove role='alert' from SoftValidations (and any child
Alerts) and rely on the parent's aria-live='assertive' so ErrorValidations and
SoftValidations use the same announcement mechanism. Ensure the chosen approach
is applied to both SoftValidations and ErrorValidations rendering paths so
live-region behavior is consistent.
In @src/layout/Input/InputComponent.tsx:
- Around line 136-139: The aria-describedby logic on InputComponent is excluding
validation IDs when description is missing, causing screen readers to miss
validation messages; update the aria-describedby computation (check
overrideDisplay?.renderedInTable, textResourceBindings?.title, and whether
validations exist) so that inputDescribedBy always includes the
`${baseComponentId}-validations` id when validations are present, and only
conditionally append the description id if a description exists (use the
existing inputDescribedBy and textResourceBindings checks to build the combined
describedBy string rather than requiring both title and description).
- Line 131: inputDescribedBy currently concatenates getDescriptionId(id) even
when it returns undefined, producing an invalid string like "undefined
component-validations"; change the construction of inputDescribedBy (the
variable computed using getDescriptionId, id and baseComponentId) to only
include the description id when getDescriptionId(id) returns a truthy value
(e.g., conditionally join parts or filter falsy values and join with a space) so
the resulting aria-describedby contains only real IDs such as "<descriptionId>
<baseComponentId>-validations" or just "<baseComponentId>-validations" when no
description exists.
In @src/layout/TextArea/TextAreaComponent.tsx:
- Line 44: The computed string textAreaDescribedBy in TextAreaComponent.tsx can
include the literal "undefined" when getDescriptionId(id) returns undefined;
change the assignment of textAreaDescribedBy to only include
getDescriptionId(id) when it is defined (e.g., build an array
[getDescriptionId(id), `${baseComponentId}-validations`], filter out falsy
entries, then join with a space) so the ARIA reference never contains the word
"undefined" and still includes `${baseComponentId}-validations`.
🧹 Nitpick comments (1)
src/features/validation/ComponentValidations.module.css (1)
13-17: Consider renaming for clarity.The class name
.softValidationListItemsuggests it's for individual<li>elements, but it's actually applied to the<ul>container (line 135 in ComponentValidations.tsx). Consider renaming to.softValidationListfor better semantic clarity.📝 Suggested naming improvement
-.softValidationListItem { +.softValidationList { padding-left: 0; margin-bottom: 0; list-style-type: none; }Then update the usage in ComponentValidations.tsx line 135 accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/features/validation/ComponentValidations.module.csssrc/features/validation/ComponentValidations.tsxsrc/layout/Input/InputComponent.tsxsrc/layout/TextArea/TextAreaComponent.tsx
🧰 Additional context used
📓 Path-based instructions (2)
{**/*.module.css,**/*.{ts,tsx}}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CSS Modules for component styling and leverage Digdir Design System components when possible
Files:
src/features/validation/ComponentValidations.module.csssrc/features/validation/ComponentValidations.tsxsrc/layout/TextArea/TextAreaComponent.tsxsrc/layout/Input/InputComponent.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid usinganytype or type casting (as type) in TypeScript code; improve typing by avoiding casts andanys when refactoring
Use objects for managing query keys and functions, andqueryOptionsfor sharing TanStack Query patterns across the system for central management
Files:
src/features/validation/ComponentValidations.tsxsrc/layout/TextArea/TextAreaComponent.tsxsrc/layout/Input/InputComponent.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-25T12:53:54.399Z
Learnt from: CR
Repo: Altinn/app-frontend-react PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T12:53:54.399Z
Learning: Applies to {**/*.module.css,**/*.{ts,tsx}} : Use CSS Modules for component styling and leverage Digdir Design System components when possible
Applied to files:
src/features/validation/ComponentValidations.module.css
🧬 Code graph analysis (3)
src/features/validation/ComponentValidations.tsx (2)
src/layout/Alert/index.tsx (1)
Alert(8-14)src/features/language/Lang.tsx (1)
Lang(15-23)
src/layout/TextArea/TextAreaComponent.tsx (1)
src/components/label/Label.tsx (1)
getDescriptionId(135-141)
src/layout/Input/InputComponent.tsx (2)
src/components/label/Label.tsx (1)
getDescriptionId(135-141)src/app-components/Input/Input.tsx (1)
InputProps(15-42)
⏰ 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). (4)
- GitHub Check: Install
- GitHub Check: Type-checks, eslint, unit tests and SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: Install
🔇 Additional comments (3)
src/features/validation/ComponentValidations.module.css (1)
8-11: LGTM!The
.softValidationclass provides appropriate spacing and prevents breaking the validation alert across pages/columns.src/features/validation/ComponentValidations.tsx (2)
3-3: LGTM!Correctly imports Alert from the Digdir Design System, aligning with the coding guidelines to leverage design system components.
72-90: LGTM!The refactored structure with direct conditional rendering of validation groups is cleaner and more maintainable than the previous nested approach.
| <div | ||
| id={`${baseId}-validations`} | ||
| data-validation={indexedId} | ||
| aria-live='assertive' | ||
| style={{ display: 'contents' }} | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and its size
wc -l src/features/validation/ComponentValidations.tsxRepository: Altinn/app-frontend-react
Length of output: 121
🏁 Script executed:
# Read the entire file to understand the context and verify the ARIA configuration
cat -n src/features/validation/ComponentValidations.tsxRepository: Altinn/app-frontend-react
Length of output: 5945
🏁 Script executed:
# Check if there are any other uses of aria-live or role='alert' in similar patterns
rg "aria-live|role=['\"]alert['\"]" src/features/validation/ -A 2 -B 2Repository: Altinn/app-frontend-react
Length of output: 883
Fix inconsistent ARIA live regions to prevent double announcements.
The parent container has aria-live='assertive' (line 70), while child SoftValidations components have role='alert' (line 133), which implicitly includes aria-live='assertive'. This creates nested live regions that may cause screen readers to announce validation messages twice.
Additionally, ErrorValidations relies on the parent's aria-live while SoftValidations has its own role='alert', creating an inconsistent announcement strategy across validation types.
Resolve by choosing one approach:
- Remove
aria-live='assertive'from the parent and ensure all children that need announcements haverole='alert', OR - Remove
role='alert'from individual child Alerts and rely solely on the parent'saria-live='assertive'
🤖 Prompt for AI Agents
In @src/features/validation/ComponentValidations.tsx around lines 67 - 71, The
parent div with id template using baseId currently has aria-live='assertive'
while child SoftValidations components render role='alert', causing nested live
regions and double announcements; pick one strategy and make changes
consistently: either remove aria-live='assertive' from the parent container (the
div using baseId and data-validation/indexedId) and ensure SoftValidations (and
any Alert-bearing children) keep role='alert' to own announcements, or remove
role='alert' from SoftValidations (and any child Alerts) and rely on the
parent's aria-live='assertive' so ErrorValidations and SoftValidations use the
same announcement mechanism. Ensure the chosen approach is applied to both
SoftValidations and ErrorValidations rendering paths so live-region behavior is
consistent.
| <Alert | ||
| className={classes.softValidation} | ||
| data-color={severity} | ||
| role='alert' | ||
| > | ||
| <ul className={classes.softValidationListItem}> | ||
| {validations.map((validation) => ( | ||
| <li key={getUniqueKeyFromObject(validation)}> | ||
| <Lang | ||
| id={validation.message.key} | ||
| params={validation.message.params} | ||
| customTextParameters={validation.message.customTextParameters} | ||
| /> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </Alert> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@digdir/designsystemet-react Alert component ARIA role attributes
💡 Result:
Short summary — ARIA for @digdir/designsystemet-react Alert
- Static alerts (visible on page load): no special ARIA required; treat as normal content. [1][4]
- Dynamic alerts (appear after page load):
- Critical/urgent: use role="alert" on a live region wrapping the Alert (role="alert" ≈ aria-live="assertive"). [1][2][4]
- Non‑critical/status messages: use role="status" or aria-live="polite" (recommend combining role="status" + aria-live="polite" for best compatibility). [1][2]
- Don’t combine role="alert" with aria-live on the same element (may be announced twice in some screen readers). [1][2]
- Implementation notes from Designsystemet:
- Put the live-region attribute on an element that exists at page load and wrap the Alert — don’t rely on adding a populated alert element dynamically. [1]
- For common cases the design system maps semantics: result/waiting → role="status" or aria-live="polite"; errors → role="alert" or aria-live="polite"/"assertive". [1]
- References: Designsystemet Alert accessibility and System notifications pages; MDN ARIA alert role. [1][4][2]
🏁 Script executed:
# Search for Alert component imports in the codebase
rg "from.*designsystemet" --type ts --type tsx -A 2
# Check the specific file for context
head -160 src/features/validation/ComponentValidations.tsx | tail -40Repository: Altinn/app-frontend-react
Length of output: 1102
🏁 Script executed:
# Find Alert component definition or import
rg "export.*Alert" --type ts --type tsx | head -20
# Search for Alert component file in designsystemet (may be node_modules or type definition)
find . -name "*.ts" -o -name "*.tsx" | xargs rg -l "interface Alert|type Alert" | head -5
# Check how severity is used with data-color attribute
rg "data-color.*severity|severity.*data-color" --type ts --type tsx -B 3 -A 3Repository: Altinn/app-frontend-react
Length of output: 232
🏁 Script executed:
# Read the full ComponentValidations.tsx file to see imports
cat -n src/features/validation/ComponentValidations.tsx | head -50
# Search for Alert import from designsystemet
grep -n "import.*Alert" src/features/validation/ComponentValidations.tsx
# Check other Alert usages in the codebase with role attribute
rg "role=['\"]alert['\"]" -A 2 -B 2Repository: Altinn/app-frontend-react
Length of output: 5711
🏁 Script executed:
# Check AlertSeverity definition
cat -n src/layout/Alert/config.generated.ts | grep -A 10 "AlertSeverity"
# Check if there are any other uses of 'warning'/'info'/'success' severity with role attributes
rg "severity.*warning|severity.*info|severity.*success" -B 3 -A 3Repository: Altinn/app-frontend-react
Length of output: 8002
🏁 Script executed:
# Read AlertBaseComponent.tsx to see how it handles role and aria-live
cat -n src/layout/Alert/AlertBaseComponent.tsxRepository: Altinn/app-frontend-react
Length of output: 1405
Remove explicit role='alert' or add proper aria-live with severity-aware handling.
The role='alert' on line 133 is semantically incorrect for non-critical alerts. According to ARIA best practices and the web search results, role="alert" should only be used for critical/urgent messages that require immediate screen reader announcement. Your component handles three severity levels:
'warning': may warrant assertive announcement'info'and'success': non-critical status messages that should use polite announcements
Currently, all three receive role='alert' (aria-live="assertive"), making info and success messages incorrectly assertive. Either:
- Remove the explicit
role='alert'and add severity-awarearia-live(using the same logic asAlertBaseComponent.tsx: warning/danger → assertive, others → polite), or - Use
role='status'witharia-live='polite'for info/success, androle='alert'witharia-live='assertive'only for warnings.
Also note: the web search warns against combining role and aria-live on the same element in some screen readers. Use one or the other, or wrap the Alert in a separate live region container.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/layout/Input/InputComponent.tsx:
- Around line 138-141: The current aria-describedby logic prevents validation
announcements when descriptions are missing and the naive change risks pointing
to a non-existent element; update the logic in InputComponent (the
aria-describedby assignment using overrideDisplay?.renderedInTable,
textResourceBindings?.title/description, and inputDescribedBy) to only include
inputDescribedBy if the validation container will actually be rendered (i.e.,
there are validations for baseComponentId-validations as produced by
ComponentValidations) or alternatively always render the validation container
(ComponentValidations) in the DOM (even when empty) so inputDescribedBy is
always valid; choose one approach and implement the corresponding conditional
check/always-render change so aria-describedby never references a missing
element while still allowing validations to be announced when description is
absent.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/layout/Input/InputComponent.tsxsrc/layout/TextArea/TextAreaComponent.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/layout/TextArea/TextAreaComponent.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Avoid usinganytype or type casting (as type) in TypeScript code; improve typing by avoiding casts andanys when refactoring
Use objects for managing query keys and functions, andqueryOptionsfor sharing TanStack Query patterns across the system for central management
Files:
src/layout/Input/InputComponent.tsx
{**/*.module.css,**/*.{ts,tsx}}
📄 CodeRabbit inference engine (CLAUDE.md)
Use CSS Modules for component styling and leverage Digdir Design System components when possible
Files:
src/layout/Input/InputComponent.tsx
🧬 Code graph analysis (1)
src/layout/Input/InputComponent.tsx (2)
src/components/label/Label.tsx (1)
getDescriptionId(135-141)src/app-components/Input/Input.tsx (1)
InputProps(15-42)
⏰ 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). (3)
- GitHub Check: Type-checks, eslint, unit tests and SonarCloud
- GitHub Check: Install
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
src/layout/Input/InputComponent.tsx (1)
7-7: LGTM! Clean ID construction for accessibility.The import and ID construction logic correctly:
- Derives descriptionId using the helper function
- Creates a consistent validationsId pattern
- Falls back gracefully when description is absent
Also applies to: 131-133
| 'aria-describedby': | ||
| overrideDisplay?.renderedInTable !== true && textResourceBindings?.title && textResourceBindings?.description | ||
| ? getDescriptionId(id) | ||
| ? inputDescribedBy | ||
| : undefined, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/layout/Input/InputComponent.tsx | sed -n '130,150p'Repository: Altinn/app-frontend-react
Length of output: 1228
🏁 Script executed:
# Search for validation container ID pattern
rg -n 'validations' src/layout/Input/InputComponent.tsx | head -20Repository: Altinn/app-frontend-react
Length of output: 230
🏁 Script executed:
rg -n 'ComponentValidations' src/layout/Input/InputComponent.tsx -A 3 -B 3Repository: Altinn/app-frontend-react
Length of output: 51
🏁 Script executed:
cat -n src/layout/Input/InputComponent.tsx | sed -n '125,145p'Repository: Altinn/app-frontend-react
Length of output: 1326
🏁 Script executed:
# Search for where validation content is rendered in Input directory
rg -n 'validations' src/layout/Input/ -A 2 -B 2 | head -50Repository: Altinn/app-frontend-react
Length of output: 1795
🏁 Script executed:
fd -e tsx -e ts src/layout/Input/ | xargs rg -l 'return.*(' | head -5Repository: Altinn/app-frontend-react
Length of output: 511
🏁 Script executed:
rg -n '<div.*validationsId|id=.*validationsId' src/layout/Input/ -A 2 -B 2Repository: Altinn/app-frontend-react
Length of output: 51
🏁 Script executed:
# Get the entire InputComponent.tsx file or at least the render section
wc -l src/layout/Input/InputComponent.tsxRepository: Altinn/app-frontend-react
Length of output: 108
🏁 Script executed:
cat src/layout/Input/InputComponent.tsx | grep -n "return\|ComponentValidations\|validationsId" | head -30Repository: Altinn/app-frontend-react
Length of output: 1065
🏁 Script executed:
rg -n 'id.*validationsId|validationsId.*=' src/layout/Input/InputComponent.tsx -B 2 -A 2Repository: Altinn/app-frontend-react
Length of output: 376
🏁 Script executed:
# Get line count first
wc -l src/layout/Input/InputComponent.tsxRepository: Altinn/app-frontend-react
Length of output: 108
🏁 Script executed:
# View the return statement from line 253 onwards
cat -n src/layout/Input/InputComponent.tsx | sed -n '253,272p'Repository: Altinn/app-frontend-react
Length of output: 759
🏁 Script executed:
rg -n 'ComponentValidations' src/layout/Input/ -A 5Repository: Altinn/app-frontend-react
Length of output: 51
🏁 Script executed:
rg -n 'validationsId' src/layout/ -B 3 -A 3 | head -50Repository: Altinn/app-frontend-react
Length of output: 1399
🏁 Script executed:
# Search for ComponentStructureWrapper to understand how validations are rendered
rg -n 'ComponentStructureWrapper' src/layout/ --type tsx --type ts -A 10 | head -50Repository: Altinn/app-frontend-react
Length of output: 96
🏁 Script executed:
rg -n 'id=.*validationsId' src/layout/ -B 3 -A 3 | head -50Repository: Altinn/app-frontend-react
Length of output: 51
🏁 Script executed:
rg -n 'export.*ComponentStructureWrapper|function ComponentStructureWrapper' src/ --type tsx --type ts -A 15 | head -80Repository: Altinn/app-frontend-react
Length of output: 96
🏁 Script executed:
# Search for ComponentStructureWrapper definition
rg -n 'ComponentStructureWrapper' src/layout/ -A 10 | head -50Repository: Altinn/app-frontend-react
Length of output: 3842
🏁 Script executed:
# Search for where validations div is rendered
rg -n 'id.*=.*validationsId' src/ -B 2 -A 2 | head -50Repository: Altinn/app-frontend-react
Length of output: 51
🏁 Script executed:
# Find the ComponentStructureWrapper file
fd 'ComponentStructureWrapper' src/Repository: Altinn/app-frontend-react
Length of output: 109
🏁 Script executed:
# View the ComponentStructureWrapper file
cat -n src/layout/ComponentStructureWrapper.tsxRepository: Altinn/app-frontend-react
Length of output: 2089
🏁 Script executed:
# Find and view AllComponentValidations
rg -n 'export.*AllComponentValidations|function AllComponentValidations' src/ -A 20 | head -60Repository: Altinn/app-frontend-react
Length of output: 2020
🏁 Script executed:
# Search for the validationsId div pattern
fd 'ComponentValidations' src/ -type fRepository: Altinn/app-frontend-react
Length of output: 239
🏁 Script executed:
# Find ComponentValidations file
find src -name '*ComponentValidations*'Repository: Altinn/app-frontend-react
Length of output: 173
🏁 Script executed:
# View the full ComponentValidations.tsx file
cat -n src/features/validation/ComponentValidations.tsx | head -100Repository: Altinn/app-frontend-react
Length of output: 4395
🏁 Script executed:
# Confirm the validation container behavior - check if it can be rendered empty
cat -n src/features/validation/ComponentValidations.tsx | sed -n '36,70p'Repository: Altinn/app-frontend-react
Length of output: 1775
Critical accessibility issue: Validations not announced when description is missing, but proposed fix would create invalid aria-describedby references.
The condition requiring both textResourceBindings?.title and textResourceBindings?.description to set aria-describedby prevents validations from being announced via screen readers when a description is absent. However, the validation container (with ID ${baseComponentId}-validations) is only rendered when validations exist (ComponentValidations.tsx, lines 62-64 returns null if no validations).
The proposed fix of unconditionally using aria-describedby: overrideDisplay?.renderedInTable !== true ? inputDescribedBy : undefined would reference a non-existent DOM element when no validations are present, creating broken aria-describedby attributes.
A proper fix requires checking whether validations will be rendered before setting aria-describedby, or restructuring the validation container to always render in the DOM (even if empty) to make the simplified condition safe.
🤖 Prompt for AI Agents
In @src/layout/Input/InputComponent.tsx around lines 138 - 141, The current
aria-describedby logic prevents validation announcements when descriptions are
missing and the naive change risks pointing to a non-existent element; update
the logic in InputComponent (the aria-describedby assignment using
overrideDisplay?.renderedInTable, textResourceBindings?.title/description, and
inputDescribedBy) to only include inputDescribedBy if the validation container
will actually be rendered (i.e., there are validations for
baseComponentId-validations as produced by ComponentValidations) or
alternatively always render the validation container (ComponentValidations) in
the DOM (even when empty) so inputDescribedBy is always valid; choose one
approach and implement the corresponding conditional check/always-render change
so aria-describedby never references a missing element while still allowing
validations to be announced when description is absent.
|
1 participant
Description
This PR adds
"role="alert"to the Alert component used for the soft validations in order to make the screen reader announce the contents when it renders.I added an id to the validations and reference it in both input and textarea components
"aria-describedby"because the debounced save when typing seemed to interrupt the validation message. Adding this seems to give the alert the priority.I also moved som inline styling into css modules.
Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
Accessibility Improvements
UI Changes
✏️ Tip: You can customize this high-level summary in your review settings.