umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.41k stars 2.66k forks source link

defaultConfig section has no effect in manifest file #9842

Closed adamhearn closed 3 years ago

adamhearn commented 3 years ago

In order to use the Multi URL Picker as a Macro parameter, I created the following manifest in my project:

{
  "propertyEditors": [
    {
      "alias": "UrlPickerMacro",
      "name": "Url Picker (Macro)",
      "group": "Macro Parameter Editors",
      "icon": "icon-link",
      "isParameterEditor": true,
      "editor": {
        "view": "~/umbraco/views/propertyeditors/multiurlpicker/multiurlpicker.html",
        "valueType": "JSON"
      },
      "defaultConfig": {
        "ignoreUserStartNodes": "0",
        "minNumber": "0",
        "maxNumber": "1"
      }
    }
  ]
}

Problem

It would seem that the defaultOptions section of the manifest file no longer has any effect? The same manifest file in Umbraco V7 works as expected (the Multi URL Picker editor used in a Macro is visually limited to and enforces at most, a single link).

Umbraco version

I am seeing this issue on Umbraco version: 8.11.1

Reproduction

Create manifest as above Restart app pool if necessary Create a Macro and configure a parameter with the newly added Parameter Editor "Url Picker (Macro)" Add the Macro to a content page Note that the URL picker does not visually indicate the editor is configured to allow only one link

no config prompt

Add multiple links, Submit Re-edit to confirm multiple links have been configured

Config not Applied

Expected Behaviour

I expect the section to be applied as it was in V7 and the editor to limit the number of links. The documentation suggests it's supported?

Additional Information

I have the same issue (passing defaultConfig settings) when doing the same for the RichText Editor.

patrickdemooij9 commented 3 years ago

Oke, I was able to investigate this a bit.

The default config is loading inside of DataEditorConverter: https://github.com/umbraco/Umbraco-CMS/blob/34e80d86e8c0b754f6b7a02e307f53cb32806bbe/src/Umbraco.Core/Manifest/DataEditorConverter.cs#L108. However, the config is only loaded if the property editor also contains the prevalues object: https://github.com/umbraco/Umbraco-CMS/blob/34e80d86e8c0b754f6b7a02e307f53cb32806bbe/src/Umbraco.Core/Manifest/DataEditorConverter.cs#L89.

Now, this means that you can use the following manifest to go around this:

{
    "propertyEditors": [
        {
            "alias": "UrlPickerMacro",
            "name": "Url Picker (Macro)",
            "group": "Macro Parameter Editors",
            "icon": "icon-link",
            "isParameterEditor": true,
            "editor": {
                "view": "~/umbraco/views/propertyeditors/multiurlpicker/multiurlpicker.html",
                "valueType": "JSON"
            },
            "prevalues": {

            },
            "defaultConfig": {
                "ignoreUserStartNodes": "0",
                "minNumber": "0",
                "maxNumber": "1"
            }
        }
    ]
}

This seems to be working in the lastest contribution branch. However, I wonder if we really need that check in the code. @emma-hq or @nul800sebastiaan, is this correct behavior? Should you be able to set a defaultconfig without specifying a prevalues element? If yes, we should change the code so that it works like that. If no, we should update the documentation to state that this is not possible.

nul800sebastiaan commented 3 years ago

Aha! Thanks for digging in @patrickdemooij9! Yes, that seems unnecessary to require prevalues, those are only applicable to a few datatypes (like the dropdown) - so that check should be removed. Good workaround for now! But this should be updated to not require the prevalues element.

umbrabot commented 3 years ago

Hi @adamhearn,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly Umbraco GitHub bot :-)

patrickdemooij9 commented 3 years ago

Created a PR for this: https://github.com/umbraco/Umbraco-CMS/pull/9877