umco / umbraco-inner-content

A helper library for Umbraco Doc Type based property editors providing overlays and conversion helpers
https://our.umbraco.org/projects/backoffice-extensions/inner-content/
MIT License
2 stars 16 forks source link

Discard unsaved changes doesn't work when model.value is an object #49

Closed dnwhte closed 5 years ago

dnwhte commented 5 years ago

I can't be sure if this is an Inner Content issue not (not sure how to test).

I have a simple property editor with two fields. For example, model.value.text and model.value.element. I make edits in the overlay and discard/cancel the changes and save. When I come back to the node, I see the "discarded" changes were saved.

Any ideas?

dnwhte commented 5 years ago

A little more info.

I don't think this is a solution, but I can get it working by making the close method on contentEditorOverlay the same as the submit method. The only difference is that the close method model is the old model.

https://github.com/umco/umbraco-inner-content/blob/develop/src/Our.Umbraco.InnerContent/Web/UI/App_Plugins/InnerContent/js/innercontent.js#L235

scope.contentEditorOverlay = {
    view: Umbraco.Sys.ServerVariables.umbracoSettings.appPluginsPath + "/innercontent/views/innercontent.dialog.html",
    show: false,
    submitButtonLabelKey: "bulk_done",
    closeButtonLabelKey: "general_cancel",
    submit: function (model) {
        if (scope.config.callback) {
            // Convert model to basic model
            scope.config.data.model = innerContentService.createDbModel(model.dialogData.item);

            // Notify callback
            scope.config.callback(scope.config.data);
        }
        scope.closeAllOverlays();
    },
    close: function (model) {
        if (scope.config.callback) {
            // Convert model to basic model
            scope.config.data.model = innerContentService.createDbModel(model.dialogData.item);

            // Notify callback
            scope.config.callback(scope.config.data);
        }
        scope.closeAllOverlays();
    }
};
dnwhte commented 5 years ago

Think I got it. It's because an object is a reference type.

One solution is to deep copy non primitive types before assigning to prop.value on the editorModel.

Create a function to check for primitives

var isPrimitive = function (test) {
    return (test !== Object(test));
};

Change https://github.com/umco/umbraco-inner-content/blob/develop/src/Our.Umbraco.InnerContent/Web/UI/App_Plugins/InnerContent/js/innercontent.js#L535 to

prop.value = isPrimitive(dbModel[prop.alias]) ? dbModel[prop.alias] : angular.copy(dbModel[prop.alias]);        

I can submit a PR if you are good with it.

mattbrailsford commented 5 years ago

Looks good to me. A PR would be awesome, thank you 🤘