iOS 18 touch injection hit testing fix for SwiftUI view hierarchies#1323
Conversation
…on changes on iOS 18.
lolindrath
left a comment
lolindrath
left a comment
There was a problem hiding this comment.
Looking really good! I ran our tests and almost all of them passed. clearTextFromView is still failing - do you think that's related to this issue? The error I see is:
with label "Label name" is not tappable. It may be blocked by other views.
|
@lolindrath I can't really tell as I don't know what is the view hierarchy and test's logic in that failing test you mentioned. Would you like to share some more details? Meanwhile I verified that current implementations of tester methods for text manipulation are working with the SwiftUI TextField view. I also added related tests. |
|
Thank you very much for contributing this! Tagging in @congt who can also tag in some other two repo admins to help move this forward. I'm on sabbatical for a while and not actively doing work things. I'll give it a once over, but I no longer have access to run the Block internal test suite to make sure it doesn't have any other side effects. |
justinseanmartin
left a comment
justinseanmartin
left a comment
There was a problem hiding this comment.
Overall, this looks like a really well done change. Thanks for taking the time and effort to add all the tests. It seems like it could alter the behavior even in UIKit views, is that right? It's fine as long as it doesn't break behavior there.
FYI - There is a note about the supported iOS versions in the README that looks like it needs updating. It's already stale though. I'd guess this change also warrants a minor version bump and release cut to signify the notable added support. It is probably worth mentioning the SwiftUI support in the README as well. All this probably can be handled by repo maintainers after merging this PR as part of cutting a release, but defer to @congt et al on that.
| s.authors = 'Michael Thole', 'Eric Firestone', 'Jim Puls', 'Brian Nickel' | ||
| s.source = { :git => "https://github.com/kif-framework/KIF.git", :tag => "v#{ s.version.to_s }" } | ||
| s.platform = :ios, '11.0' | ||
| s.platform = :ios, '13.0' |
There was a problem hiding this comment.
Definitely seems reasonable to bump this IMO.
| - (float)animationSpeed | ||
| { | ||
| if (!self.keyWindow) { | ||
| UIWindow *keyWindow = [self windowSceneKeyWindow]; |
There was a problem hiding this comment.
Thanks for doing the update here
| /* | ||
| From observation - this can be either of following types: | ||
| - UIView type (e.g. when using UIViewRepresentable inside SwiftUI) | ||
| - specialized SwiftUI view compatible with UIView, | ||
| - newly introduced structure SwiftUI.UIKitGestureContainer implementing UIResponder interface. | ||
| What's important it seems it is compatible with setView:(UIView *) method. | ||
| */ |
There was a problem hiding this comment.
Great context, thank you for including this.
|
@justinseanmartin thank you for your feedback on this! Answering your question - whether or not it alters the current behavior for UIKit views. When there are just UIKit views in the view hierarchy it'll iterate it trying to create the new hit test context structure, which won't happen as it is only created when SwiftUI or SwiftUI + UIKit views are involved in the hierarchy. So it'll end up returning same ol' When I initially started working on this last year I tried to avoid that loop heuristically e.g. when the initially returned So far after running the entire test suite before and after those changes I feel that the performance impact is negligible. |
@BartlomiejWlodarczak - I'm not worried about the perf impact either FWIW. KIF spends a lot of cycles walking the view hierarchy and I don't think that |
|
@justinseanmartin based on this work, what's the possibility of moving this into a formal KIF tagged release? We're eager to consume this and move to our functional automation on iOS 18 simulators. Thanks! |
Sorry for the slow response. Looks like some tests are still broken. @BartlomiejWlodarczak Could you take a look? |
|
@congt Sorted it out together with some build warnings from the logs. |
|
@justinseanmartin @congt can you make a new release version with the merged changes? |
|
@ispatel22 - I'll coordinate with Cong to make sure a release gets cut. In the meantime, you should be able to point at revision 259c5e6. |
|
Here is the PR to bump the version #1325 |
|
3.11.2 was just released |
7 participants
This PR should fix the issue #1302
There were additional mechanisms provided by Apple for SwiftUI / UIKit gesture interoperability. They needed to implement it somehow, hence the presence of new structures
SwiftUI.UIKitGestureContainerand_UIHitTestContext. I tried to mimic what's actually going on during "real" touches on iOS 18.I had also needed to bump the deployment target to iOS 13, since I was planning on adding a basic SwiftUI testing screen to the tests host app.
@justinseanmartin
The UIWebView related logic can be ditched fully in favor of WKWebView if you decide to merge this PR.
I made adjustments for the rest of deprecation warnings.