Code review comment for lp://staging/~gerboland/unity-api/appManVersion2

Revision history for this message
Michael Zanetti (mzanetti) wrote :

Here's a couple of comments of things we should think of. Not necessarily disagreeing with them, just worth a though/discussion imo:

====

20 + Q_PROPERTY(Stages supportedStages READ supportedStages NOTIFY supportedStagesChanged)

Do we assume that if an app claims support for both stages, all of its surfaces will support them? Probably yes.

====

31 + * Holds the current application focus state. True if a surface of this application is focused, false otherwise.

Really needed? what would we gain with this information? Wouldn't it make more sense to obtain it the other was round? For instance:

var focusedApp = SurfaceManager.appId(SurfaceManager.focusedSurfaceId)

====

78 + Q_INVOKABLE virtual bool suspend() = 0;
89 + Q_INVOKABLE virtual bool resume() = 0;

Where would you call this from? I'm not sure if we should expose this to unity but rather handle this stuff internally in the applicationmanager. We might add something like inhibitSuspension() at a later point if we want to give the user control over it.

Also it doesn't really match with the existing API, given that we have startApplication() and stopApplication() in ApplicationManager (not ApplicationInfo) such things should be at the same place. If we decide to expose this to the shell, I'm not opposed to have them in both places (ApplicationManager::suspendApplication(appId) and ApplicationInfo::suspend()).

====

301 void setFocused(bool focused);

What would this do?

review: Needs Information

« Back to merge proposal