Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

fix a11y nested span bug in psammead-bulletin #4600

Merged
merged 20 commits into from
Dec 10, 2021
Merged

Conversation

DarioR01
Copy link
Contributor

@DarioR01 DarioR01 commented Nov 18, 2021

Resolves #9561

Fix a11y bug causing talkback needing multiple gesture to read every span inside a parent with role=text (This should only require 1 gesture)

Code changes:

  • added id to span with role=text
  • added span with id to children
  • added aria-labelledby to anchor element

  • (BBC contributors only) This PR follows the repository use guidelines
  • I have assigned myself to this PR and the corresponding issues
  • Automated jest tests added (for new features) or updated (for existing features)
  • This PR requires manual testing

@DarioR01 DarioR01 self-assigned this Nov 18, 2021
@DarioR01 DarioR01 added a11y Accessibility-related task a11y-swarm An a11y swarm (clarify dev or full team in the desc) needs to be carried out before moving to test bug Something isn't working labels Nov 18, 2021
@@ -279,10 +279,12 @@ exports[`Bulletin should render audio correctly 1`] = `
dir="ltr"
>
<a
aria-labelledby="bulletin-https://bbc.co.uk"
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is just an example? Having a url in an id looks rather strange...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I hope there is a better id in Simorgh

@greenc05
Copy link
Contributor

Looks good thanks @DarioR01

>
{headlineText}
</LiveLabel>
) : (
// eslint-disable-next-line jsx-a11y/aria-role
<span role="text">
<span role="text" id={`bulletin-${ctaLink}`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

URLs contain characters that can't be in IDs sadly - see this issue for more info: bbc/simorgh#9696

One option would be to use regex to clean up the URL - something like const id = ctaLink.replace(/\W/g, '') should remove everything except a-z, A-Z and 0-9

Copy link
Contributor

Choose a reason for hiding this comment

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

@DarioR01 I've implemented this in another area https://github.com/bbc/psammead/pull/4602/files#diff-cf9b2acec38beebd665b213ba1986338507662983a46b941657c8bbd6bfaac2dR57-R70, so hopefully it should be straightforward enough here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sorry for the late reply to everyone. It seems like I missed this from last week. I will sanitise the link after the a11y swarm 👍. Thank you @amoore108 for the implementation example 😄

@DarioR01 DarioR01 requested a review from ryanmccombe December 2, 2021 14:47
@DarioR01 DarioR01 merged commit 7971e18 into latest Dec 10, 2021
@DarioR01 DarioR01 deleted the fix-a11y-bulletin branch January 12, 2022 13:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y Accessibility-related task a11y-swarm An a11y swarm (clarify dev or full team in the desc) needs to be carried out before moving to test bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TalkBack incorrectly reading links in nested spans
5 participants