♿️ - Settings > Notifications - Manage project notifications + Email frequency #552

dusi · 2019-01-11T00:09:49Z

📲 What

Improves the accessibility of the following rows

Settings > Notifications > Manage project notifications
Settings > Notifications > Email frequency

🤔 Why

Making a11y great again

🛠 How

Using accessibilityTraits and accessibilityLabel + accessibilityValue

✅ Acceptance criteria

Test under the following conditions
👉 iOS 10, iOS 11 & iOS 12
👉Voice Over ON
👉 In order to test localization prefer to QA with one of the supported language set as the primary 📱 language

Settings > Notifications > Manage project notifications row is read by VoiceOver as Manage project notification, X, button where X is the number of projects backed so (i.e. Manage project notifications, twelve, button (make sure this is localized)
Settings > Notifications > Email frequency row is read by VoiceOver as Email frequency, Y, button where X is the selected option of frequency so (i.e. Email frequency, Individual emails, button) (make sure to login as a creator to see this option)

dusi · 2019-01-11T00:11:57Z

Kickstarter-iOS/Views/Cells/SettingsNotificationCell.swift

    self.emailNotificationsButton.rac.selected = viewModel.outputs.emailNotificationsEnabled
    self.emailNotificationsButton.rac.hidden = viewModel.outputs.emailNotificationButtonIsHidden
    self.projectCountLabel.rac.text = viewModel.outputs.projectCountText
    self.pushNotificationsButton.rac.selected = viewModel.outputs.pushNotificationsEnabled
    self.pushNotificationsButton.rac.hidden = viewModel.outputs.pushNotificationButtonIsHidden
-    self.projectCountLabel.rac.accessibilityHint = viewModel.outputs


I wasn't able to make VoiceOver read the hint (maybe this needs more testing on an actual device with different OS version). @Scollaco could you please run AppStore version on iOS 10/11 and see if this hint is being spoken?

If not we should also be able to remove profile_project_count_projects_backed from the Strings file

I was able to make the VoiceOver read the hint once I changed self.projectCountLabel.rac.accessibilityHint to self.rac.accessibilityHint, because than the hint would be coupled to the cell and not to the label.

Interesting.

Not really sure why it won't use hint on a UILabel. But anyways in order to follow the native Settings.app patterns and the a11y audit, I think we should remove this.

@ifbarrera since you've added this localized hint, do you feel strongly about it?

@dusi nope don't feel strongly about it, you can go ahead and remove it for consistency with the rest of Settings accessibility work!

Scollaco

Thanks for doing this, @dusi . I just left a few comments.

Scollaco · 2019-01-15T20:23:27Z

Kickstarter-iOS/Views/Cells/SettingsNotificationCell.swift

@@ -90,13 +94,12 @@ final class SettingsNotificationCell: UITableViewCell, NibLoading, ValueCell {
  override func bindViewModel() {
    super.bindViewModel()

+    self.rac.accessibilityValue = viewModel.outputs.projectCountText


After testing out the code, I noticed that tapping any cell on Settings > Notifications - let's say Project Updates -will make the VoiceOver say Project Updates, X, where X is the project count. I may be happening because the accessibilityValue is being set to viewModel.outputs.projectCountText in every cell here. I think that this can be solved but returning projectCountText only if the cellType = .projectNotifications

Good eye! Thanks for catching this!

Scollaco · 2019-01-15T20:47:22Z

Kickstarter-iOS/Views/Cells/SettingsNotificationCell.swift

    self.emailNotificationsButton.rac.selected = viewModel.outputs.emailNotificationsEnabled
    self.emailNotificationsButton.rac.hidden = viewModel.outputs.emailNotificationButtonIsHidden
    self.projectCountLabel.rac.text = viewModel.outputs.projectCountText
    self.pushNotificationsButton.rac.selected = viewModel.outputs.pushNotificationsEnabled
    self.pushNotificationsButton.rac.hidden = viewModel.outputs.pushNotificationButtonIsHidden
-    self.projectCountLabel.rac.accessibilityHint = viewModel.outputs


I was able to make the VoiceOver read the hint once I changed self.projectCountLabel.rac.accessibilityHint to self.rac.accessibilityHint, because than the hint would be coupled to the cell and not to the label.

Scollaco · 2019-01-15T20:50:30Z

Library/ViewModels/SettingsNotificationCellViewModel.swift

@@ -14,7 +14,6 @@ public protocol SettingsNotificationCellViewModelOutputs {
  var emailNotificationsEnabled: Signal<Bool, NoError> { get }
  var emailNotificationButtonIsHidden: Signal<Bool, NoError> { get }
  var pushNotificationButtonIsHidden: Signal<Bool, NoError> { get }
-  var manageProjectNotificationsButtonAccessibilityHint: Signal<String, NoError> { get }


We would need to re-add this signal after making the suggested change above.

dusi · 2019-01-17T05:18:16Z

♿️ Settings > Notifications - Manage project notifications

Scollaco

Thanks for addressing the feedback, @dusi . It's looking good now! 👍

Removing the approval due to a rebase

Scollaco

Still working after rebasing. I also tested in Spanish and can confirm that VoiceOver is localized.

…frequency (#552) * Add button traits to project notifications & email frequency rows * Remove unused accessibility hint * Remove accessibility value

* Accessibility - Settings - Dismiss button (#532) * Add accessibility label to dismiss button & fix the minimum width * Set navigation item properties using key paths * Accessibility - Settings - Rows (#534) * Add button traits to settings cells * Invert disclosure indicators in Settings > Help * Add accessibility hint in case email is unverified (#535) * Accessibility - Settings - Change email - Current email (#537) * Group current email title and value for a better accessibility * Simplify the way we set accessibility label on current email container * ♿️ - Settings - Message Banner (#539) * Accessibility - Settings - Dismiss button (#532) * Add accessibility label to dismiss button & fix the minimum width * Set navigation item properties using key paths * Accessibility - Settings - Rows (#534) * Add button traits to settings cells * Invert disclosure indicators in Settings > Help * Add accessibility hint in case email is unverified (#535) * Focuses Voice Over on the banner when presented * Delay banner dismissal a bit more when VoiceOver is ON * Localize banner message * Expose delete my kickstarter account button instead of the label (#549) * ♿️- Settings - Notifications - Heading (#550) * Accessibility - Settings - Dismiss button (#532) * Add accessibility label to dismiss button & fix the minimum width * Set navigation item properties using key paths * Accessibility - Settings - Rows (#534) * Add button traits to settings cells * Invert disclosure indicators in Settings > Help * Accessibility - Settings - Dismiss button (#532) * Add accessibility label to dismiss button & fix the minimum width * Set navigation item properties using key paths * Accessibility - Settings - Rows (#534) * Add button traits to settings cells * Invert disclosure indicators in Settings > Help * Add accessibility hint in case email is unverified (#535) * Accessibility - Settings - Change email - Current email (#537) * Group current email title and value for a better accessibility * Simplify the way we set accessibility label on current email container * a11y for notification settings * swiftlint fix * ♿️- Settings - Change email / Change password input fields (#545) * textfield a11y improvement in change email and password screens * change stackview alignment * new snapshots * fixed strings * ♿️ - Settings - Switch Toggle Rows (#544) * Fix newsletter switch cell accessibility * Fix project notifications switch cell accessibility * Fix settings privacy switch cell accessibility * Refocus VoiceOver on the cell when the confirmation dialog disappears * Place UIAccessibility notification behind a VoiceOver running check * Prevent refreshing the following cell when confirming the alert dialogue * Alphabetize properties * Localize properly * ♿️ - Settings - Notifications - Daily Digest picker bug (#547) * corrected picker view hidden in iphone X in VM * Library tests * Update snapshot tests * vm and snapshot tests * snapshot tests * wip - digest picker is selected * properly aligned tableview constraint * removed output to hide picker * emailPickerViewHeight * VoiceOver re-focuses on the Email frequency cell * adjusted animation added guard * changed comment * Update Kickstarter-iOS/Views/Controllers/SettingsNotificationsViewController.swift Co-Authored-By: cdolm92 <cdolm92@gmail.com> * Apply suggestions from code review comments adjustment Co-Authored-By: cdolm92 <cdolm92@gmail.com> * Update Kickstarter-iOS/Views/Controllers/SettingsNotificationsViewController.swift Co-Authored-By: cdolm92 <cdolm92@gmail.com> * ♿️ - Settings > Notifications - Manage project notifications + Email frequency (#552) * Add button traits to project notifications & email frequency rows * Remove unused accessibility hint * Remove accessibility value * Infer UIView.AnimationOptions type * Combine conditions * Use more functional return Co-Authored-By: dusi <pavel.dusatko@gmail.com> * Fix Github's messed up suggestion * Update accessibility label to better match with localized server errors * Add localized string * Use localized string * Align filter * Remove unused call to setNeedsLayout

dusi assigned Scollaco Jan 11, 2019

dusi requested review from Scollaco and cdolm92 January 11, 2019 00:10

dusi commented Jan 11, 2019

View reviewed changes

dusi changed the title ~~A11y settings manage project notifications~~ ♿️ - Settings > Notifications - Manage project notifications + Email frequency Jan 11, 2019

dusi added the needs review label Jan 11, 2019

Scollaco reviewed Jan 15, 2019

View reviewed changes

Scollaco previously approved these changes Jan 17, 2019

View reviewed changes

Scollaco added ready to merge and removed needs review labels Jan 17, 2019

dusi force-pushed the a11y-settings branch from 90ec7fb to ade427d Compare January 17, 2019 22:28

dusi added 3 commits January 17, 2019 15:05

Add button traits to project notifications & email frequency rows

e9fd65d

Remove unused accessibility hint

582e854

Remove accessibility value

19d6f13

dusi force-pushed the a11y-settings-manage-project-notifications branch from a550499 to 19d6f13 Compare January 17, 2019 23:06

dusi removed the ready to merge label Jan 17, 2019

Scollaco approved these changes Jan 18, 2019

View reviewed changes

dusi merged commit d69c736 into a11y-settings Jan 18, 2019

dusi deleted the a11y-settings-manage-project-notifications branch January 18, 2019 18:03

dusi mentioned this pull request Jan 29, 2019

♿️ - Settings #546

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

♿️ - Settings > Notifications - Manage project notifications + Email frequency #552

♿️ - Settings > Notifications - Manage project notifications + Email frequency #552

dusi commented Jan 11, 2019 •

edited by Scollaco

Loading

dusi Jan 11, 2019

Scollaco Jan 15, 2019

dusi Jan 16, 2019

ifbarrera Jan 16, 2019

Scollaco left a comment

Scollaco Jan 15, 2019

dusi Jan 15, 2019

Scollaco Jan 15, 2019

Scollaco Jan 15, 2019

dusi commented Jan 17, 2019

Scollaco left a comment •

edited

Loading

Scollaco left a comment

♿️ - Settings > Notifications - Manage project notifications + Email frequency #552

♿️ - Settings > Notifications - Manage project notifications + Email frequency #552

Conversation

dusi commented Jan 11, 2019 • edited by Scollaco Loading

📲 What

🤔 Why

🛠 How

✅ Acceptance criteria

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scollaco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dusi commented Jan 17, 2019

Scollaco left a comment • edited Loading

Choose a reason for hiding this comment

Scollaco left a comment

Choose a reason for hiding this comment

dusi commented Jan 11, 2019 •

edited by Scollaco

Loading

Scollaco left a comment •

edited

Loading