Skip to content
This repository was archived by the owner on Aug 11, 2024. It is now read-only.

Single AttributeGraph cycle detected for becomeFirstResponder #253

Closed
gordonbrander opened this issue Mar 25, 2022 · 3 comments · Fixed by #254
Closed

Single AttributeGraph cycle detected for becomeFirstResponder #253

gordonbrander opened this issue Mar 25, 2022 · 3 comments · Fixed by #254

Comments

@gordonbrander
Copy link
Collaborator

gordonbrander commented Mar 25, 2022

Issue introduced by #251.

We introduced synchronous becomeFirstResponder to fix keyboard tearing. However, in this one case, it is causing an AttributeGraph cycle. The cycle does seem to be shortcutted by our early-exit check within textViewDidBeginEditing ("View updating. Skipping.") so that's good.

Workaround... I'm at my wit's end. I've tried a bunch of stuff with MarkupViewRepresentable, none of which worked. Even refraining from setting the focus binding in the delegate doesn't solve it. If I call resignFirstResponder (or sometimes becomeFirstResponder) synchronously in updateUIView then I hit this case.

So it seems the only thing for it is to call it async, which means we need to time animations to when focus actually finishes, to prevent keyboard tearing. This means treating editor focus binding as "what I wish focus was", and not "what focus actually is".

Perhaps we can introduce callbacks, or have two bindings, one for desiredFocus and another for currentFocus?

Update: it looks like this may not work out. Unfortunately, it seems that becomeFirstResponder MUST be called synchronously to avoid animation jank with NavigationView sliding panel animation. This is because the view doesn't even get created until the animation is started (it's in a closure).

It may be work accepting this single attributegraph cycle warning for now. I'm not sure there is any workaround.

Update: another idea. Maybe we call first responder methods synchronously, but only in makeUIView?

Update 2: this seems to work. It's not a complete fix, but it at least solves for the problem of wanting to create a view and immediately focus it.

Steps to reproduce

  • Focus entry
  • Hit back button

Log:

2022-03-25 15:28:42.600867-0700 Subconscious[21497:7029634] [editor] updateUIViewFocus: call becomeFirstResponder
=== AttributeGraph: cycle detected through attribute 2231832 ==
2022-03-25 15:28:42.630402-0700 Subconscious[21497:7029634] [editor] textViewDidBeginEditing: View updating. Skipping.
2022-03-25 15:28:42.632594-0700 Subconscious[21497:7029634] [editor] textViewDidEndEditing: set focus binding to nil.
2022-03-25 15:28:42.632745-0700 Subconscious[21497:7029634] [action] setFocus(focus: nil, field: Subconscious.AppModel.Focus.editor)
2022-03-25 15:28:42.659326-0700 Subconscious[21497:7029634] [editor] textViewDidEndEditing: focus binding already in sync. Skipping.
@gordonbrander
Copy link
Collaborator Author

gordonbrander commented Mar 25, 2022

This library has the same issue blsage/iTextField#11

@gordonbrander
Copy link
Collaborator Author

@natanel has asked:
Can you explain the AttributeGraph that comes up while debugging sometimes?

Curt (Apple)
I’m afraid we can’t discuss implementation details, but be sure to check out the talk “Demystify SwiftUI” for the details of the dependency graph. https://developer.apple.com/wwdc21/10022 It’s a great talk and should help provide intuition on how things work.

https://www.bigmountainstudio.com/community/public/posts/65727-wwdc-2021-questions-answers-from-slack-the-unofficial-archive#:~:text=%40Natanel%20has%20asked,how%20things%20work.

@gordonbrander
Copy link
Collaborator Author

Clever autofocus hack (using .task to set a timer in view) https://www.vbutko.com/articles/how-to-manage-focus-state-using-focusstate-in-swiftui/

gordonbrander added a commit that referenced this issue Mar 26, 2022
Fixes #253

Follow-up to #251.

Fixes keyboard tearing for NavigationView animation while still allowing
us to call first responder change methods async in updateUIView, fixing
attribute graph cycle warnings.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant