Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

The implementation AppController and the location handling are in magnolia-ui-admincentral in the not so appropriate package name info.magnolia.ui.admincentral.app.simple.

 

Action: Find a better package structure

Action: Clean upp AppContext and SubAppContext from methods used internally only.

Proposal

...

    • g. info.magnolia.ui.framework.app.api
    • or jus out of the simple subpackage
  • info.magnolia.ui.admincentral.app.simple.AppContextImpl
    • general problem: used heavily by the appController and also for the app itself to get contextual information: serving two needs. see http://jira.magnolia-cms.com/browse/MGNLUI-403
      • The idea of AppContext is to expose functionality to the App. That is, to an instance of an app.

        It should not expose methods that are only used by AppControllerImpl. These should be hidden from all other classes.

        If necessary AppControllerImpl#runningApps should be changed to <String, AppContextImpl>. Also if the intention of AppContextImpl is unclear it should be renamed to reflect the fact that is not just the default implementation of AppContext, it does much more and in fact fullfils much of the contract defined by AppController.

      • rename the AppController to AppLauncher or similar, introduce a new AppController class which takes care of the contract to the AppLauncher and keep the AppContext as thin as possible, only serving contextual information to the actual App

AppController

info.magnolia.ui.admincentral.app.simple
move out of simple subpackage

info.magnolia.ui.admincentral.app.simple.AppControllerImpl
interface seems fine
usage of appId is should be changed to appname
some methods take the appId and the location as parameter, aphid is part of the location
general question:
usage of FocusEvent

info.magnolia.ui.admincentral.app.simple.AppControllerImpl#setViewPort
use focus event instead?

info.magnolia.ui.admincentral.app.simple.AppControllerImpl#getAppWithoutStarting
apparently the idea was to actually start the app for this usage

info.magnolia.ui.admincentral.app.simple.AppControllerImpl#startIfNotAlreadyRunningThenFocus
only used in test cases. see general question about focusEvent

info.magnolia.ui.admincentral.app.simple.AppControllerImpl#startIfNotAlreadyRunning
deprecated, maybe not right, see focusEvent

info.magnolia.ui.admincentral.app.simple.AppControllerImpl#doStartIfNotAlreadyRunning
this also checks if the app is running, if so it delegates to that app.. wrong naming

info.magnolia.ui.admincentral.app.simple.AppControllerImpl#stopApp
only used internally

info.magnolia.ui.admincentral.app.simple.AppControllerImpl#onLocationChanged
too heavy, should delegate to methods rather, see focusEvent

Location handling

DefaultLocation is assumed everywhere, a sign that the interface is too generic making it largely irrelevant.

Apps should be able to use Location objects of their own to make it easier to navigate to a specific place. Instead of constructing a fragment string directly you should be able to use a class such as PagesLocation and pass to it the parameters necessary.

 

Action: Make the Location interface useful by pulling up from DefaultLocation. 

Action: Escape location in URL.

Shell Apps

Q: Do we plan any improvements here?

...