Skip to content

Apply empty accessibility traits to AttributedLabel if supplied traits is nil#590

Merged
meherkasam-square merged 1 commit intosquare:mainfrom
meherkasam-square:meher/attributed-label
Sep 26, 2025
Merged

Apply empty accessibility traits to AttributedLabel if supplied traits is nil#590
meherkasam-square merged 1 commit intosquare:mainfrom
meherkasam-square:meher/attributed-label

Conversation

@meherkasam-square
Copy link
Contributor

No description provided.

@meherkasam-square meherkasam-square self-assigned this Sep 26, 2025
@meherkasam-square meherkasam-square changed the title Apply staticText accessibility trait to AttributedLabel if supplied traits is nil Apply empty accessibility traits to AttributedLabel if supplied traits is nil Sep 26, 2025
@meherkasam-square meherkasam-square marked this pull request as ready for review September 26, 2025 03:42
@meherkasam-square meherkasam-square requested a review from a team as a code owner September 26, 2025 03:42
Copy link
Collaborator

@kyleve kyleve left a comment

Choose a reason for hiding this comment

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

Makes sense – just confirm, this fixes an issue where we'd move from non-empty to empty traits, but not apply those empty traits?

@meherkasam-square
Copy link
Contributor Author

Makes sense – just confirm, this fixes an issue where we'd move from non-empty to empty traits, but not apply those empty traits?

Indeed, that's exactly what was happening before this fix.

@meherkasam-square meherkasam-square merged commit 9cba469 into square:main Sep 26, 2025
4 checks passed

private func updateAccessibilityTraits(with model: AttributedLabel) {
guard let traits = model.accessibilityTraits else {
accessibilityTraits = []
Copy link
Member

Choose a reason for hiding this comment

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

Do you think there'd be any benefit by only setting them if they aren't already empty? Not sure if there's any overhead when poking this property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There shouldn't be any additional overhead in setting this property. This function is being called only when we're setting a number of other accessibility properties so it gets bundled into a larger set of changes.

johnnewman-square added a commit that referenced this pull request Nov 4, 2025
…Label (#597)

This reverts #590. During
integration testing, the original change caused KIF failures while
searching for labels with the `staticText` trait. The work to audit the
failures and make necessary code changes has been ticketed
[here](https://block.atlassian.net/browse/UI-9567).

This revert allows the changes in
#596 to be fully tested.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants