Merge lp://staging/~mzanetti/unity8/fix-focus-on-app-launch into lp://staging/unity8

FAILED: Continuous integration, rev:2914
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3666/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4867
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2982
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2982
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4895
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4706
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4706/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4706
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4706/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4706
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4706/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4706
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4706/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4706
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4706/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4706
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4706/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3666/rebuild

"""
root.focus = false;
"""

This looks redundant. I think code should be caring about whom to give focus to, not in explicitly unfocusing the item that's losing it as this is the side-effect of focusing someone else anyway. Is that really needed?

--------------------------------

"""
stage.focus = true;
"""

In some other focus-related branch you have the Stage focusing itself on its own accord. Would be good to consolidate that responsibility in Shell if feasible to avoid clashes. Ideally I think Shell logic should be the one deciding which Shell immediate child will get focus at any given time.

> """
> root.focus = false;
> """
>
> This looks redundant. I think code should be caring about whom to give focus
> to, not in explicitly unfocusing the item that's losing it as this is the
> side-effect of focusing someone else anyway. Is that really needed?
>
> --------------------------------
>
> """
> stage.focus = true;
> """
>
> In some other focus-related branch you have the Stage focusing itself on its
> own accord. Would be good to consolidate that responsibility in Shell if
> feasible to avoid clashes. Ideally I think Shell logic should be the one
> deciding which Shell immediate child will get focus at any given time.

yeah, you're certainly having a point there... the current situation isn't nice... however, it's not that simple... sometimes some things need to be focused by a GlobalShortcut which is buried somewhere inside components, other times something deep down in a component trigger a focus change into another component... I'd love to rework this somehow and am thinking how to best solve it, but that's gonna be a bigger thing...

> """
> root.focus = false;
> """
>
> This looks redundant. I think code should be caring about whom to give focus
> to, not in explicitly unfocusing the item that's losing it as this is the
> side-effect of focusing someone else anyway. Is that really needed?

yes... I just moved it a bit from where it was before... currently the launcher works in a way that when it is closed it unfocuses itself and in shell we have:

Launcher { onFocusChanged: if (!focus) stage.focus = true }

so that's kinda ok for here... I do certainly agree that in general this needs to be consolidated in a way eventually.

On 04/04/2017 13:37, Michael Zanetti wrote:
>> """
>> root.focus = false;
>> """
>>
>> This looks redundant. I think code should be caring about whom to give focus
>> to, not in explicitly unfocusing the item that's losing it as this is the
>> side-effect of focusing someone else anyway. Is that really needed?
> yes... I just moved it a bit from where it was before... currently the launcher works in a way that when it is closed it unfocuses itself and in shell we have:
>
> Launcher { onFocusChanged: if (!focus) stage.focus = true }
>
> so that's kinda ok for here... I do certainly agree that in general this needs to be consolidated in a way eventually.

Maybe something like this would be a step in the right direction:

Launcher { onClosed: stage.focus = true }

On 04.04.2017 18:42, Daniel d'Andrada wrote:
> On 04/04/2017 13:37, Michael Zanetti wrote:
>>> """
>>> root.focus = false;
>>> """
>>>
>>> This looks redundant. I think code should be caring about whom to give focus
>>> to, not in explicitly unfocusing the item that's losing it as this is the
>>> side-effect of focusing someone else anyway. Is that really needed?
>> yes... I just moved it a bit from where it was before... currently the launcher works in a way that when it is closed it unfocuses itself and in shell we have:
>>
>> Launcher { onFocusChanged: if (!focus) stage.focus = true }
>>
>> so that's kinda ok for here... I do certainly agree that in general this needs to be consolidated in a way eventually.
>
> Maybe something like this would be a step in the right direction:
>
> Launcher { onClosed: stage.focus = true }
>
>

It doesn't necessarily only happen when closed... e.g. with alt+F1 you
focus it for keyboard navigation... when then you press esc the launcher
unfocuses itself and focus should go back to the stage even if the
launcher might not close because the setting says it should stay opened.