Fix issue with Focus call crashing

[andydotxyz]

Remove crash when calling Focus before canvas is visible.
Also fix issue where item focused before visible was not remembered (worked < 2.0.0).

Fixes #1893

Checklist:

• Tests included.
• Lint and formatter run with no errors.
• Tests all pass.

[toaster]

Whether or not this should be supported is an interesting question. However what is more important is:
"This used to work, did we mean to break it in 2.0.0"
to which the answer is clearly, No - this was an accident.

So do we want to proagate the break, and put it in the notes for all breaking changes in 2.0 upgrade notes, or should we fix it beacause it has been supported in the past.

I propose the former. Also, disabling the ability to focus an out-of-tree object was a conscious decision and therefore not an accident. I just did not think that this “feature” would be in use.
The bug in the implementation which led to the crash was an accident, of course.

Without this feature a develop that wishes an element to be focused will have to get a callback from the main() of their app to say that it has set the window, so the focusing can happen. This feels like it is bleeding different concerns of the app.

As soon as a developer has added the object to the content tree, it can be focused. I still cannot see where this is a problem.

[toaster]

Reasonable feature, but where is the test?

[toaster]

Also, This could be a different PR :).

[andydotxyz]

Test added

[andydotxyz]

As soon as a developer has added the object to the content tree, it can be focused. I still cannot see where this is a problem.

We have discussed this many times now - a complex app probably does not want to expose a widget reference all the way up the tree when creating and/or add a new "you're added" callback for main to call.

Let me propose another example (beyond what the original bug reporter showed us).
With data binding the developer no longer needs a reference to any widget to access values, and so they create the object tree and launch the app. Data binding handles all the data transfer so the application no longer needs any widget references. But the developer needs to somehow reach into the heirarchy to access a specific widget after it is set to the window so that it can be focused.

[andydotxyz]

Also, disabling the ability to focus an out-of-tree object was a conscious

In the future we need to get better at documenting the intentional breaks - it would have been ideal to include this in the upgrade notes so it does not surprise more people.

[toaster]

Let me propose another example (beyond what the original bug reporter showed us).
With data binding the developer no longer needs a reference to any widget to access values, and so they create the object tree and launch the app. Data binding handles all the data transfer so the application no longer needs any widget references. But the developer needs to somehow reach into the heirarchy to access a specific widget after it is set to the window so that it can be focused.

Understood. Seems we need some elegant solution without pitfalls for this.

[toaster]

Also, disabling the ability to focus an out-of-tree object was a conscious

In the future we need to get better at documenting the intentional breaks - it would have been ideal to include this in the upgrade notes so it does not surprise more people.

Agreed.

[toaster]

Almost there :).