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

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

"""
        Connections {
            target: ApplicationManager

            onFocusRequested: {
                [...]
                greeter.notifyAppFocused(appId);
            }

            onFocusedApplicationIdChanged: {
                greeter.notifyAppFocused(ApplicationManager.focusedApplicationId);
                [...]
            }

            onApplicationAdded: {
                [...]
                greeter.notifyAppFocused(appId);
                [...]
            }
        }
"""

All those greeter.notifyAppFocused calls look redundant and not really correct.

A focus request, as it name says, it's just a request. It will be processed by the window manager part of unity8 (eg PhoneStage or TabletStage) which will finally change the app that is focused, causing ApplicationManager.focusedApplicationId to change. Thus for every focus change request you will be calling greeter.notifyAppFocused twice. And what's worse, you're assuming that a request will always be successful and are telling greeter that a given app now has focus before it actually happens.

The same goes for the onApplicationAdded case.

Can't we have greeter.notifyAppFocused only in onFocusedApplicationIdChanged?

« Back to merge proposal