-
Notifications
You must be signed in to change notification settings - Fork 54
Radio Schedules - Fix VoiceOver bug with the NEXT card headline #3348
Conversation
There was a problem hiding this 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="" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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...
Looks good, thanks @DenisHdz |
There was a problem hiding this 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 👍
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:
HeadingContentWrapper
back with the role='text'