Fix issue where text entry do not have "text field" or a hint in the description #186

DavidBrunow · 2024-01-17T14:16:52Z

Fixes #57. Takes over from #58.

Sources/AccessibilitySnapshot/Core/Swift/Classes/UIAccessibility+SnapshotAdditions.swift

DavidBrunow · 2024-01-19T14:09:59Z

Example/SnapshotTests/__Snapshots__/SnapshotTestingTests/testUIKitTextEntry.393x852-16-4-3x.png

@NickEntin FYI this is what I'm getting from VoiceOver on iOS 17.3.

DavidBrunow · 2024-01-19T14:14:24Z

...le/SnapshotTests/__Snapshots__/SnapshotTestingTests/testSwiftUITextEntry.393x852-16-4-3x.png

The bounding box for the TextEditor looks off -- on device the element is directly below the last text field and on the leading edge of the screen. If possible I'd like to look into that issue separately from the other issue being resolved in this effort.

NickEntin

This is looking great, thanks for working on this!

...le/SnapshotTests/__Snapshots__/SnapshotTestingTests/testSwiftUITextEntry.393x852-16-4-3x.png

Example/AccessibilitySnapshot/SwiftUITextEntry.swift

Example/AccessibilitySnapshot/TextInputsViewController.swift

Sources/AccessibilitySnapshot/Core/Swift/Classes/UIAccessibility+SnapshotAdditions.swift

Example/SnapshotTests/__Snapshots__/SnapshotTestingTests/testUIKitTextView.393x852-16-4-3x.png

DavidBrunow · 2024-02-07T13:45:23Z

@NickEntin do you need anything from me on this? I want to make sure I'm not blocking anything 😄

NickEntin · 2024-02-09T08:37:25Z

@DavidBrunow Apologies, been a hectic couple weeks. I'll go ahead and update the snapshot images for you.

NickEntin · 2024-02-09T08:45:36Z

Ahh looks like the snapshots are flaky cause of the cursor flashing. I think we need to figure out #15 before we land this one. 😕

NickEntin · 2024-02-09T11:15:34Z

I think we need to figure out #15 before we land this one.

#206 should resolve this.

DavidBrunow · 2024-02-09T13:40:35Z

@DavidBrunow Apologies, been a hectic couple weeks. I'll go ahead and update the snapshot images for you.

Not a problem and I'm not trying to add pressure, just wanted to make sure there wasn't more I could do 😄

NickEntin · 2024-02-17T23:24:56Z

I think if you rebase this on main the builds should pass now 🤞

…ir description

Co-authored-by: Nick Entin <[email protected]>

…be updated from CI)

DavidBrunow · 2024-02-18T03:13:23Z

I think if you rebase this on main the builds should pass now 🤞

This is done.

DavidBrunow · 2024-02-18T12:45:40Z

I think if you rebase this on main the builds should pass now 🤞

This is done.

~~I did not recapture the snapshots 🤦. I'll do that now, hopefully my machine will capture them right.~~

Actually I'm going to wait on this since you've mentioned needing to capture them on build machines in the past. Let me know if I need to take any action on this.

NickEntin · 2024-02-18T23:56:07Z

Ahh yeah, let me pull the images from the CI jobs and update it now. I don't think the iOS 16 job should have failed, so maybe a different problem... 🤔

NickEntin · 2024-02-19T00:56:49Z

...le/SnapshotTests/__Snapshots__/SnapshotTestingTests/testSwiftUITextEntry.390x844-14-5-3x.png

So I guess this sort of worked in iOS 14, then regressed in later versions. 🤷‍♂️

DavidBrunow · 2024-02-19T01:42:01Z

Ahh yeah, let me pull the images from the CI jobs and update it now. I don't think the iOS 16 job should have failed, so maybe a different problem... 🤔

All checks have passed ✅ 🎉

NickEntin · 2024-02-20T07:51:35Z

Example/AccessibilitySnapshot/TextViewViewController.swift

+            }
+            applyVerticalSubviewDistribution(distributionSpecifiers)
+
+            textViewWithText.accessibilityFrame = textViewWithText.frame


