robotlegs / robotlegs-framework

An ActionScript 3 application framework for Flash and Flex
https://robotlegs.tenderapp.com/
MIT License
967 stars 261 forks source link

IMediatorMapping.autoRemoveEnabled not working with extension Starling View Map #158

Open krausste opened 10 years ago

krausste commented 10 years ago

Hi! I am using the default extension Mediator Map together with the extension Starling View Map (https://github.com/jamieowen/robotlegs2-starling-viewmap). This is causing the IMediatorMapping.autoRemoveEnabled to not work anymore. The reason is the handling of autoRemoveEnabled in the Mediator Map. In my opinion it should be applied when a mediator is going to be removed and not when the mediator is created.

I think the current handling is even a bug. When having more then one Mediator mapped to a view, but you only want to auto remove one of them, you would unmediate all mediators even if you set autoRemoveEnabled to false for the other Mediators.

I created a fix by creating an extension overriding the Mediator Map extension, but it is very dirty. I will try to attach my Mediator Map extension with the fix. It would be nice if you could fix this in the Mediator Map extension directly.

Thank you in advance!

Suggestion how to fix Mediator Map:

MediatorManager.as

public function removeMediator( mediator:Object, item:Object, mapping:IMediatorMapping ):void
{
    // the fix:
    if (!mapping.autoRemoveEnabled)
    {
        return;
    }

    const displayObject:DisplayObject = item as DisplayObject;

    if (displayObject)
        displayObject.removeEventListener(Event.REMOVED_FROM_STAGE, onRemovedFromStage);

    if (mediator && mapping.autoRemoveEnabled)
        destroyMediator(mediator);

}
creynders commented 10 years ago

When having more then one Mediator mapped to a view, but you only want to auto remove one of them, you would unmediate all mediators even if you set autoRemoveEnabled to false for the other Mediators.

Yes, you're quite right, that looks like a bug to me too.

I'd solve it differently though, I'd ditch MediatorManager#onRemovedFromStage and rewrite MediatorManager#addMediator like this:

displayObject.addEventListener(Event.REMOVED_FROM_STAGE, function(event:Event){
    if (mapping.autoRemoveEnabled){
        removeMediator(mediator, item, mapping);
    }
});

But I'm no expert on the mediatormap. @darscan What do you think?

krausste commented 10 years ago

This would only solve the issue for old display list view map. Keep in mind that there are already view map extensions for Starling and Away3D. Would be nice to have a solution that can handle all kind of views (Old Display List, Starling, Away3D etc.)