Code review comment for lp://qastaging/~mterry/unity8/greeter-focus

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

In GreeterPrompt, you're resisting on doing the imperative way. Let it go! :)

First, there's no point in doing "focus = false". When you do "focus = true" somewhere it will already do "focus = false" on whatever item previously had it.

Secondly, you're still binding to the focus property. The binding on the item initially invisible will be immediately broken. The remaining one will be broken the first time you switch from button to text field or vice versa. Getting bindings broken is bad. Leads to misleading code. When you read a "foo: bar" binding in some qml file you never expect it to stop working out of nowhere.

Third. Since focus really works as if it were a single "property" or "variable" per FocusScope, it makes sense to do this focus assignment in a single place, centrally.

QML would be better off if they designed it as a FocusScope.focusedItem instead of spreading out writable boolean properties throughout all items...

So, here's my suggestion:
http://bazaar.launchpad.net/~dandrader/unity8/greeter-focus/revision/2535
That way you also don't have to explain yourself.

« Back to merge proposal