-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
IOS-10448 [Mistica] RowList A11y update #399
Conversation
β¦trait to header
Added full cell accessbility config struct First approach for improving cells accessibility
Update ListCellContent accessibility (via delegate) when headlineView is updated Fixed some typos Deleted no longer used FullCellAccessibilityConfig and created accessibility type Created separate files for list cell accessibility data
Added comments to help understanding this new feature
private lazy var accessibilityTypeCell: UISegmentedControlTableViewCell = { | ||
let cell = UISegmentedControlTableViewCell(reuseIdentifier: "accessibilityTypeCell") | ||
cell.segmentedControl.insertSegment(withTitle: "Default", at: 0, animated: false) | ||
cell.segmentedControl.insertSegment(withTitle: "Informative", at: 1, animated: false) | ||
cell.segmentedControl.insertSegment(withTitle: "Custom informative", at: 2, animated: false) | ||
cell.segmentedControl.insertSegment(withTitle: "Interactive", at: 3, animated: false) | ||
cell.segmentedControl.insertSegment(withTitle: "Double interaction", at: 4, animated: false) | ||
cell.segmentedControl.selectedSegmentIndex = 0 | ||
return cell | ||
}() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New cell to configure accessibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only see a small issue and that is that the literals can not be read as a whole.
Perhaps it would be interesting to rotate the segmented control vertically XD
https://stackoverflow.com/questions/3490358/can-i-show-an-uisegmentedcontrol-object-in-vertical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried rotation with no success xD. It seems also not possible to modify segmented control to allow multiple lines. Maybe the solution is to create a new cell type for "many" options using buttons
let accessibilityInteractiveData = AccessibilityListCellInteractiveData { [weak self] in | ||
let alertController = UIAlertController(title: nil, message: "Custom action", preferredStyle: .alert) | ||
let alertAction = UIAlertAction(title: "Accept", style: .cancel) | ||
alertController.addAction(alertAction) | ||
|
||
self?.present(alertController, animated: true) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Show an alert just to simulate a custom action
var headlineView: UIView? { | ||
var accessibilityActivationAction: (() -> Void)? | ||
|
||
var headlineView: AccessibleTextualView? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to it's possible to use any UIView, used a protocol that exposes an accessible text
didSet { | ||
oldValue?.removeFromSuperview() | ||
|
||
if let view = headlineView { | ||
insertArrangedSubview(view, at: 0) | ||
updateSpacing() | ||
} | ||
|
||
listCellContentViewDelegate?.accessibilityChanged() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notify delegate that accessibility has changed
override public func accessibilityActivate() -> Bool { | ||
guard let accessibilityActivationAction else { | ||
return false | ||
} | ||
|
||
accessibilityActivationAction() | ||
return true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Execute custom accessibility action on double tap if it's defined. This case only occurs in cells with accessibility of type doubleInteraction
when double tap the center section (for cells with interactive
accesibility type, this action is managed in the parent cell view)
if case .doubleInteraction(let accessibilityInteractiveData) = accessibilityType { | ||
// If double interaction accessibility, make centerSection accessible to be focusable (isAccessibilityElement = true) | ||
centerSection.isAccessibilityElement = true | ||
// Set center section label to the provided one (or the default one if not provided) | ||
centerSection.accessibilityLabel = accessibilityInteractiveData.label ?? defaultAccessibilityLabel | ||
// Set accessibility activation action to be executed on center section double tap | ||
centerSection.accessibilityActivationAction = accessibilityInteractiveData.action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case of double interaction accessibility, center section has to be accessible to be focusable by VoiceOver. The label associated will be the provided one or if no label provided, the default one (defined in Figma specs) and the optional accessibility action is stored
let accessibilityComponents: [String?] = [ | ||
titleText, | ||
headlineText, | ||
subtitleText, | ||
detailText | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Composed default accessibility label following Figma specs
// Set accessibility order following Figma spec: | ||
// https://www.figma.com/design/Be8QB9onmHunKCCAkIBAVr/%F0%9F%94%B8-Lists-Specs?node-id=0-1&node-type=CANVAS&t=jgG9X5qKokaMwJjm-0 | ||
accessibilityElements = [centerSection.titleLabel, headlineView as Any, centerSection.subtitleLabel, centerSection.detailLabel, controlView as Any].compactMap { $0 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If accessibility is informative, define elements order according Figma specs
accessibilityElements = [centerSection.titleLabel, headlineView as Any, centerSection.subtitleLabel, centerSection.detailLabel, controlView as Any].compactMap { $0 } | ||
case .doubleInteraction: | ||
// If double interaction, just two elements: center section and right section | ||
accessibilityElements = [centerSection, controlView as Any].compactMap { $0 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case of double interaction accessibility, there will be two elements: center section (which will manage it's own accessibility) and the control view
extension TagView: AccessibleTextualView { | ||
public var accessibleText: String? { | ||
return text | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most common heading view is a TagView, so implemented AccessibleTextualView protocol by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data associated to the cell accessibility type. It includes the label (can be null, if so, the default one will be used) and the associated action (that can also be nil)
self.action = action | ||
} | ||
|
||
public static var `default`: AccessibilityListCellInteractiveData = .init(action: nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default data has no label (it will use the default one) and no action
} | ||
} | ||
|
||
lazy var titleLabel = IntrinsictHeightLabel() | ||
lazy var subtitleLabel = IntrinsictHeightLabel() | ||
lazy var detailLabel = IntrinsictHeightLabel() | ||
|
||
var listCellContentViewDelegate: ListCellContentViewDelegate? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weak?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private lazy var accessibilityTypeCell: UISegmentedControlTableViewCell = { | ||
let cell = UISegmentedControlTableViewCell(reuseIdentifier: "accessibilityTypeCell") | ||
cell.segmentedControl.insertSegment(withTitle: "Default", at: 0, animated: false) | ||
cell.segmentedControl.insertSegment(withTitle: "Informative", at: 1, animated: false) | ||
cell.segmentedControl.insertSegment(withTitle: "Custom informative", at: 2, animated: false) | ||
cell.segmentedControl.insertSegment(withTitle: "Interactive", at: 3, animated: false) | ||
cell.segmentedControl.insertSegment(withTitle: "Double interaction", at: 4, animated: false) | ||
cell.segmentedControl.selectedSegmentIndex = 0 | ||
return cell | ||
}() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only see a small issue and that is that the literals can not be read as a whole.
Perhaps it would be interesting to rotate the segmented control vertically XD
https://stackoverflow.com/questions/3490358/can-i-show-an-uisegmentedcontrol-object-in-vertical
Sources/MisticaCommon/Utils/Accessibility/ListCells/AccessibilityListCellInteractiveData.swift
Outdated
Show resolved
Hide resolved
Present interactive cell alert in tabbarcontroller to avoid warning
π This PR is included in version 33.1.0 π The release is available on GitHub release Your semantic-release bot π¦π |
ποΈ Jira ticket
IOS-10448 [Mistica] RowList A11y update
π₯ What's the goal?
Update Mistica to satisfy A11y for row lists according to Figma specs. Several issues has been detected. These are some of the main ones:
1.- All the cells were behaving the same way (due to the "default" UITableViewCell's accessibility). All the main elements (inside center view) were being read in natural order. As specified in Figma specs the order is custom (
title
must precedeheadline
)2.- Mistica is not using native accessory views so they were being focused as any other element (e.g: Navigation arrow or switches)
3.- In custom action cells like cells with a switch (e.g: Cells to enable/disable PIN code to access the app) it's not possible to simulate native behavior that focus the whole cell and toggles the switch when user double taps.
π§ How do we do it?
Due to Mistica cells are so configurable that admit generic views in some of their components, it's not possible to deduce the appropiate accessibility type. The proposed solution is letting the client the responsability of determine the cell accessibility type.
For that purpose, a new enum
AccessibilityListCellType
has been created to set accessibility type on cells. It admits four possibile values:interactive
: The default value. The cell is focused as a whole. Typically used in navigation cells. It admits two configurations:label
: If provided, it will be the label read when the cell is focused. If not provided, the default label will be read (with the order specified in Figma specs)action
: If provided, the custom action to be executed when double tap (Usefull for cells contaning a switch to be able to toggle the switch when double tapping the cell)double interaction
: Covered this case specified in spect but shouldn't be used (not a good UX). The cell has two focusable elements: center view (main cell data) and control view (right view). Should be used if the cell has an interactive element in the right side. It admits the same config asinteractive
cell.informative
: Each cell element is focusable individually.customInformative
: This case could be assumed by theinteractive
one but to clearly separate this case from an interactive cell (has en action associated like navigation) it has it's own case. The cell is focused as a whole and the label read is the (mandatory) provided oneWith all these possibilities there are some combinations that will have no sense, for example, adding an interactive element in the right side of the cell and set the accessibility type as
customInteractive
, which will hide the action to the user using accessibility. So it's responsability of the client to use the appropiate accessibility typeπ§ͺ How can I verify this?
RPReplay_Final1725957831.MP4
π AppCenter build
https://install.appcenter.ms/orgs/tuenti-organization/apps/mistica-ios/distribution_groups/public/releases/264