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

Radio Schedules - Fix VoiceOver bug with the NEXT card headline #3348

Merged
merged 2 commits into from
Apr 8, 2020

Conversation

DenisHdz
Copy link
Contributor

@DenisHdz DenisHdz commented Apr 8, 2020

Resolves #3347

Overall change:
In this PR we removed the span that was wrapping the Promo Card Headline in order to simplify the markup. Unfortunately, this is necessary to avoid VoiceOver splitting the content within it.

Code changes:

  • Add HeadingContentWrapper back with the role='text'

  • 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

@DenisHdz DenisHdz added ws-home Tasks for the WS Home Team radio-schedules labels Apr 8, 2020
@DenisHdz DenisHdz self-assigned this Apr 8, 2020
Copy link
Contributor

@Bopchy Bopchy left a comment

Choose a reason for hiding this comment

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

👍

@@ -226,34 +226,39 @@ exports[`ProgramCard should render correctly for live 1`] = `
href="/news/articles/cn7k01xp8kxo"
>
<span
class=""
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks great, just wondering if it's fine to have empty classes in snapshots?

Copy link
Contributor Author

@DenisHdz DenisHdz Apr 8, 2020

Choose a reason for hiding this comment

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

I believe that since we don't add styles when declaring that styled component:
const HeadingContentWrapper = styled.span.attrs({ role: 'text' })``;
it leaves the class empty as it doesn't have to target it to apply styles.

It also happens for the DurationTextWrapper.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also wondered why that was there...

@greenc05
Copy link
Contributor

greenc05 commented Apr 8, 2020

Looks good, thanks @DenisHdz

Copy link
Contributor

@OlgaLyubin OlgaLyubin left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this 👍

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

Successfully merging this pull request may close these issues.

Radio Schedules - VoiceOver reading out wrongly the NEXT card headline
5 participants