Was this necessary to fix an issue? This should be the default value.

In any case, accessibilityFrame is defined in the screen coordinate space while the frame is defined in the superview's coordinate space. In a full screen presentation this should generally match, but isn't always the case. We should do the coordinate space conversion here (if we need to keep it):

Suggested change

textViewWithText.accessibilityFrame = textViewWithText.frame

textViewWithText.accessibilityFrame = UIAccessibility.convertToScreenCoordinates(

textViewWithText.frame,

in: self

)

I copied this over from the previous implementation in #58. I don't see why it would be needed. I'll remove it and see if there are negative side effects.

I've removed that line.

NickEntin · 2024-02-22T00:50:24Z

Thanks David, this looks great! I'll do one final pass to make sure we're good to merge.

One last thing: have you signed the contributor license agreement?

DavidBrunow · 2024-02-22T01:03:07Z

Thanks David, this looks great! I'll do one final pass to make sure we're good to merge.

One last thing: have you signed the contributor license agreement?

I have not, working on that now.

DavidBrunow · 2024-02-22T01:39:57Z

Thanks David, this looks great! I'll do one final pass to make sure we're good to merge.

One last thing: have you signed the contributor license agreement?

FYI I will not get this done tonight, checking with folks at work to ensure there are no issues.

DavidBrunow · 2024-02-22T15:23:18Z

Thanks David, this looks great! I'll do one final pass to make sure we're good to merge.
One last thing: have you signed the contributor license agreement?

FYI I will not get this done tonight, checking with folks at work to ensure there are no issues.

@NickEntin I have signed the contributor license agreement.

NickEntin

Awesome, thanks for working through this @DavidBrunow! Really glad to see this fixed.

DavidBrunow force-pushed the bugfix/swiftUITextFieldMissingDescription branch from 5eb3b40 to c3eb5d0 Compare January 17, 2024 14:17

DavidBrunow commented Jan 17, 2024

View reviewed changes

Sources/AccessibilitySnapshot/Core/Swift/Classes/UIAccessibility+SnapshotAdditions.swift Outdated Show resolved Hide resolved

DavidBrunow commented Jan 19, 2024

View reviewed changes

DavidBrunow force-pushed the bugfix/swiftUITextFieldMissingDescription branch from e751385 to 391cdd0 Compare January 19, 2024 14:12

DavidBrunow commented Jan 19, 2024

View reviewed changes

DavidBrunow force-pushed the bugfix/swiftUITextFieldMissingDescription branch from 391cdd0 to 8485578 Compare January 19, 2024 14:22

DavidBrunow marked this pull request as ready for review January 19, 2024 14:23

NickEntin reviewed Jan 26, 2024

View reviewed changes

DavidBrunow force-pushed the bugfix/swiftUITextFieldMissingDescription branch from 661f3a0 to 34dc851 Compare January 26, 2024 13:36

DavidBrunow mentioned this pull request Jan 26, 2024

SwiftUI TextEditor bounding box does not render properly #202

Open

NickEntin reviewed Jan 30, 2024

View reviewed changes

Example/SnapshotTests/__Snapshots__/SnapshotTestingTests/testUIKitTextView.393x852-16-4-3x.png Outdated Show resolved Hide resolved

NickEntin added the blocked This issue or pull request requires another change to be made first label Feb 9, 2024

NickEntin removed the blocked This issue or pull request requires another change to be made first label Feb 17, 2024

David Brunow and others added 9 commits February 17, 2024 21:12

Fix issue where text fields do not have "text field" or a hint in the…

b4cdc14

…ir description

Make implementation match VoiceOver

69c19d9

Update test function name to match snapshot

bae710b

Update Example/AccessibilitySnapshot/TextInputsViewController.swift

cfe0403

Co-authored-by: Nick Entin <[email protected]>

Handle text entry editing accessibility trait

0fa0164

Turn off recording

a265241

Add translations for German and Russian

39f19b1

Update snapshot images from iOS 16

16b7565

