Page tree
Skip to end of metadata
Go to start of metadata

Abstract

The goal is to finalize the API and settle for naming and feature set. The documentation, both javadoc and otherwise also needs to be improved.

The current packaging needs to be rethought. This is to be tackled in a separate effort (See Concept - package reorganisation concretes).

We should finalize the API in terms of finding the right level of abstraction. The API should guide the app developer and not expose internals.

Design

The framework is a set of interfaces and abstract utility classes that assist by implementing an app.

These interfaces and classes are in magnolia-ui-framework. The implementation AppController and the location handling are in magnolia-ui-admincentral.

Proposed Implementation

From An App Developers perspective

Approved Steps currently in development

  1. Convert abstract *Apps (AbstractApp, AbstractSubApp, ...) to concrete classes (BaseApp, BaseSubApp, ...)
  2. Interface for AppFrameView (AppView), inject interface into BaseApp
  3. AppView.Listener implemented by AppContext -> listens to AppView events
    1. remove tab dependency from events
  4. Remove vaadin dependencies from AppContext
  5. Rename AppContextImpl ->AppInstanceControllerImpl
  6. AppInstanceControllerImpl implements two interfaces:
    1. AppContext for contextual information relevant for the app 
    2. AppInstanceController exposing the contract to the AppController 
  7. AppController: use AppDescriptorRegistry instead of AppLauncherLayoutManager

Missing:

  • info.magnolia.ui.framework.app.AppController#getCurrentAppInstanceController
    • should go
  • javadoc
  • Create app framework specific interfaces for e.g. AppView, SubAppView
    • SubAppView interface is still missing
  • stop method in subapp

Maybes

  1. remove vaadin dependencies from magnolia-ui-framework
    1. Move info.magnolia.ui.framework.view.View into vaadin-integration

Additional Information

Documentation

UML Diagrams

 

Overview

TypeLocationStatus QuoProposed Action

AbstractApp

 

info.magnolia.ui.framework.appabstract but many subclasses (e.g. PagesApp, MessagesApp) don't add additional functionalityconvert to non-abstract type & rename to BaseApp. Get rid of most subclasses. Inject the AppContext and App(Frame)View directly into the AbstractApp instead of passing it from the AppClass.
AbstractContentAppinfo.magnolia.ui.admincentral.app.content

see AbstractApp

Used for opening the ChooseDialog. A relict from the time we had the workbench definition bound to the App.

Remove class and create an interface for ChooseDialog

The idea of this class was serving the workbench configuration, as this has moved to the subApp there is no real usage for this class. The logic in there for serving a ChooseDialog hould be moved to an interface, so that the actual implementation of opening the Dialog is up to the App and not bound to the framework.

AbstractSubAppinfo.magnolia.ui.framework.appsee AbstractAppconvert to non-abstract - e.g. PageMainSubApp could go
AbstractItemSubAppinfo.magnolia.ui.admincentral.app.contentsee AbstractAppconvert to non-abstract- e.g. ContactsItemSubApp could completely go then
AppController 

The App controller that manages the lifecycle of running apps and raises callbacks to the app.
It provides methods to start, stop and focus already running {@link App}s.
Registers handlers to the following location change events triggered by the {@link LocationController}:

  • LocationChangedEvent
  • LocationChangeRequestedEvent

 

  • The AppController takes care of LocationChangedEvents coming from info.magnolia.ui.admincentral.app.simple.AppControllerImpl#onLocationChanged which can for example be fired by the info.magnolia.ui.framework.location.LocationController#goTo when clicking on icon in the AppLauncher. It takes the Location passed and extracts AppName and subAppId and gets the corresponding AppDescriptor from the info.magnolia.ui.framework.app.launcherlayout.AppLauncherLayoutManager.
  • It creates a new AppContext for the App
  • keeps history of running app and currently focused app
  • used by the MagnoliaShell to query the current Status of the appController
  • Used by FieldBuilders to get the AppContext and ComponentProviders for opening ChooseDialogs.
  • input from frist review
    • should not have a dependency to the ui classes (layout)
    • has to make sure a user have permission to run an App (-> not in Layout)
    • construction of an app can be specific (able to overwrite)
  • usage of FocusEvent: seems to not be used consistently. Could also be extended to get rid of the setViewPort and associated calls
  • usage of appId is should be changed to appName
  • some methods take the appId and the location as parameter, appId is part of the location
  • 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
AppContext 

The AppContext serves multiple different purposes:

  • An app uses the AppContext to interact with the App Framework. Some examples:
    • resolve the configured App Label for setting the correct title in the tab.
    • send notification Messages to the MessagesManager
  • The AppController uses it to delegate handling of location changes and to the app and subApp
  • Takes care of reading subApp configurations and mapping locations to to the correct subApp
  • creates both componentProviders for App and subApp
  • creates the AppFrameView and adds the subAppView to it
  • handles events send from the AppFrameView
    • onTabChanged, onTabClosed
    • uses MagnoliaTab to map tab to subAppContext

