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.

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

Component/checkbox bug #979

Merged
merged 24 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0abd060
[CheckboxGroup#864] Add isDisabled parameter to uikit checkbox group
aycil-alican Mar 25, 2024
2e18c12
[CheckboxGroup#864] Fix partially disable issue on swiftui checkbox g…
aycil-alican Mar 25, 2024
dbc7e34
[Checkbox#866] Fix extanding content issue in stack for single checkbox
aycil-alican Mar 25, 2024
b5ed99f
Merge branch 'main' into component/checkbox_bug_864
aycil-alican Mar 26, 2024
72bb08a
[Checkboxr#876] Fix single checkbox accesibility
aycil-alican Apr 4, 2024
65b3387
[Checkboxr#876] Fix checkbox group accesibility
aycil-alican Apr 4, 2024
d4cabc4
[Checkboxr#876] Add accessibilityLabel for checkbox group on demo pro…
aycil-alican Apr 4, 2024
66a5484
[Checkboxr#876] Fix texts
aycil-alican Apr 5, 2024
c29122d
[Checkboxr#876] Fix accessibilty traits as a button
aycil-alican Apr 5, 2024
6c4a774
[Checkbox#876] remove unnecessary command lines
aycil-alican Apr 5, 2024
a5b6062
[Checkbox#876] fix pr comments
aycil-alican Apr 8, 2024
5884319
[Checkbox#876] Fix typo issues
aycil-alican Apr 12, 2024
ef2e4b1
Merge pull request #877 from adevinta/checkbox_bug_876
aycil-alican Apr 16, 2024
2ad8d51
Merge branch 'component/checkbox_bug' into component/checkbox_bug_866
aycil-alican Apr 16, 2024
92cc503
Merge pull request #868 from adevinta/component/checkbox_bug_866
aycil-alican Apr 16, 2024
ea3213e
Merge branch 'component/checkbox_bug' into component/checkbox_bug_864
aycil-alican Apr 16, 2024
f229f23
Merge pull request #867 from adevinta/component/checkbox_bug_864
aycil-alican Apr 16, 2024
61d43fb
Merge branch 'main' into component/checkbox_bug
aycil-alican Apr 16, 2024
ec4ab03
Merge branch 'component/checkbox_bug' of github.com:adevinta/spark-io…
aycil-alican Apr 16, 2024
56a0cca
[Checkbox#876] Fix wrong accessibility value
aycil-alican Apr 29, 2024
7a899a9
Merge branch 'main' into component/checkbox_bug
aycil-alican May 15, 2024
51e85b6
[CheckboxGroup#939] Fix layout update issue after change alignment
aycil-alican May 15, 2024
a526e53
Merge pull request #940 from adevinta/component/checkbox_bug_939
aycil-alican May 17, 2024
6c10471
Merge branch 'main' into component/checkbox_bug
aycil-alican May 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,20 @@ import Foundation

/// The accessibility identifiers for the checkbox.
public enum CheckboxAccessibilityIdentifier {
/// The default accessibility identifier. Can be changed by the consumer
/// The default checkbox accessibility identifier.
public static let checkbox = "spark-check-box"
/// The default accessibility identifier. Can be changed by the consumer
/// The default checkbox group accessibility identifier.
public static let checkboxGroup = "spark-check-box-group"
/// The identifier of checkbox group ui view title
public static let checkboxGroupTitle = "spark-check-box-group-title"
/// The default checkbox group item accessibility identifier.
public static func checkboxGroupItem(_ id: String) -> String {
Self.checkbox + "-\(id)"
}
}

public enum CheckboxAccessibilityValue {
public static let checked = "1"
public static let indeterminate = "0.5"
public static let unchecked = "0"
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ final class CheckboxGroupViewModel: ObservableObject {
@Published var checkedImage: Image
@Published var layout: CheckboxGroupLayout
@Published var spacing: LayoutSpacing
@Published var accessibilityIdentifierPrefix: String
@Published var titleFont: TypographyFontToken
@Published var titleColor: any ColorToken
@Published var intent: CheckboxIntent
Expand All @@ -28,7 +27,6 @@ final class CheckboxGroupViewModel: ObservableObject {
init(
title: String?,
checkedImage: Image,
accessibilityIdentifierPrefix: String,
theme: Theme,
intent: CheckboxIntent = .main,
alignment: CheckboxAlignment = .left,
Expand All @@ -43,7 +41,6 @@ final class CheckboxGroupViewModel: ObservableObject {
self.spacing = theme.layout.spacing
self.titleFont = theme.typography.subhead
self.titleColor = theme.colors.base.onSurface
self.accessibilityIdentifierPrefix = accessibilityIdentifierPrefix
}

func calculateSingleCheckboxWidth(string: String?) -> CGFloat {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ final class CheckboxGroupViewModelTests: XCTestCase {
self.sut = CheckboxGroupViewModel(
title: "Title",
checkedImage: Image(uiImage: self.checkedImage),
accessibilityIdentifierPrefix: "id",
theme: self.theme
)
}
Expand All @@ -45,7 +44,6 @@ final class CheckboxGroupViewModelTests: XCTestCase {
XCTAssertEqual(sut.intent, .main, "Intent does not match")
XCTAssertEqual(sut.titleFont.uiFont, self.theme.typography.subhead.uiFont, "Title font does not match" )
XCTAssertEqual(sut.titleColor.uiColor, self.theme.colors.base.onSurface.uiColor, "Title color does not match" )
XCTAssertEqual(sut.accessibilityIdentifierPrefix, "id", "Accessibility identifier does not match" )
}

func test_singleCheckbox_width() throws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public struct CheckboxGroupView: View {
/// - checkboxAlignment: The checkbox is positioned on the leading or trailing edge of the view.
/// - theme: The Spark-Theme.
/// - accessibilityIdentifierPrefix: All checkbox-views are prefixed by this identifier followed by the `CheckboxGroupItemProtocol`-identifier.
@available(*, deprecated, message: "Please use init without accessibilityIdentifierPrefix. It was given as a static string.")
public init(
title: String? = nil,
checkedImage: Image,
Expand All @@ -47,11 +48,38 @@ public struct CheckboxGroupView: View {
theme: Theme,
intent: CheckboxIntent = .main,
accessibilityIdentifierPrefix: String
) {
self.init(
title: title,
checkedImage: checkedImage,
items: items,
layout: layout,
alignment: alignment,
theme: theme,
intent: intent
)
}

/// Initialize a group of one or multiple checkboxes.
/// - Parameters:
/// - title: An optional group title displayed on top of the checkbox group..
/// - checkedImage: The tick-checkbox image for checked-state.
/// - items: An array containing of multiple `CheckboxGroupItemProtocol`. Each array item is used to render a single checkbox.
/// - layout: The layout of the group can be horizontal or vertical.
/// - checkboxAlignment: The checkbox is positioned on the leading or trailing edge of the view.
/// - theme: The Spark-Theme.
public init(
title: String? = nil,
checkedImage: Image,
items: Binding<[any CheckboxGroupItemProtocol]>,
layout: CheckboxGroupLayout = .vertical,
alignment: CheckboxAlignment,
theme: Theme,
intent: CheckboxIntent = .main
) {
let viewModel = CheckboxGroupViewModel(
title: title,
checkedImage: checkedImage,
accessibilityIdentifierPrefix: accessibilityIdentifierPrefix,
theme: theme,
intent: intent,
alignment: alignment,
Expand Down Expand Up @@ -94,7 +122,8 @@ public struct CheckboxGroupView: View {
.onChange(of: self.itemContents) { newValue in
self.isScrollableHStack = true
}
.accessibilityIdentifier("\(self.viewModel.accessibilityIdentifierPrefix).\(CheckboxAccessibilityIdentifier.checkboxGroup)")
.accessibilityElement(children: .contain)
.accessibilityIdentifier(CheckboxAccessibilityIdentifier.checkboxGroup)
}

private func makeHStackView() -> some View {
Expand Down Expand Up @@ -156,7 +185,6 @@ public struct CheckboxGroupView: View {
}

private func checkBoxView(item: Binding<any CheckboxGroupItemProtocol>) -> some View {
let identifier = "\(self.viewModel.accessibilityIdentifierPrefix).\(item.id.wrappedValue)"
return CheckboxView(
text: item.title.wrappedValue,
checkedImage: self.viewModel.checkedImage,
Expand All @@ -166,7 +194,8 @@ public struct CheckboxGroupView: View {
isEnabled: item.isEnabled.wrappedValue,
selectionState: item.selectionState
)
.accessibilityIdentifier(identifier)
.disabled(!item.isEnabled.wrappedValue)
.accessibilityIdentifier(CheckboxAccessibilityIdentifier.checkboxGroupItem(item.id.wrappedValue))
}
}

Expand Down
19 changes: 15 additions & 4 deletions core/Sources/Components/Checkbox/View/SwiftUI/CheckboxView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,24 @@ public struct CheckboxView: View {
}
)
.buttonStyle(PressedButtonStyle(isPressed: self.$isPressed))
.accessibilityIdentifier(CheckboxAccessibilityIdentifier.checkbox)
.isEnabledChanged { isEnabled in
self.viewModel.isEnabled = isEnabled
}
.fixedSize(horizontal: false, vertical: true)
.accessibilityIdentifier(CheckboxAccessibilityIdentifier.checkbox)
.accessibilityValue(setAccessibilityValue(selectionState: self.viewModel.selectionState))
.accessibilityRemoveTraits(.isSelected)
}

private func setAccessibilityValue(selectionState: CheckboxSelectionState) -> String {
switch selectionState {
case .selected:
return CheckboxAccessibilityValue.checked
case .indeterminate:
return CheckboxAccessibilityValue.indeterminate
case .unselected:
return CheckboxAccessibilityValue.unchecked
}
}

@ViewBuilder
Expand Down Expand Up @@ -148,9 +162,6 @@ public struct CheckboxView: View {
.frame(width: self.checkboxIndeterminateWidth, height: self.checkboxIndeterminateHeight)
}
}
.if(self.selectionState == .selected) {
$0.accessibilityAddTraits(.isSelected)
}
.id(Identifier.checkbox.rawValue)
.matchedGeometryEffect(id: Identifier.checkbox.rawValue, in: self.namespace)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public final class CheckboxGroupUIView: UIControl {
private var subscriptions = Set<AnyCancellable>()
private var items: [any CheckboxGroupItemProtocol]
private var subject = PassthroughSubject<[any CheckboxGroupItemProtocol], Never>()
private var accessibilityIdentifierPrefix: String

@ScaledUIMetric private var spacingLarge: CGFloat
@ScaledUIMetric private var padding: CGFloat = CheckboxControlUIView.Constants.lineWidthPressed
Expand Down Expand Up @@ -124,6 +123,20 @@ public final class CheckboxGroupUIView: UIControl {
self.itemsStackView.arrangedSubviews.compactMap { $0 as? CheckboxUIView }
}

/// A Boolean value indicating whether the component is in the enabled state.
public override var isEnabled: Bool {
didSet{
guard isEnabled != oldValue else { return }
if isEnabled {
self.checkboxes.enumerated().forEach { index, item in
item.isEnabled = self.items.indices.contains(index) ? self.items[index].isEnabled : true
}
} else {
self.checkboxes.forEach { $0.isEnabled = false }
}
}
}

// MARK: - Initialization

/// Not implemented. Please use another init.
Expand All @@ -142,7 +155,8 @@ public final class CheckboxGroupUIView: UIControl {
/// - theme: The Spark-Theme.
/// - intent: Current intent of checkbox group
/// - accessibilityIdentifierPrefix: All checkbox-views are prefixed by this identifier followed by the `CheckboxGroupItemProtocol`-identifier.
public init(
@available(*, deprecated, message: "Please use init without accessibilityIdentifierPrefix. It was given as a static string.")
public convenience init(
title: String? = nil,
checkedImage: UIImage,
items: [any CheckboxGroupItemProtocol],
Expand All @@ -151,6 +165,35 @@ public final class CheckboxGroupUIView: UIControl {
theme: Theme,
intent: CheckboxIntent = .main,
accessibilityIdentifierPrefix: String
) {
self.init(
title: title,
checkedImage: checkedImage,
items: items,
layout: layout,
alignment: alignment,
theme: theme,
intent: intent
)
}

/// Initialize a group of one or multiple checkboxes.
/// - Parameters:
/// - title: An optional group title displayed on top of the checkbox group..
/// - checkedImage: The tick-checkbox image for checked-state.
/// - items: An array containing of multiple `CheckboxGroupItemProtocol`. Each array item is used to render a single checkbox.
/// - layout: The layout of the group can be horizontal or vertical.
/// - checkboxAlignment: The checkbox is positioned on the leading or trailing edge of the view.
/// - theme: The Spark-Theme.
/// - intent: Current intent of checkbox group
public init(
title: String? = nil,
checkedImage: UIImage,
items: [any CheckboxGroupItemProtocol],
layout: CheckboxGroupLayout = .vertical,
alignment: CheckboxAlignment = .left,
theme: Theme,
intent: CheckboxIntent = .main
) {
self.title = title
self.checkedImage = checkedImage
Expand All @@ -160,7 +203,6 @@ public final class CheckboxGroupUIView: UIControl {
self.checkboxAlignment = alignment
self.theme = theme
self.intent = intent
self.accessibilityIdentifierPrefix = accessibilityIdentifierPrefix
self.spacingLarge = theme.layout.spacing.large
self.spacingSmall = theme.layout.spacing.small
super.init(frame: .zero)
Expand All @@ -172,6 +214,7 @@ public final class CheckboxGroupUIView: UIControl {
self.setupView()
self.enableTouch()
self.updateTitle()
self.updateAccessibility()
}

// MARK: - Methods
Expand All @@ -185,6 +228,12 @@ public final class CheckboxGroupUIView: UIControl {
self._padding.update(traitCollection: self.traitCollection)
}

private func updateAccessibility() {
self.accessibilityIdentifier = CheckboxAccessibilityIdentifier.checkboxGroup
self.isAccessibilityElement = false
self.accessibilityContainerType = .semanticGroup
}

private func setupItemsStackView() {
self.updateLayout()

Expand All @@ -207,9 +256,8 @@ public final class CheckboxGroupUIView: UIControl {
selectionState: item.selectionState,
alignment: self.alignment
)
let identifier = "\(self.accessibilityIdentifierPrefix).\(item.id)"
checkbox.accessibilityIdentifier = CheckboxAccessibilityIdentifier.checkboxGroupItem(item.id)

checkbox.accessibilityIdentifier = identifier
checkbox.publisher.sink { [weak self] in
guard
let self,
Expand All @@ -231,8 +279,6 @@ public final class CheckboxGroupUIView: UIControl {

private func setupView() {

self.accessibilityIdentifier = "\(self.accessibilityIdentifierPrefix).\(CheckboxAccessibilityIdentifier.checkboxGroup)"

self.addSubview(self.titleStackView)
self.scrollView.addSubview(self.itemsStackView)
self.addSubview(self.scrollView)
Expand Down Expand Up @@ -311,6 +357,7 @@ extension CheckboxGroupUIView {

private func updateAlignment() {
self.checkboxes.forEach { $0.alignment = self.alignment }
self.layoutIfNeeded()
}

private func updateIntent() {
Expand Down
36 changes: 33 additions & 3 deletions core/Sources/Components/Checkbox/View/UIKit/CheckboxUIView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,6 @@ public final class CheckboxUIView: UIControl {
}

private func commonInit() {
self.accessibilityIdentifier = CheckboxAccessibilityIdentifier.checkbox

self.setupViews()
self.enableTouch()
self.subscribe()
Expand Down Expand Up @@ -340,11 +338,13 @@ public final class CheckboxUIView: UIControl {
self.viewModel.$opacity.subscribe(in: &self.cancellables) { [weak self] opacity in
guard let self else { return }
self.layer.opacity = Float(opacity)
self.setAccessibilityEnable()
}

self.viewModel.$selectionState.subscribe(in: &self.cancellables) { [weak self] selectionState in
guard let self else { return }
self.controlView.selectionState = selectionState
self.setAccessibilityValue(state: selectionState)
}

self.viewModel.$alignment.subscribe(in: &self.cancellables) { [weak self] alignment in
Expand All @@ -358,6 +358,7 @@ public final class CheckboxUIView: UIControl {
self.textLabel.isHidden = labelHidden
self.textLabel.font = self.viewModel.font.uiFont
self.textLabel.attributedText = text.leftValue
self.setAccessibilityLabel(text.leftValue?.string)
}

self.viewModel.$checkedImage.subscribe(in: &self.cancellables) { [weak self] icon in
Expand All @@ -382,7 +383,36 @@ public final class CheckboxUIView: UIControl {
private extension CheckboxUIView {

private func updateAccessibility() {
self.accessibilityLabel = self.textLabel.text
self.accessibilityIdentifier = CheckboxAccessibilityIdentifier.checkbox
self.isAccessibilityElement = true
self.accessibilityTraits.insert(.button)
self.accessibilityTraits.remove(.selected)
self.setAccessibilityLabel(self.textLabel.text)
self.setAccessibilityValue(state: self.selectionState)
self.setAccessibilityEnable()
}

private func setAccessibilityLabel(_ label: String?) {
self.accessibilityLabel = label
}

private func setAccessibilityValue(state: CheckboxSelectionState) {
switch state {
case .selected:
self.accessibilityValue = CheckboxAccessibilityValue.checked
case .indeterminate:
self.accessibilityValue = CheckboxAccessibilityValue.indeterminate
case .unselected:
self.accessibilityValue = CheckboxAccessibilityValue.unchecked
}
}

private func setAccessibilityEnable() {
if self.isEnabled {
self.accessibilityTraits.remove(.notEnabled)
} else {
self.accessibilityTraits.insert(.notEnabled)
}
}

private func updateTheme(colors: CheckboxColors) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ struct CheckboxGroupListView: View {
layout: self.layout == .selected ? .vertical : .horizontal,
alignment: self.alignment,
theme: self.theme,
intent: self.intent,
accessibilityIdentifierPrefix: "checkbox-group"
intent: self.intent
)
.onChange(of: self.groupType) { newValue in
self.items = self.setItems(groupType: newValue)
Expand Down
Loading
Loading