[Hold for payment 2022-01-27] Add aria-role and aria-label to Terms of Service and Privacy Policy

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Navigate to settings
2. Click on about
3. Using the navigation keys, try to click on Terms of Service or Privacy Policy

Expected Result:

User should be able to reach the terms of service and privacy policy by using the navigation keys

Actual Result:

Terms of Service & Privacy Policy are non-accessible (non-tabbable, can’t be read & opened by screen reader).

Workaround:

Unknown.

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.0.93-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Expensify-Bug.mov

Expensify/Expensify Issue URL:

View all open jobs on GitHub

---

From Mateo Morris (Couldn't find the user in GH) https://expensify.slack.com/archives/C01GTK53T8Q/p1630607495347700

Terms of Service & Privacy Policy are non-accessible (non-tabbable, can’t be read & opened by screen reader). Just needs an aria-role & aria-label.

[MelvinBot]

Triggered auto assignment to @johnmlee101 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[PrashantMangukiya]

Proposed Solution:

We have to set accessible and accessibilityLabel for terms of service and privacy policy link within src/pages/settings/AboutPage.js

At present accessible and accessibilityLabel not added as shown code below:

App/src/pages/settings/AboutPage.js

Lines 130 to 133 in 6cd518e

	<Text
	style={[styles.chatItemMessageHeaderTimestamp, styles.link]}
	onPress={() => Linking.openURL(CONST.TERMS_URL)}
	>

App/src/pages/settings/AboutPage.js

Lines 143 to 146 in 6cd518e

	<Text
	style={[styles.chatItemMessageHeaderTimestamp, styles.link]}
	onPress={() => Linking.openURL(CONST.PRIVACY_URL)}
	>

Once code updated for both it looks as under:

  <Text
      style={[styles.chatItemMessageHeaderTimestamp, styles.link]}
      onPress={() => Linking.openURL(CONST.TERMS_URL)}
      accessible
      accessibilityLabel={translate('initialSettingsPage.readTheTermsAndPrivacyPolicy.phrase2')}
  >

  <Text
      style={[styles.chatItemMessageHeaderTimestamp, styles.link]}
      onPress={() => Linking.openURL(CONST.PRIVACY_URL)}
      accessible
      accessibilityLabel={translate('initialSettingsPage.readTheTermsAndPrivacyPolicy.phrase4')}
  >

Here is the screen recording after code updated as per my suggestion:

Screen.Recording.2021-09-04.at.2.48.46.PM.mov

At present External label not set with this issue, so just giving suggestion. It might be helpful for someone if handled internally.

If this issue consider as External and my solution accepted then kindly invite me on Upwork project. Here is my Upwork profile: https://www.upwork.com/freelancers/~01c718889ed7821f82

Thank you.

[elemdos]

Proposal

After taking another look, it turns out there's an existing component we can use instead which includes the aria-role. And since the link text is descriptive, aria-label is redundant: "The aria-label attribute provides a way to place a descriptive text label on an object, such as a link, when there are no elements visible on the page that describe the object" w3.org

So these lines here

App/src/pages/settings/AboutPage.js

Lines 130 to 150 in 6cd518e

	<Text
	style={[styles.chatItemMessageHeaderTimestamp, styles.link]}
	onPress={() => Linking.openURL(CONST.TERMS_URL)}
	>
	{translate(
	'initialSettingsPage.readTheTermsAndPrivacyPolicy.phrase2',
	)}
	</Text>
	{' '}
	{translate(
	'initialSettingsPage.readTheTermsAndPrivacyPolicy.phrase3',
	)}
	{' '}
	<Text
	style={[styles.chatItemMessageHeaderTimestamp, styles.link]}
	onPress={() => Linking.openURL(CONST.PRIVACY_URL)}
	>
	{translate(
	'initialSettingsPage.readTheTermsAndPrivacyPolicy.phrase4',
	)}
	</Text>

should be replaced with

<TextLink
    style={[styles.chatItemMessageHeaderTimestamp, styles.link]}
    href={CONST.TERMS_URL}
>
    {translate(
        'initialSettingsPage.readTheTermsAndPrivacyPolicy.phrase2',
    )}
</TextLink>
{' '}
{translate(
    'initialSettingsPage.readTheTermsAndPrivacyPolicy.phrase3',
)}
{' '}
<TextLink
    style={[styles.chatItemMessageHeaderTimestamp, styles.link]}
    href={CONST.PRIVACY_URL}
>
    {translate(
        'initialSettingsPage.readTheTermsAndPrivacyPolicy.phrase4',
    )}
</TextLink>

tabbing.mov

[PrashantMangukiya]

Hi @MateoMorris , your solution also looks good. One thing I notice is that whenever we use TextLink component as per your suggestion it shows pipe at left and right of text. If we add accessibilityLabel to Text component as per my solution, it shows box around the text, i.e. similar to shows box around close and back button on that page when tabbing. i.e. Both approach have same result but some visual difference, not sure it will make any difference from usage/ui point of view or not. Thanks.

[elemdos]

@PrashantMangukiya I noticed that, but in this case it's still better to use TextLink because it renders to an anchor tag as opposed to a span (i.e. it's accessible to both keyboard and screenreader users). A compromise would be to add accessibilityRole="link", but accessibility-wise it's always preferable to use native web elements whenever possible.

[MelvinBot]

@johnmlee101 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[johnmlee101]

Hey all, love the proactiveness here. Going to mark as external and review the two proposals.

[MelvinBot]

Triggered auto assignment to @JmillsExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.