Skip to content

Conversation

m1aw
Copy link
Contributor

@m1aw m1aw commented Mar 25, 2025

Summary

The objective of this PR is to make the SF errors for Expiry date to be read. This wasn't happen as they are not triggered onBlur but onChange.

There's 3 main changes:

  • We no longer filter only onBlur events.
    • The errors that are detected via onChange also need to be triggered.
  • aria-relevant has been changed from all to additions
    • This was causing the error to be read when the field got fixed (error removed from the panel)
  • We now remove the SRPanel when there's no error messages.
    • This fixes the panel not reading the error a second time after it happens. As in write wrong expiry date, delete, write wrong expiry date.

Tested scenarios

  • Expiry date error is read once is typed
    • Tested on MacOS VoiceOver Chrome
    • Test on other screen readers
  • Expiry date error is read a second time it occurs
    • Tested on MacOS VoiceOver Chrome
    • Test on other screen readers
  • Expiry date error is NOT read when removed
    • Tested on MacOS VoiceOver Chrome
    • Test on other screen readers

Test regressions:

  • Triggered each of the non onBlur errors above. Check if they get read out twice.
  • Test screen reader on QRCode to check there's no regression

Fixed issue:

Copy link

changeset-bot bot commented Mar 25, 2025

⚠️ No Changeset found

Latest commit: 7e842b9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

Copy link
Contributor

size-limit report 📦

Path Size
UMD 102.35 KB (-0.02% 🔽)
Auto 109.41 KB (+0.01% 🔺)
ESM - Core 24.7 KB (-0.16% 🔽)
ESM - Core + Card 61.67 KB (-0.17% 🔽)
ESM - Core + Dropin with Card 66.13 KB (-0.13% 🔽)

@sponglord
Copy link
Contributor

We now remove the SRPanel when there's no error messages

What exactly do you mean? Where is this "removal" happening?

@m1aw
Copy link
Contributor Author

m1aw commented Mar 26, 2025

@sponglord the removal is happening in SRMessages.ts because we have return messages ?.

@sponglord
Copy link
Contributor

@sponglord the removal is happening in SRMessages.ts because we have return messages ?.

But that was always there, wasn't it? Your description makes it sound like this is new functionality.

For context: the reason I'm talking about this at all is because your description says

We now remove the SRPanel when there's no error messages.

id: 'ariaLiveSRPanel',
ariaAttributes: {
'aria-relevant': 'all',
'aria-relevant': 'additions',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you confirm that all the errors still get read out when the (Card) PM is completely empty and the "Pay" button is pressed?
(I thought that was why I'd used "all" instead of "additions" originally - but it was a long time ago...!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have just tested the Pay button scenario. It does not change with aria-relevant.
However It's not always reading from aria-live. I will double check exactly if is a new behaviour.

@m1aw
Copy link
Contributor Author

m1aw commented Mar 27, 2025

But that was always there, wasn't it? Your description makes it sound like this is new functionality.

I guess I didn't make it clear. It was always there but it was only an inner div of the div where aria-live was. Now I moved the aria-live attribute to be after the message condition.

So when message is empty the aria-live gets gets removed.

@sponglord
Copy link
Contributor

But that was always there, wasn't it? Your description makes it sound like this is new functionality.

I guess I didn't make it clear. It was always there but it was only an inner div of the div where aria-live was. Now I moved the aria-live attribute to be after the message condition.

So when message is empty the aria-live gets gets removed.

Does that not break the a11y requirement, for such an "error" panel, that there is a permanent element in the DOM with an aria-live attribute?

@m1aw
Copy link
Contributor Author

m1aw commented Mar 28, 2025

Does that not break the a11y requirement, for such an "error" panel, that there is a permanent element in the DOM with an aria-live attribute?

It was never the case that it needed to be permanently in the DOM. Was it?

@sponglord
Copy link
Contributor

sponglord commented Mar 28, 2025

It was never the case that it needed to be permanently in the DOM. Was it?

It definitely has to be in the DOM at startup, so the screenreader can find and identify it as an area to watch.
That was clear from levelAccess

[element] with aria-live=”polite[or whatever]" should pre-exist in the DOM, in the markup, at page load

(from a meeting recording we had access to)

Whether after that it has to be permanently present I don't know - but I'm presuming so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants