Product Development

Page tree
Skip to end of metadata
  • Created on 2014-04-28, last edited by Magnolia International on 2014-05-08
Go to start of metadata

 

Rationale

Stumbled upon this issue when looking at MGNLUI-2101@jira (move of properties) and discovering MGNLUI-2809@jira (moving roles and users work via D&D but not via move action). Realizing this would be a constant source of bugs in custom implementations of apps in the future, I think we need to fix it before there's more custom implementations around. See  ARCHI-15 - Getting issue details... STATUS  for future improvements.

Problem

  • Move has to be implemented twice, once to be invoked by action, once from DropHandler.
  • Apps customising move (e.g. security, pages) had to reimplement both to provide their functionality correctly.
  • There is no connection between Action and DropHandler

Implementation
 

The question might pop in "why is action delegating to drop handler and not the other way around?". Answer is simply because of current dependencies between modules. Actions are delivered by content-app while D&D functionality comes from workbench and content-app has dependency on workbench.

Another question might be why is drop handler configured in app context rather then in sub app only? Since it's shared by both action (invoked from dialog) and tree (living inside browser sub app) app context is the closest common context both share.

 

 

  • No labels

5 Comments

  1. Magnolia International

    I would consider the dependency problem you mention a problem indeed, rather than a reason. Maybe the drop handler should be configured to execute an arbitrary action? (Just like the workbench configures "default" actions for double-click)

  2. Jan Haderka

    How about having MoveHandler in app context and having both action and drop handler to delegate execution to it?

    1. Magnolia International

      Yet another indirection ? What would be the benefit ? And why would other interactions not have that as well ? (e.g DeleteHandler etc)

  3. Mikaël Geljić

    Wow great stuff, I like it! I actually wasn't aware that we had two different impls for moving items... :S

    Just a bit of thoughts about the dependency thing, I'm personally not too fond of using the Vaadin DropHandler interface when coming from the move dialog; we had a bit of the same dependency issue for inplace editing.

    Before 5.3 we used to have JCR logic for editing properties in BrowserPresenter (then invoking this from the tree with a chain of listeners + from editProperty dialog). And I wanted inplace-editing (workbench) to perform save through an action (ui-contentapp). As a side note, ideally I would have reused the same action for inplace-editing and the editProperty dialog but couldn't find a clean way back then (and use cases are a bit different too).

    Anyway, I introduced an ActionEvent that takes an action 'name', itemId and arguments to fire from the workbench — which ultimately delegates to the ActionExecutor. The idea was to enable triggering actions from any point in the UI, although I admit that relying on action name is not a first-choice solution either.

    That's it, just wanted to chime in, in my opinion we don't have a silver-bullet solution for this yet, but on the other hand I could definitely live with what you've done Jan, it's way much better now. (smile)

    1. Jan Haderka

      Yeah, I was not so thrilled about using vaadin class either, but i figured that it's the one least specific, w/o having to introduce yet another interface for it, but if you all agree this is a good thing to do, then I'm happy to do it. All I want is to make sure this goes into 5.3, but we should not cave on doing it right just to get it in.