Skip to content

Conversation

@ccastrotrejo
Copy link
Contributor

@ccastrotrejo ccastrotrejo commented Jan 26, 2026

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Adds role="alert" attribute to MessageBar components in the Settings tab validation for error and warning messages. This ensures screen readers immediately announce error/warning messages (like "Trigger condition cannot be empty") when they appear in the DOM.

Problem: Users relying on screen readers were not being notified when validation errors or warnings appeared in the Settings tab, making it difficult for them to understand that an action was required.

Solution: Applied role="alert" to MessageBar components with error or warning intent types. This ARIA role tells assistive technologies to announce the content immediately when it appears, without requiring the user to navigate to it.

Impact of Change

  • Users: Screen reader users will now receive immediate announcements when validation errors or warnings appear in the Settings tab, improving accessibility compliance
  • Developers: No API changes; behavior is unchanged for visual users
  • System: No performance impact; minimal DOM attribute addition

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: Settings tab with screen reader enabled (VoiceOver/NVDA) to verify error announcements

Unit Tests Added

Added comprehensive unit tests for CustomizableMessageBar component in both libs/designer and libs/designer-v2:

  • Verifies role="alert" is applied for error and warning types
  • Verifies role="alert" is NOT applied for info and success types
  • Tests dismiss button rendering and callback functionality
  • Tests basic message rendering

Contributors

@ccastrotrejo

Screenshots/Videos

N/A - Accessibility improvement (audible, not visual)

Screen readers now announce error messages like 'Trigger condition cannot be empty'
when they appear in the Settings tab. Uses role=alert for error/warning intents
to ensure immediate announcement when the message is added to the DOM.
Copilot AI review requested due to automatic review settings January 26, 2026 17:50
@github-actions
Copy link

github-actions bot commented Jan 26, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(designer): Add role=\"alert\" to MessageBar for screen reader announcements
  • Issue: None — title is clear, follows conventional commit style, and accurately describes the change
  • Recommendation: No changes required.

Commit Type

  • Properly selected (fix).
  • Commit type note: Only one commit type is selected (correct).

Risk Level

  • The PR body marks this as Low risk and the repository label risk:low is present. The code diff (small DOM attribute change + tests) aligns with a low-risk accessibility fix.

What & Why

  • Current: Adds role=\"alert\" attribute to MessageBar components in Settings tab validation for error and warning messages and explains why (screen reader announcement improvement).
  • Issue: None — description is concise and explains the accessibility problem and the chosen solution.
  • Recommendation: Keep as-is. (Optionally add a brief note about why role=\"alert\" is preferred vs aria-live for future reference.)

Impact of Change

  • Impact: Clear and accurate. Users — accessibility improved; Developers — no API change; System — negligible impact.
  • Recommendation: None required.
    • Users: Screen reader users will receive immediate announcements for validation errors/warnings.
    • Developers: No surface breaking changes.
    • System: Negligible performance/DOM impact.

Test Plan

  • Assessment: Unit tests were actually added in both libs/designer and libs/designer-v2 and verify the presence/absence of role="alert" for appropriate message types. Manual testing is checked and described. This satisfies the Test Plan requirements for this change.
  • Recommendation: Ensure CI runs these new tests and they pass. If possible, add a short note in the PR description referencing the test files/locations (they are present but calling them out helps reviewers).

Contributors

  • Assessment: Author/contributor is listed (@ccastrotrejo).
  • Recommendation: If PMs/designers/testers contributed, consider adding them to the Contributors section; not required to pass.

⚠️ Screenshots/Videos

  • Assessment: Marked N/A which is appropriate for an accessibility (non-visual) change.
  • Recommendation: No visual artifacts required. Optionally, mention which screen readers/OS you manually verified with (PR already references VoiceOver/NVDA in the Test Plan checkbox — consider filling that checkbox or adding a line describing results).

Summary Table

Section Status Recommendation
Title Title is clear and follows convention.
Commit Type Correct commit type selected.
Risk Level Low risk is appropriate based on changes.
What & Why Good concise explanation.
Impact of Change Accurate and small-scope.
Test Plan Unit tests added and manual testing noted.
Contributors Contributor listed; add others if applicable.
Screenshots/Videos ⚠️ N/A is fine for accessibility change.

Final message: This PR passes the PR-body/title compliance checks. The advised risk level is "low" which matches the submitter's selection and the repo label. The code diff shows a minimal, focused accessibility improvement (adding role="alert" to MessageBar for error/warning) plus unit tests added in both libs — everything required by the template is present.

Please ensure CI completes successfully and the new unit tests pass in the pipeline. Optionally: call out the exact screen readers/OS used for manual verification (VoiceOver/NVDA) in the Test Plan or check the corresponding checkbox to make it explicit. Otherwise, no further changes are required. Thank you for improving accessibility!


Last updated: Mon, 26 Jan 2026 18:52:02 GMT

@ccastrotrejo ccastrotrejo added the risk:low Low risk change with minimal impact label Jan 26, 2026
@ccastrotrejo ccastrotrejo changed the title fix(a11y): add role=alert to MessageBar for screen reader announcements fix(a11y): Add role="alert" to MessageBar for screen reader announcements Jan 26, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves accessibility for screen reader users by adding role="alert" to error and warning MessageBar components in the settings validation UI. This ensures that important error messages (like "Trigger condition cannot be empty") are immediately announced by screen readers when they appear.

Changes:

  • Added role="alert" attribute to MessageBar component for error and warning intents in both designer and designer-v2 libraries
  • Cleaned up localization strings.json by reorganizing a concurrency setting description key

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
libs/designer/src/lib/ui/settings/validation/errorbar.tsx Added role="alert" for error/warning MessageBar intents to enable immediate screen reader announcements
libs/designer-v2/src/lib/ui/settings/validation/errorbar.tsx Same accessibility improvement as designer library for consistency
Localize/lang/strings.json Reorganized localization key for concurrency setting description (RvT4mt → 6ylGHb)

@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

Tests verify:
- role=alert is applied for error and warning types
- role=alert is not applied for info and success types
- dismiss button renders and functions correctly
@ccastrotrejo ccastrotrejo changed the title fix(a11y): Add role="alert" to MessageBar for screen reader announcements fix(designer): Add role="alert" to MessageBar for screen reader announcements Jan 26, 2026
@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

1 similar comment
@github-actions
Copy link

📊 Coverage check completed. See workflow run for details.

@ccastrotrejo ccastrotrejo changed the title fix(designer): Add role="alert" to MessageBar for screen reader announcements fix(designer): Add role="alert" to MessageBar for screen reader announcements Jan 26, 2026
@ccastrotrejo ccastrotrejo merged commit b433ad0 into main Jan 26, 2026
15 checks passed
@ccastrotrejo ccastrotrejo deleted the ccastrotrejo/displayErrorA11y branch January 26, 2026 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants