Skip to content

fix(ui-alerts): add variantScreenReaderLabel prop to Alert to improve screenreader usability#1962

Merged
balzss merged 1 commit into
masterfrom
fix/alert-message-screenreader
May 28, 2025
Merged

fix(ui-alerts): add variantScreenReaderLabel prop to Alert to improve screenreader usability#1962
balzss merged 1 commit into
masterfrom
fix/alert-message-screenreader

Conversation

@balzss

@balzss balzss commented May 7, 2025

Copy link
Copy Markdown
Contributor

INSTUI-4533

test plan:

  • open the alert component's page in the docs and go through with screenreader -> it should announce the type of the alert

@balzss balzss self-assigned this May 7, 2025
@balzss balzss requested review from ToMESSKa and matyasf May 7, 2025 11:59
@github-actions

github-actions Bot commented May 7, 2025

Copy link
Copy Markdown
PR Preview Action v1.6.1
Preview removed because the pull request was closed.
2025-05-28 15:39 UTC

Comment thread packages/ui-alerts/src/Alert/props.ts Outdated

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.

@balzss JAWS unfortunately reads the ':' as 'colon', but I checked that ',' works as a pause with VoiceOver and JAWS too, so I would recommend that instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@ToMESSKa ToMESSKa May 27, 2025

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.

@balzss The text still says "Note the : at", so it might be good to change that too.

@ToMESSKa ToMESSKa 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.

@balzss only the first example has the new prop, maybe it would be a good idea to add it to the rest of the examples too

@matyasf matyasf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

variantScreenReaderLabel does nothing when added to the liveregion example in VoiceOver, as I see the text is not added to the liveRegion.

Also please add some text about this new prop, e.g. to the info boxes

@balzss balzss force-pushed the fix/alert-message-screenreader branch from 8317832 to 1ff0b9b Compare May 27, 2025 10:29
@balzss balzss requested review from ToMESSKa and matyasf May 27, 2025 10:29
@balzss balzss force-pushed the fix/alert-message-screenreader branch from 1ff0b9b to 3df0be3 Compare May 27, 2025 10:44

@matyasf matyasf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good to me, just please update the docs to use ',' as Tamas mentioned

@ToMESSKa ToMESSKa 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.

see my comment in props.ts

@balzss balzss force-pushed the fix/alert-message-screenreader branch from 3df0be3 to 8d36fef Compare May 28, 2025 15:01
@balzss balzss requested a review from ToMESSKa May 28, 2025 15:02
@balzss balzss merged commit 814a0ea into master May 28, 2025
12 checks passed
@balzss balzss deleted the fix/alert-message-screenreader branch May 28, 2025 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants