Skip to content

fix: make voiceover read org detailes#4264

Merged
JamalAlabdullah merged 12 commits into
mainfrom
4114-wcag-brudd-413-statusbeskjeder-aa---skjermleser-voiceover-does-not-read-found-organization-after-search-in-org-lookup-likely-same-in-person-lookup
Jun 23, 2026
Merged

fix: make voiceover read org detailes#4264
JamalAlabdullah merged 12 commits into
mainfrom
4114-wcag-brudd-413-statusbeskjeder-aa---skjermleser-voiceover-does-not-read-found-organization-after-search-in-org-lookup-likely-same-in-person-lookup

Conversation

@JamalAlabdullah

@JamalAlabdullah JamalAlabdullah commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Description

In org lookup and after Hent opplysninger, VoiceOver now reads the org number and related details (name, address, etc.) in one message. It also reads the error message.

Screen.Recording.2026-06-12.at.13.28.37.mov

Related Issue(s)

Verification/QA

  • Manual functionality testing
    • I have tested these changes manually
    • Creator of the original issue (or service owner) has been contacted for manual testing (or will be contacted when released in alpha)
    • No testing done/necessary
  • Automated tests
    • Unit test(s) have been added/updated
    • Cypress E2E test(s) have been added/updated
    • No automatic tests are needed here (no functional changes/additions)
    • I want someone to help me make some tests
  • UU/WCAG (follow these guidelines until we have our own)
    • I have tested with a screen reader/keyboard navigation/automated wcag validator
    • No testing done/necessary (no DOM/visual changes)
    • I want someone to help me perform accessibility testing
  • User documentation @ altinn-studio-docs
    • Has been added/updated
    • No functionality has been changed/added, so no documentation is needed
    • I will do that later/have created an issue
  • Support in Altinn Studio
    • Issue(s) created for support in Studio
    • This change/feature does not require any changes to Altinn Studio
  • Sprint board
    • The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board
    • I don't have permissions to do that, please help me out
  • Labels
    • I have added a kind/* and backport* label to this PR for proper release notes grouping
    • I don't have permissions to add labels, please help me out

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features
    • Enhanced accessibility for organisation lookups with a polite live-region announcement flow, semantic grouping/ARIA labeling, and announcement reset when clearing.
  • Tests
    • Added/expanded React and e2e tests for validation, successful lookup announcements (including sibling text), keyboard submit, error handling, clear behavior, and read-only mode.
  • Chores
    • Improved the Cypress helper to clear number-formatted inputs by typing directly into the targeted field.
  • Refactor
    • Simplified Text rendering by removing an unused label-id pathway.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds ARIA live-region announcements to OrganisationLookupComponent to address VoiceOver silence during organisation lookup. It builds a localized status message from the looked-up org number and nearby Text fields via layout traversal, sets and focuses a hidden role="status" live region after persistence, clears messages on input or clear actions, adjusts ARIA grouping in both Text and OrganisationLookup components, and adds unit and e2e tests covering success, error, and read-only scenarios.

Changes

Screen reader announcements for organisation lookup

Layer / File(s) Summary
Hooks, state, and announcement logic
src/layout/OrganisationLookup/OrganisationLookupComponent.tsx
Adds useRef import, layout traversal imports for live-region and localization support, statusMessage state and statusRef, LIVE_REGION_RESET_DELAY_MS constant, and announceOrgDetails that traverses layout parent/child relationships to collect bound Text values and build a localized announcement string. Updates handleSubmit to call announceOrgDetails after waitForSave(true) on successful lookup.
Form submission, input and clear integration
src/layout/OrganisationLookup/OrganisationLookupComponent.tsx
Clears statusMessage in handleClear and on orgnr input change to avoid stale announcements.
ARIA live region and component grouping
src/layout/OrganisationLookup/OrganisationLookupComponent.tsx, src/layout/Text/TextComponent.tsx
Wraps organisation name in a role="group" with an aria-label and adds a visually-hidden role="status" live region (statusRef, lang, tabIndex={-1}, data-testid) that renders statusMessage. TextComponent removes getLabelId import and id destructuring, simplifying ARIA relationships.
Unit and e2e tests
src/layout/OrganisationLookup/OrganisationLookupComponent.test.tsx, test/e2e/integration/component-library/organisationlookup.ts, test/e2e/support/custom.ts
Adds Jest/RTL tests for rendering, client validation, successful lookup with announcement content (including sibling Text fields), Enter-key submit, server "not found" and invalid responses, request failures, clear behavior, and read-only rendering without action buttons. Updates e2e tests to blur before submit, verify server error flow, and assert validation. Fixes numberFormatClear to target input element directly via cy.wrap.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

backport-ignore

Suggested reviewers

  • lassopicasso
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: make voiceover read org detailes' is directly related to the main change in the PR which implements screen reader announcements for organization lookup results.
Description check ✅ Passed The description includes a summary of changes, references the related issue #4114, and includes verification/QA checklist template, though specific checkboxes are not marked as completed.
Linked Issues check ✅ Passed The PR addresses issue #4114 by implementing screen reader announcements for organization lookup results via live region accessibility features, meeting the WCAG 4.1.3 Status Messages requirement.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the screen reader announcement issue in org lookup; removal of getLabelId from TextComponent appears related to UI refactoring to support the new accessibility pattern.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 4114-wcag-brudd-413-statusbeskjeder-aa---skjermleser-voiceover-does-not-read-found-organization-after-search-in-org-lookup-likely-same-in-person-lookup

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/layout/Text/TextComponent.tsx (1)

35-45: ⚡ Quick win

Reconsider role="group" for single element wrapper.

The div wrapper with role="group" around a single DisplayText component may be semantically inappropriate. The ARIA group role is typically intended for grouping multiple interactive elements or related form controls, not wrapping a single text display element.

Additionally, both the wrapper and the DisplayText component reference the same label ID (getLabelId(id)), which could result in redundant screen reader announcements.

If the group is required for the organisation lookup announcement flow, please consider:

  1. Documenting the specific accessibility requirement it addresses
  2. Verifying that the group doesn't create redundant announcements
  3. Evaluating whether a simpler semantic wrapper (e.g., plain <div> without role) would suffice
♻️ Consider removing the group wrapper if not essential
-    <div
-      role='group'
-      aria-labelledby={getLabelId(id)}
-    >
-      <DisplayText
-        value={value}
-        iconUrl={icon}
-        iconAltText={langAsString(textResourceBindings.title)}
-        labelId={getLabelId(id)}
-      />
-    </div>
+    <DisplayText
+      value={value}
+      iconUrl={icon}
+      iconAltText={langAsString(textResourceBindings.title)}
+      labelId={getLabelId(id)}
+    />
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/layout/Text/TextComponent.tsx` around lines 35 - 45, The wrapper div
currently uses role="group" and shares the same aria-labelledby as DisplayText
(getLabelId(id)), which can cause redundant screen-reader announcements; remove
the role="group" from the wrapper (or remove the wrapper entirely if not needed)
and ensure only the element that should be labelled keeps aria-labelledby —
update either the div or the DisplayText prop so only one element references
getLabelId(id); if the group role is truly required, add a short inline comment
documenting the specific accessibility need and verify no duplicate
announcements occur.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/layout/OrganisationLookup/OrganisationLookupComponent.tsx`:
- Around line 96-128: The announceOrgDetails function is programmatically
focusing the ARIA live region (statusRef.current?.focus()) after updating
statusMessage; remove that focus call so the role="status"/aria-live="polite"
region can announce changes without moving user focus. Keep the
setStatusMessage('') and delayed setStatusMessage(parts.join(', ')) behavior,
but delete the statusRef.current?.focus() invocation. Optionally extract the
numeric 100 ms delay into a named constant (e.g., LIVE_REGION_RESET_DELAY_MS)
used in the window.setTimeout call for clarity.

---

Nitpick comments:
In `@src/layout/Text/TextComponent.tsx`:
- Around line 35-45: The wrapper div currently uses role="group" and shares the
same aria-labelledby as DisplayText (getLabelId(id)), which can cause redundant
screen-reader announcements; remove the role="group" from the wrapper (or remove
the wrapper entirely if not needed) and ensure only the element that should be
labelled keeps aria-labelledby — update either the div or the DisplayText prop
so only one element references getLabelId(id); if the group role is truly
required, add a short inline comment documenting the specific accessibility need
and verify no duplicate announcements occur.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c553b76b-83b9-48b4-802e-8c99379ed223

📥 Commits

Reviewing files that changed from the base of the PR and between 8799263 and 5a05978.

📒 Files selected for processing (2)
  • src/layout/OrganisationLookup/OrganisationLookupComponent.tsx
  • src/layout/Text/TextComponent.tsx

Comment thread src/layout/OrganisationLookup/OrganisationLookupComponent.tsx
@JamalAlabdullah JamalAlabdullah added kind/bug Something isn't working backport This PR should be cherry-picked onto older release branches labels Jun 11, 2026
@JamalAlabdullah JamalAlabdullah moved this to 🔎 In review in Team Altinn Studio Jun 12, 2026
@JamalAlabdullah JamalAlabdullah added the squad/utforming Issues that belongs to the named squad. label Jun 12, 2026
…tatusbeskjeder-aa---skjermleser-voiceover-does-not-read-found-organization-after-search-in-org-lookup-likely-same-in-person-lookup

@Magnusrm Magnusrm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try using aria-atomic= true and role=status in order to get the screenreader to announce the changed content? If that works, it might be a cleaner solution than creating our own announcer.
https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA22

Comment thread src/layout/Text/TextComponent.tsx Outdated
…tatusbeskjeder-aa---skjermleser-voiceover-does-not-read-found-organization-after-search-in-org-lookup-likely-same-in-person-lookup

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/layout/Text/TextComponent.tsx`:
- Line 13: The TextComponent is not passing labelId to the DisplayText
component, which breaks the ARIA label association required for accessibility.
Compute labelId using useIndexedId and getLabelId (following the same pattern
used in other layout components like Likert and Option), and then pass this
labelId prop to the DisplayText component where it renders the value. This
ensures DisplayText can properly set the aria-labelledby attribute to establish
the programmatic relationship between the label element created by
ComponentStructureWrapper and the displayed text content.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a4667b78-9cd9-4e0d-b48b-e17d907f1d35

📥 Commits

Reviewing files that changed from the base of the PR and between eadc2b1 and b3a73da.

📒 Files selected for processing (1)
  • src/layout/Text/TextComponent.tsx

Comment thread src/layout/Text/TextComponent.tsx Outdated
…beskjeder-aa---skjermleser-voiceover-does-not-read-found-organization-after-search-in-org-lookup-likely-same-in-person-lookup
@JamalAlabdullah

JamalAlabdullah commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Did you try using aria-atomic= true and role=status in order to get the screenreader to announce the changed content? If that works, it might be a cleaner solution than creating our own announcer. https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA22

I tried role="status" / live regions first, but VoiceOver on Safari didn’t reliably announce the full message (org nr + following fields) without duplicate/wrong-language reads. The current hidden region + focus + lang was the workaround that passed VoiceOver testing.

@sonarqubecloud

Copy link
Copy Markdown

@Magnusrm Magnusrm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! 🥳

@JamalAlabdullah JamalAlabdullah merged commit 6aeaea7 into main Jun 23, 2026
16 checks passed
@JamalAlabdullah JamalAlabdullah deleted the 4114-wcag-brudd-413-statusbeskjeder-aa---skjermleser-voiceover-does-not-read-found-organization-after-search-in-org-lookup-likely-same-in-person-lookup branch June 23, 2026 14:26
@github-project-automation github-project-automation Bot moved this from 🔎 In review to ✅ Done in Team Altinn Studio Jun 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Automatic backport successful!

A backport PR has been automatically created for the release/v4.32 release branch.

The release branch release/v4.32 already existed and was updated.

The cherry-pick was clean with no conflicts. Please review the backport PR when it appears.

JamalAlabdullah added a commit that referenced this pull request Jun 23, 2026
* fix: make voiceover read org detailes

* fixed coderabbitai feedback

* added test

* fixed cypress test

* voiceover read also error message

* fixed test in organisationlookup.ts

* feedback fixing

* fixed test

* test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR should be cherry-picked onto older release branches kind/bug Something isn't working squad/utforming Issues that belongs to the named squad.

Projects

Status: ✅ Done

2 participants