Skip to content
This repository was archived by the owner on Feb 5, 2025. It is now read-only.

Conversation

@shiki
Copy link
Contributor

@shiki shiki commented Dec 13, 2019

This fixes the issues described in wordpress-mobile/WordPress-iOS#13086.

This is a dependency of wordpress-mobile/WordPress-iOS#13099. This PR needs to be reviewed first.

Changes

The changes mainly involve the LoginSiteAddressViewController and LoginSelfHostedViewController.

Announce Errors

For errors that happen after pressing Next, we sometimes use the errorLabel to display the error. The problem with this is that it is a UI element that just appears unannounced. A VoiceOver user may not know that it's there and would expect that a network request is still happening in the background.

This fixes that by making VoiceOver focus on the error label. Which, in effect, makes VoiceOver speak the error message.

Text Field Labels

Before After

For these view controllers, we rely on the text field's placeholders instead of labels to describe what the text field is. Placeholders are a bit of a concern for accessibility. For this, I was mostly concerned about the VoiceOver user navigating back to the text field again and not knowing what it is.

This changes the text fields so they will have a static label like “Username“ or “Password“.

Show Password Toggle Button

Shown Hidden
image image

The Show Password button didn't have a label. This is a toggle button so it's a bit tricky. Some apps would change the accessibilityLabel when the image changes. WPAndroid uses a static label and makes TalkBack say “selected“ or “not selected“.

For this, I opted to have a static accessibilityLabel but change the accessibilityValue to the current state of the toggle button.

- (void)updateSecureTextEntryForAccessibility
{
self.secureTextEntryToggle.accessibilityLabel = NSLocalizedString(@"Show password", @"Accessibility label for the “Show password“ button in the login page's password field.");
NSString *accessibilityValue;
if (self.isSecureTextEntry) {
accessibilityValue = NSLocalizedString(@"Hidden", "Accessibility value if login page's password field is hiding the password (i.e. with asterisks).");
} else {
accessibilityValue = NSLocalizedString(@"Shown", "Accessibility value if login page's password field is displaying the password.");
}
self.secureTextEntryToggle.accessibilityValue = accessibilityValue;
}

This is the same strategy used in wordpress-mobile/WordPress-iOS#13057 which seems to work well. I believe it's a bad idea to change the accessibilityLabel as that would indicate that there are 2 different buttons.

Others

Testing

Please use the WPiOS branch at wordpress-mobile/WordPress-iOS#13099.

  1. Log out
  2. Turn VoiceOver on
  3. Navigate to Login → Enter Site Address
  4. Confirm that the site address text field has a label
  5. Enter an invalid URL and press Next. An error will be shown in red. Confirm that VoiceOver speaks the error.
  6. Enter a valid self-hosted URL and press Next to navigate to the next page.
  7. Confirm that:
    a. The username and password text fields have labels
    b. The 1Password button has a label
    c. Lost your password is announced as a link
  8. Enter some values in the username and password fields.
  9. Navigate between the username and password fields. Confirm that their labels are still spoken by VoiceOver.
  10. Navigate VoiceOver to the Show Password button
  11. Toggle the button a couple of times. Confirm that VoiceOver would always speak the current status like “Show password, Shown“ or “Show password, Hidden“.

Reviewer Checklist

Submitter Checklist

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.
  • If it's feasible, I have added unit tests.
  • Create a release version after merging

@mindgraffiti Would you have some time to review this? 🙂

@shiki shiki added enhancement New feature or request VoiceOver labels Dec 13, 2019
@shiki shiki self-assigned this Dec 13, 2019
@shiki shiki marked this pull request as ready for review December 13, 2019 21:04
@shiki shiki requested a review from mindgraffiti December 13, 2019 21:04
Copy link
Contributor

@mindgraffiti mindgraffiti left a comment

Choose a reason for hiding this comment

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

Excellent work Jayson! VoiceOver speaks in a much smoother, easier-to-understand way now.

I'm not sure if it's in the scope of this current work but I did notice one thing while testing. If the user navigates to the main WordPress.com login, signs in by email, then chooses the "enter password instead" option, it navigates to the LoginWPcomViewController, where a quick two-finger swipe of the screen reads all the elements correctly except the enter password button. It reads the variable name onePasswordButton as "owe-nep-sword-button button".

If you have time, this method needs the same treatment that the login by site address / username / password screen has.

Also, TIL Authy does not support VoiceOver in the app ☹️

@shiki
Copy link
Contributor Author

shiki commented Dec 16, 2019

Thank you for the review, @mindgraffiti!

Re: WPCom login, it is tracked in a different task. But I appreciate you digging more into it. I haven't done that yet. I linked your research here wordpress-mobile/WordPress-iOS#13089 (comment).

Also, TIL Authy does not support VoiceOver in the app ☹️

😞 I found some of the apps I use are also not good.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request VoiceOver

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update error message when entering an invalid site address

3 participants