The general problem: used heavily by the appController and also for the app itself to get contextual information: serving multiple needs. see 

Error rendering macro 'jira'

Unable to locate Jira server for this macro. It may be due to Application Link configuration.

Should not be depending of any vaadin dependencies.

  • 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
  • Move some of the functionality of AppContext into the info.magnolia.ui.framework.app.launcherlayout.AppLauncherGroupEntry
    • info.magnolia.ui.admincentral.app.simple.AppContextImpl#getDefaultSubAppDescriptor
    • info.magnolia.ui.admincentral.app.simple.AppContextImpl#getSubAppDescriptorById
AppContextImpl  
  • find better name, it's more than an Impl of an AppContext
    • Tobias' propsal: rename AppController -> AppManager & AppContextImpl -> AppController
  • check whether AppContextImpl can go to framework (after package restructuring)

SubApp All SubApp's are considered to be equal. We can flag one (or even more) to be the default - that's the one that will be opened first and that won't be closable.
  • It should not be configurable what SubApp is the main. Wouldn't work e.g. for PagesApp if the editor would be the main (you don't get back to overview). 
  • No App without a "Main Subapp"! SecurityApp (only subapps of same level) will have to change - discussed with Andreas + Philipp
  • Should Main Subapp be an explicit type - or inlined in real App?
    • we don't want to be able to overwrite
SubbAppContext Basically in good shape - just offers a little too much.

Eliminate methods only relevant for the implementing types - not for someone writing an app.

Basically we should question all setters on this interface.

  • e.g. setAppContext(AppContext)
    • this should automatically happen behind the scenes (in the impl) - don't see why someone should explicitly call it from outside

AppDescriptor

SubAppDescriptor

 simply used for config
  • trivial: create common super-interface (class for the Configured impl's) - 90% of the code is identical
  • replace explicit setting of default by order of subapps?
AppFrameViewinfo.magnolia.ui.admincentral.app.simpleUsed to give Apps a consistent view. Holds a MagnoliaTabSheet and a Listener interface which makes the AppContext dependant on MagnoliaTabs.
  • create an interface with Listener interface inlined
  • remove all vaadin dependencies from interface
    • Reintroduce tab to tabId in MagnoliaShell. Use com.vaadin.server.KeyMapper? Kepp in mind that there can be multiple subApps open with subAppId, needs a unique identifier
    • Where is mapping between MagnoliaTab and SubApp? -> AppFrameView implementation or SubAppView?
  • make AbstractApp (BaseApp) implement the Listener interface, instead of AppContext
    • app is listening to events and delegates to appContext

 

Thoughts from first review

  • we should try to extract locationManagement

 

3) AppContextImpl 

    -> find better name, it's more than an Impl of an AppContext

    -> one option (Tobias' propsal): rename AppController -> AppManager & AppContextImpl -> AppController

 

 

4) AppLauncherLayoutManager

- only the AppLauncher should use it

 

5) structuring of modules: admin-central & framework: check whether AppContextImpl can to framework

   -> frist streamline the interfaces

 

6) Should Main Subapp be an explicit type - we don't want to be able to overwrite. 

 

7) Security App: this is the most exceptional one - will it be split into several apps or not? If yes, we could App Framework could be simplified even more

 

 

Next Steps:

Espen & me will do some further investigations, prepare better concept with clear decision points and contact u again for review - we won't do any coding for before then.

Tasks

type key summary assignee reporter priority status resolution created updated due

Unable to locate Jira server for this macro. It may be due to Application Link configuration.

 

 

 

  • No labels

3 Comments

  1. I think it should be possible to have more then one ChooserDialog for an app. My use case was creating a new AssetPlayer app. It needed a chooser dialog for the Assets App that was comfortable for selecting multiple items.

  2. The Workbench must be able to return all of the selected items. (Currently only has a method to get one selected item).

  3. The methods on AppControllerImpl can be simplified. We did many iterations there to get it right in combination with location handling. One problem was that an app would be focused multiple times causing flicker on the browser. We came to the conclusion that location handling is the way to control the lifecycle of apps.

    Also, it appears as if changes were made to support the choose dialog. Initially we would switch over to the app responsible for the entity (page, asset, contact etc) to be chosen. The app would be started if not running. Then it would show its choose dialog and then we switched back when a choice was made. I've been told that this has changed and that we're now starting a new app instance that will display only its choose dialog on top of another app. If so that probably adds complexity to AppControllerImpl and should be taken into account.