Umbraco-cms: Make DataEditor.GetValueEditor method virtual

[leekelleher]

When developing a custom Property Editor in v8, if the display is configurable by its Data Type, (e.g. hiding the label, or potentially changing the view), the only place to do this in the value editor's Configuration setter method.

Here's an example of how Nested Content does this for hiding the property's label: https://github.com/umbraco/Umbraco-CMS/blob/release-8.5.4/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs#L81-L91

_IMO, I don't think "display logic" should be done inside a property's setter method._

In v7, there was a method available called ConfigureForDisplay, that could be used to config the value editor.

This is how Nested Content did it back in v7, using the ConfigureForDisplay method: https://github.com/umbraco/Umbraco-CMS/blob/release-7.15.3/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs#L101-L113

---

I have 2 questions...

1. Was there a reason why ConfigureForDisplay was removed in v8?
   _(I've been digging through the git history - found where it initially was commented out - alas no answers about why)_

2. Would it be up for consideration to reintroduce ConfigureForDisplay? 😃

My reason for asking is that in Contentment I have a couple of property editors where a different view can be selected, e.g. say swapping out a dropdown list with a radiobutton list. _(I'm currently doing this via the Configuration setter method, it's not pretty code!)_

[abjerner]

Hi @leekelleher,

The DataEditor class has a virtual CreateValueEditor method, which could then be overriden like:

protected override IDataValueEditor CreateValueEditor()  {

    var editor = new DataValueEditor(Attribute)
    {
        HideLabel = true,
        View = "/My/Custom/View.html"
    };

    return editor;

}

It let's you set both the HideLabel and View properties, but from what I can tell, it doesn't have access to the configuration.

But on the other hand, the class also has a GetValueEditor method with the configuration as a parameter. This method is however currently not virtual, but wouldn't making it so be a viable solution?

https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Core/PropertyEditors/DataEditor.cs#L108

Suppose the method was virtual, I'd imagine we could do something like:

public override IDataValueEditor GetValueEditor(object configuration)
{

    var editor = base.GetValueEditor(configuration);

    if (editor is DataValueEditor valueEditor && configuration is MyConfiguration config)
    {
        valueEditor.HideLabel = config.HideLabel;
    }

    return editor;

}

[leekelleher]

Thanks @abjerner. Overriding GetValueEditor(object configuration) is a good option, feels cleaner - it'd work for my package needs. 👍

---

On a "Dream Corner" type of tangent...
I wonder if there is more we can do with configuring an editor using a property's data? e.g. say if nuPickers knew the property's alias and could return contextual data (for that specific property instance)? 🤔 This is where GetValueEditor(object configuration) is called: https://github.com/umbraco/Umbraco-CMS/blob/d205e98250317d197bc18299ebfe49c4d24e5ac5/src/Umbraco.Web/Models/Mapping/ContentPropertyDisplayMapper.cs#L35
The originalProp could be referenced in a new method - ConfigureForProperty is probably a better name that ConfigureForDisplay. _Anyway, I'm just daydreaming._ ☁️

[abjerner]

It's totally okay to daydream. But I fear it may require too much rewriting to fulfill your dreams 😒

[Shazwazza]

It doesn't have to be virtual to override... it's part of an interface.

You can explicitly implement it which is 100% the same as overriding:

IDataValueEditor IDataEditor.GetValueEditor(object configuration)
{
    return null;
}

Also make sure your DataEditor explicitly implements IDataEditor and there you go.

I was also going to mention this just be aware that the configuration object is by reference so if you modify it directly it will be modifying the cached version that comes out of the DataTypeService (IIRC).

[leekelleher]

Thanks @Shazwazza! I hadn't considered implementing both DataEditor, IDataEditor. I didn't want to loose the goodness of inheriting from DataEditor.

I'll give it a try! 🚀

[leekelleher]

It works well. I've ended up refactoring my property-editor, removed a bunch of extra code too, so all worked out fine. Thanks again for the tip, (I had forgot about explicitly implementing interfaces).

I'll close this off.

[nul800sebastiaan]

Additionally, GetValueEditor will be virtual from 8.7.0 onwards, as fixed in https://github.com/umbraco/Umbraco-CMS/pull/7710