Add placeholder snapshots for iOS 13, 14, and 17 (these will need to …

c9a6305

…be updated from CI)

DavidBrunow force-pushed the bugfix/swiftUITextFieldMissingDescription branch from 1c5a779 to c9a6305 Compare February 18, 2024 03:12

NickEntin added 3 commits February 18, 2024 16:36

Update reference images for iOS 13 from CI

75ec118

Update reference images for iOS 14 from CI

2091648

Update reference images for iOS 17 from CI

c7d89e6

NickEntin reviewed Feb 19, 2024

View reviewed changes

NickEntin reviewed Feb 20, 2024

View reviewed changes

Do not set the accessibility frame for text views

b5b0b38

NickEntin approved these changes Feb 23, 2024

View reviewed changes

NickEntin merged commit f039d27 into cashapp:master Feb 23, 2024
9 checks passed

DavidBrunow deleted the bugfix/swiftUITextFieldMissingDescription branch February 23, 2024 04:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue where text entry do not have "text field" or a hint in the description #186

Fix issue where text entry do not have "text field" or a hint in the description #186

DavidBrunow commented Jan 17, 2024

DavidBrunow Jan 19, 2024

DavidBrunow Jan 19, 2024

NickEntin left a comment

DavidBrunow commented Feb 7, 2024

NickEntin commented Feb 9, 2024

NickEntin commented Feb 9, 2024 •

edited

Loading

NickEntin commented Feb 9, 2024

DavidBrunow commented Feb 9, 2024

NickEntin commented Feb 17, 2024

DavidBrunow commented Feb 18, 2024

DavidBrunow commented Feb 18, 2024 •

edited

Loading

NickEntin commented Feb 18, 2024 •

edited

Loading

NickEntin Feb 19, 2024

DavidBrunow commented Feb 19, 2024

NickEntin Feb 20, 2024

DavidBrunow Feb 20, 2024

DavidBrunow Feb 20, 2024

NickEntin commented Feb 22, 2024

DavidBrunow commented Feb 22, 2024

DavidBrunow commented Feb 22, 2024

DavidBrunow commented Feb 22, 2024

NickEntin left a comment

-            textViewWithText.accessibilityFrame = textViewWithText.frame
+            textViewWithText.accessibilityFrame = UIAccessibility.convertToScreenCoordinates(
+                textViewWithText.frame,
+                in: self
+            )

Fix issue where text entry do not have "text field" or a hint in the description #186

Fix issue where text entry do not have "text field" or a hint in the description #186

Conversation

DavidBrunow commented Jan 17, 2024

DavidBrunow Jan 19, 2024

Choose a reason for hiding this comment

DavidBrunow Jan 19, 2024

Choose a reason for hiding this comment

NickEntin left a comment

Choose a reason for hiding this comment

DavidBrunow commented Feb 7, 2024

NickEntin commented Feb 9, 2024

NickEntin commented Feb 9, 2024 • edited Loading

NickEntin commented Feb 9, 2024

DavidBrunow commented Feb 9, 2024

NickEntin commented Feb 17, 2024

DavidBrunow commented Feb 18, 2024

DavidBrunow commented Feb 18, 2024 • edited Loading

NickEntin commented Feb 18, 2024 • edited Loading

NickEntin Feb 19, 2024

Choose a reason for hiding this comment

DavidBrunow commented Feb 19, 2024

NickEntin Feb 20, 2024

Choose a reason for hiding this comment

DavidBrunow Feb 20, 2024

Choose a reason for hiding this comment

DavidBrunow Feb 20, 2024

Choose a reason for hiding this comment

NickEntin commented Feb 22, 2024

DavidBrunow commented Feb 22, 2024

DavidBrunow commented Feb 22, 2024

DavidBrunow commented Feb 22, 2024

NickEntin left a comment

Choose a reason for hiding this comment

NickEntin commented Feb 9, 2024 •

edited

Loading

DavidBrunow commented Feb 18, 2024 •

edited

Loading

NickEntin commented Feb 18, 2024 •

edited

Loading