Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Accessibility - Settings - Rows #534

Merged
merged 2 commits into from
Dec 21, 2018
Merged

Accessibility - Settings - Rows #534

merged 2 commits into from
Dec 21, 2018

Conversation

dusi
Copy link
Contributor

@dusi dusi commented Dec 20, 2018 •
edited by ifbarrera
Loading

📲 What

Flags common settings cells as a button for Voice Over.

🤔 Why

Better indicates that there's an action linked to these cells for visually impaired users.

🛠 How

Simply setting accessibilityTraits = .button.

✅ Acceptance criteria

on all the following: iOS 10, iOS 11 & iOS 12
with Voice Over turned ON

  • All cells in Settings (except App version) now have button trait (i.e. Account - button, Log out - button, etc.)
  • All cells in Settings > Account now have button trait
  • All cells in Settings > Help now have button trait
  • All cells in Settings > Help now show disclosure indicator (except Contact which is presented modally) - based on discussion with @dannyalright

@dusi dusi changed the title A11y settings row Accessibility - Settings - Rows Dec 20, 2018
dusi
dusi commented Dec 20, 2018
View reviewed changes
Kickstarter-iOS/Views/Cells/SettingsAccountWarningCell.swift
@@ -9,8 +9,11 @@ final class SettingsAccountWarningCell: UITableViewCell, ValueCell, NibLoading {
@IBOutlet fileprivate weak var warningIconImage: UIImageView!

func configureWith(value shouldHideAlertIcon: Bool) {
_ = self
|> \.accessibilityTraits .~ .button
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we only use this cell in one place this should be fine. Refactoring the signature would result in a lot more changes that don't seem necessary unless we start using this cell more.

dusi
dusi commented Dec 20, 2018
View reviewed changes
Library/SettingsAccountCellType.swift
@@ -34,6 +34,10 @@ public enum SettingsAccountCellType: SettingsCellTypeProtocol, CaseIterable {
case currency
case currencyPicker
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Returning button for all rows should also be fine with currencyPicker being removed soon.

@dusi dusi requested a review from ifbarrera December 20, 2018 20:07
ifbarrera
ifbarrera approved these changes Dec 21, 2018
View reviewed changes
Copy link
Contributor

@ifbarrera ifbarrera left a comment

Choose a reason for hiding this comment

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

Tested on iPhone X, iOS 12.1.

All rows behaved as expected.

@dusi dusi merged commit 0e526f9 into a11y-settings Dec 21, 2018
@dusi dusi deleted the a11y-settings-row branch December 21, 2018 19:43
@dusi
Copy link
Contributor Author

dusi commented Dec 27, 2018

Audit Settings Accessibility

dusi added a commit that referenced this pull request Jan 3, 2019
@dusi
Accessibility - Settings - Rows (#534)
6514013 
* Add button traits to settings cells

* Invert disclosure indicators in Settings > Help
dusi added a commit that referenced this pull request Jan 7, 2019
@dusi
Accessibility - Settings - Rows (#534)
5a8551a 
* Add button traits to settings cells

* Invert disclosure indicators in Settings > Help
@dusi dusi mentioned this pull request Jan 7, 2019
Merged
dusi added a commit that referenced this pull request Jan 8, 2019
@dusi
♿️ - Settings - Message Banner (#539)
00772d7 
* 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
dusi added a commit that referenced this pull request Jan 8, 2019
@dusi
Accessibility - Settings - Rows (#534)
933172a 
* Add button traits to settings cells

* Invert disclosure indicators in Settings > Help
dusi added a commit that referenced this pull request Jan 8, 2019
@dusi
♿️ - Settings - Message Banner (#539)
d6721f4 
* 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
cdolm92 added a commit that referenced this pull request Jan 9, 2019
@cdolm92
♿️- Settings - Notifications - Heading (#550)
f9d2a34 
* 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
dusi added a commit that referenced this pull request Jan 17, 2019
@dusi
Accessibility - Settings - Rows (#534)
f29bbb4 
* Add button traits to settings cells

* Invert disclosure indicators in Settings > Help
dusi added a commit that referenced this pull request Jan 17, 2019
@dusi
♿️ - Settings - Message Banner (#539)
f485276 
* 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
dusi pushed a commit that referenced this pull request Jan 17, 2019
@cdolm92 @dusi
♿️- Settings - Notifications - Heading (#550)
0349d3a 
* 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
dusi added a commit that referenced this pull request Jan 29, 2019
@dusi
Accessibility - Settings - Rows (#534)
9217fd3 
* Add button traits to settings cells

* Invert disclosure indicators in Settings > Help
dusi added a commit that referenced this pull request Jan 29, 2019
@dusi
♿️ - Settings - Message Banner (#539)
e3b2ce7 
* 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
dusi pushed a commit that referenced this pull request Jan 29, 2019
@cdolm92 @dusi
♿️- Settings - Notifications - Heading (#550)
b6836d9 
* 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
dusi added a commit that referenced this pull request Feb 6, 2019
@dusi
♿️ - Settings (#546)
d2cf6f9 
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ifbarrera ifbarrera ifbarrera approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dusi @ifbarrera