Improve focus management #9074
Improve focus management #9074
Conversation
|
@Hixie Still needs docs but probably ready for some feedback. |
abarth
force-pushed
the
focus_tree3
branch
3 times, most recently
from
547cb28 to
3c8d4cd
Compare
There was a problem hiding this comment.
I think I'd prefer "extends Object with TreeDiagnostics". Layer is not is-a TreeDiagnostics. Consider the case of a class that inherits from the AbstractNode class and wants this.
There was a problem hiding this comment.
Also, should we port Widget, Element, and RenderObject to use TreeDiagnostics?
There was a problem hiding this comment.
Probably. I think one of those has a different (less dense) tree, but maybe we should make them all the same.
There was a problem hiding this comment.
RenderObject has a much more complicated tree but it uses the same API, so maybe we can mix in TreeDiagnostics and then just override the part that needs overriding to get the effect we have now.
There was a problem hiding this comment.
how deep are your trees? I guess there'll be 3 or 4 deep, no more, so this is probably manageable.
If they get longer than this we might want to consider using the attach/detach pattern.
There was a problem hiding this comment.
oh never mind i misunderstood the type of _manager.
There was a problem hiding this comment.
let's call this "unfocus" rather than "blur".
There was a problem hiding this comment.
can we include the manager and hasFocus in this?
There was a problem hiding this comment.
maybe just hasFocus since the manager is multi-line and includes this guy.
There was a problem hiding this comment.
we don't generally null out fields in dispose; if it is important to do so here for some reason, please add a comment as to why
There was a problem hiding this comment.
No real reason. I'll skip it.
There was a problem hiding this comment.
if i globalkey-reparent the EditableText's parent (the one that created the focus node) from one focus scope to another while the editable text is focused, what updates the focus tree?
There was a problem hiding this comment.
The current design leaves you focused in the old scope. You'll move to the new scope the next time you become focused. I wasn't sure what behavior we wanted in this case. Presumably if you stay in the same scope, you should remain focused but if you move to a new scope then maybe you should become unfocused?
There was a problem hiding this comment.
We discussed this in person. For the record, my concern was that the widget tree and the focus tree should remain similar, since we almost always end up with bugs when we have two trees that are related to each other but they spend time nonisomorphic.
abarth
force-pushed
the
focus_tree3
branch
4 times, most recently
from
80c7a5d to
dad9a0e
Compare
|
Should be ready for review again. PTAL. |
There was a problem hiding this comment.
we should have a test that verifies the output here is sane (just a dummy TreeDiagnosticsMixin class with an object hierarchy with a root with two children and one of those children has a child, and then compare the output to a known-good string).
|
|
| │ │ | ||
| ├─child node C: TestTree | ||
| │ | ||
| ''')); |
There was a problem hiding this comment.
Looks like the algorithm here is slightly wrong -- it's not terminating branches on the last child.
There was a problem hiding this comment.
Yes, that's caused by the simple TestTree code. I could make it fancier, but it didn't seem worthwhile.
| String debugDescribeChildren(String prefix) { | ||
| final StringBuffer buffer = new StringBuffer(); | ||
| for (TestTree child in children) | ||
| buffer.write(child.toStringDeep("$prefix \u251C\u2500child ${child.name}: ", "$prefix \u2502")); |
We now have an explicit focus tree that we manage. Instead of using GlobalKeys to manage focus, we use FocusNode and FocusScopeNode objects. The FocusNode is Listenable and notifies when its focus state changes. Focus notifications trigger by tree mutations are now delayed by one frame, which is necessary to handle certain complex tree mutations. In the common case of focus changes being triggered by user input, the focus notificiation still arives in the same frame.
3 participants
We now have an explicit focus tree that we manage. Instead of using
GlobalKeys to manage focus, we use FocusNode and FocusScopeNode objects.
The FocusNode is Listenable and notifies when its focus state changes.
Focus notifications trigger by tree mutations are now delayed by one
frame, which is necessary to handle certain complex tree mutations. In
the common case of focus changes being triggered by user input, the
focus notificiation still arives in the same frame.