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

Add accessibility identifiers to SingleChoiceList rows and text fields #7273

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

acb-mv
Copy link
Contributor

@acb-mv acb-mv commented Dec 3, 2024

This adds the ability to add accessibilityIdentifiers to tappable rows and custom entry controls in a SingleChoiceList, preparing it for UI tests.


This change is Reviewable

@acb-mv acb-mv added the iOS Issues related to iOS label Dec 3, 2024
@acb-mv acb-mv self-assigned this Dec 3, 2024
Copy link
Contributor

@rablador rablador left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @acb-mv)


ios/MullvadVPN/View controllers/Settings/SwiftUI components/SingleChoiceList.swift line 223 at r1 (raw file):

            customValueInput = ""
        }
        .accessibilityIdentifier(itemAccessibilityIdentifier(item))

Have we actually tested this ?
Since we have our own custom override of UIAccessibilityIdentification which will try to case the value to the AccessibilityIdentifier enum, if that string is not present in the raw representation, it will just crash at runtime.

Or is SwiftUI using a different protocol for accessibility ?

@acb-mv
Copy link
Contributor Author

acb-mv commented Dec 3, 2024

ios/MullvadVPN/View controllers/Settings/SwiftUI components/SingleChoiceList.swift line 223 at r1 (raw file):

Previously, buggmagnet wrote…

Have we actually tested this ?
Since we have our own custom override of UIAccessibilityIdentification which will try to case the value to the AccessibilityIdentifier enum, if that string is not present in the raw representation, it will just crash at runtime.

Or is SwiftUI using a different protocol for accessibility ?

SwiftUI has a modifier which accepts a String as the identifier.
https://developer.apple.com/documentation/swiftui/view/accessibilityidentifier(_:)

Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

@buggmagnet buggmagnet force-pushed the SingleChoiceList-AccessibilityIdentifier branch from ca7ed9b to 2972a55 Compare December 4, 2024 06:34
@buggmagnet buggmagnet merged commit 638d706 into main Dec 4, 2024
10 of 11 checks passed
@buggmagnet buggmagnet deleted the SingleChoiceList-AccessibilityIdentifier branch December 4, 2024 06:36
Copy link

github-actions bot commented Dec 4, 2024

🚨 End to end tests failed. Please check the failed workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants