department-of-veterans-affairs / va-mobile-library

https://department-of-veterans-affairs.github.io/va-mobile-library/
ISC License
0 stars 0 forks source link

DS - Discovery - Inline Links Accessibility #186

Open kellylein opened 4 months ago

kellylein commented 4 months ago

Description

Split from ticket #168 to tend to specific accessibility concerns around the Inline variant of the Link component.

As a Veteran, I want to see links within text that are formatted in a way that easy to read, so that I am not distracted by how the content is presented and can focus on the meaning of the content.

As part of this story we want to:

Look into the best way for our Link Component to support inline links in a way that is formatted well. See these Slack threads for more context:

Videos Pulled from Slack thread:

  1. Demoing the visual side behavior w/ wrapping looking good
  2. How the screen reader behaves (note: this is with accessibility label override on one link that is being completely ignored)
    Videos 1 & 2 (iOS)

https://github.com/department-of-veterans-affairs/va-mobile-library/assets/8690976/caaaa2b3-f543-4029-bcdc-055c21a559d4

https://github.com/department-of-veterans-affairs/va-mobile-library/assets/8690976/2336c5af-e2f4-49cd-9507-a93d5aa29b58

Same 2 videos with proposed behavior when the screen reader is on:

Videos 3 & 4 (iOS) https://github.com/department-of-veterans-affairs/va-mobile-library/assets/8690976/373f9cf9-7caf-4416-b89b-265cb21ad32e https://github.com/department-of-veterans-affairs/va-mobile-library/assets/8690976/459cee5b-3b3f-4bcf-a545-a1f868571ec2

Android which behaves rather differently and may be mostly fine:

Android video https://github.com/department-of-veterans-affairs/va-mobile-library/assets/8690976/8a832187-73b5-44d1-8eec-7d161dfeed84

Acceptance Criteria

- [ ] Discuss options with Brea from an accessibility perspective, notably around: - How does a screen reader behave in web around hyperlinks within a paragraph - Is the Android behavior good? - Thoughts on iOS proposed workaround (going to poorly wrapped form only when screen reader is on) - [ ] General point: look into whether a11yLabel override for non-Link text is working as expected (not checked on #168) - [ ] General point: validate a11yHints work within inline Links (not checked on #168) - [ ] If general Android behavior is satisfactory: - [ ] Fix whatever around the icon area is making the screen reader read "0" aloud - [ ] Fix whatever is causing the a11yLabel to not be read on the first link in the Inline story - [ ] If not: implement preferred Android behavior - [ ] If proposed iOS workaround is satisfactory: - [ ] Proceed with [previous inline PR suggestion](https://github.com/department-of-veterans-affairs/va-mobile-library/pull/188#discussion_r1501114063) to clean this up by building the map first and only conditionalizing the `inlineSingle` setting to reduce duplcations - Test around with it--when initially split it was crashing so it was unable to be implemented, but [the problem was identified and fixed](https://github.com/department-of-veterans-affairs/va-mobile-library/pull/188/commits/a6944a8fb8d168338dea9fd81c481dd6b22b9f46) right before putting #168 to PR so the iOS workaround may be good as is - [ ] If not: implement preferred iOS behavior - [ ] Once both are working as desired, test in flagship app - Due to outstanding accessibility concerns, the Inline variant was not tested in app, but should be as it is significantly different than every other Link variety - Testing should include both accessibility pieces with this ticket as well as general non-screen reader behavior (e.g. wrapping generally and at different font sizes) - Specific spots to check: - [ ] See lines 373-390 in `src/screens/BenefitsScreen/ClaimsScreen/AppealDetailsScreen/AppealStatus/AppealCurrentStatus/AppealCurrentStatus.tsx` of the flagship code - [ ] See `getLinkifiedText` function in secure messaging; [Dylan said more about it in Slack](https://adhoc.slack.com/archives/C05DQPHUVNZ/p1707924255729829?thread_ts=1707923272.834929&cid=C05DQPHUVNZ) - [ ] Check if bug #203 manifests in iOS in app or if it's specific to Expo/Storybook **Accessibility Requirements** - [ ] See normal ACs--whole ticket about accessibility ## Notes & Open Questions - Dev Notes: - Based on behavior observed, the aria-label (and possibly a11yHint) properties do not appear to be working for the RN Text version while they do work for RN Pressable version
brea11y commented 4 months ago

Hey guys!

TimRoe commented 4 months ago

With the decision to remove inline Link support, this commit can be viewed for seeing the relevant code to be partially reimplemented/modified when this ticket is worked as the basis for a Paragraph component.

One additional technical note: unify link type='inline' and inlineSingle props to only having a single prop that converts the Link component to being built as an RN Text instead of RN Pressable for when leveraged intending to be part of a paragraph block of text.