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

Revert "NonReparentingFocus - Reparent focus node in deactivate to match framework #1969

Merged

Conversation

knopp
Copy link
Contributor

@knopp knopp commented Apr 24, 2024

This reverts commit 60bb442.

This causes some issues:

Tried to make a child into a parent of itself.
[***]: 'package:flutter/src/widgets/focus_manager.dart':
[***]: Failed assertion: line 1013 pos 12: 'child != this'
[***]: 
[***]: Either the assertion indicates an error in the framework itself, or we should provide substantially
[***]: more information in this error message to help you determine and fix the underlying cause.
[***]: In either case, please report this assertion by filing a bug on GitHub:
[***]:   https://github.com/flutter/flutter/issues/new?template=2_bug.yml
[***]: 
[***]: When the exception was thrown, this was the stack:
[***]: #2      FocusNode._reparent (package:flutter/src/widgets/focus_manager.dart:1013:12)
[***]: #3      FocusAttachment.reparent (package:flutter/src/widgets/focus_manager.dart:255:14)
[***]: #4      _NonReparentingFocusState.deactivate (package:super_editor/src/infrastructure/focus.dart:58:30)
[***]: #5      StatefulElement.deactivate (package:flutter/src/widgets/framework.dart:5683:11)

I think the reason for this is that FocusState from framework reparents itself to parent during build() for reasons not entirely clean to me. It's a fragile system and rolling custom focus widget seem to break some assumptions that rest of the framework does (i.e. related focus scope caching that was added recently).

I think we should figure out whether NonReparentingNode is necessary at all in the presence of OverlayPortal and if not remove it completely.

@knopp
Copy link
Contributor Author

knopp commented Apr 24, 2024

@knopp knopp force-pushed the revert_non_reparenting_node_fix branch from f21a716 to de1730d Compare April 24, 2024 13:37
@matthew-carroll matthew-carroll merged commit 76c6562 into superlistapp:main Apr 24, 2024
11 checks passed
@matthew-carroll
Copy link
Contributor

@angelosilvestre can you cherry pick this reversion?

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.

None yet

2 participants