Umbraco-cms: Add property in package.manifest to re-use property value converters for simple types

[bjarnef]

At the moment when you have a custom property editors and are working with either strongly typed models or using ModelsBuilder in Visual Studio, one has to implement a property value converter to return a specific type. It does make sense in many cases e.g. when storing json and one want to convert this to a specific model.

However as a frontend developer this might be an extra step and the cool thing about property editors in Umbraco v7 is that you can do a lot just with html, css and javascript without knowing about C#.

For example in my own package Switcher https://our.umbraco.org/projects/backoffice-extensions/switcher/ I can implement a property value converter to return either false or true (while ModelsBuilder at the moment just return an object). However it might be a bit overkill to add an extra assembly just for simple types and checkbox property editor already has a converter for this to return true/false from a stored int 0/1 - it would be cool if you just could re-use built-in property value converters in Umbraco core for simple types:

• String
• Int32
• Boolean
• (decimal/double/float) - e.g. for property editors allowing to store decimal values, like Umbraco.Decimal property editor.
• (DateTime)
• (String/int array) - e.g. as Umbraco.MultipleTextstring

I imagine the package.manifest could have a property for return type or output type like in the screenshot. https://our.umbraco.org/documentation/extending/property-editors/package-manifest

Original issue: https://issues.umbraco.org/issue/U4-10005

[nul800sebastiaan]

Sounds like a good idea, I'll have to discuss with the team soon, will get back to you!

[bjarnef]

Thanks @nul800sebastiaan
It seems a bit overkill that each smaller packages / property editors should implement there own property value converter and add an additional assembly to the project or the developer should implement a property value converter like this https://our.umbraco.com/packages/backoffice-extensions/switcher/bugs-feedback-and-suggestions/95608-using-switcher-with-modelsbuilder#comment-302289 where these already exists in core.

I guess you could end up with a bunch of int and bool property value converters then.

[bjarnef]

Hi @nul800sebastiaan

Any update on the outcome of the internal discussion? 😁
Maybe @Shazwazza @zpqrtbnk or @clausjensen have some ideas on this?

I few packages where this could be useful.

SVG Icon Picker
https://our.umbraco.com/packages/backoffice-extensions/svg-icon-picker/
(return string instead of object).

Switcher
https://our.umbraco.com/packages/backoffice-extensions/switcher/
(return bool instead of object).

[nul800sebastiaan]

Sorry for the delay, this sounds good! Not sure how this can be implemented yet but maybe PVCEs could look at an editor's valueType (in the package.manifest file)?.

I've marked this as "Up for grabs" so that you or someone else coming along could create a pull request for it.

[bjarnef]

@nul800sebastiaan are there any plans to include this in core? At the moment there a number of packages which save a simple value from the property editor, here are a few examples to name a few.

SVG Icon Picker
https://our.umbraco.com/packages/backoffice-extensions/svg-icon-picker/
(return string instead of object).

Switcher
https://our.umbraco.com/packages/backoffice-extensions/switcher/
(return bool instead of object).

Nesting Contently
https://our.umbraco.com/packages/backoffice-extensions/nesting-contently/
(return bool instead of object).

For example ModelsBuilder generate these properties as object (which is default).
Of course these packages can have their own property value converters added and included a additional assembly, but it seems a bit overkill and in core there is already property value converters from similar core property editors, so it would be nice to be able to re-use these.

As a start it would be great to be able to re-use value converters for these types.

• String
• Int32
• Boolean

[nul800sebastiaan]

We'd be happy to, the current state is that we're looking for contributions to make this happen! 👍

[bjarnef]

Maybe HQ can add some thoughts or ideas to get started with, e.g:

• Should we have a new property for this in package.manifest or should we get the type based on the existing valueType? https://our.umbraco.com/documentation/extending/property-editors/package-manifest#editor
• Does it need some changes in the database to make this work?
• I can find a property value converter for Textstring (textbox and textarea) https://github.com/umbraco/Umbraco-CMS/tree/v8/dev/src/Umbraco.Web/PropertyEditors/ValueConverters but I am not sure where a true/false (checkbox/toggle) is converted to bool and a numeric input is converted to int or decimal?

[nul800sebastiaan]

Sure:

• I would say yes, that seems like a good idea, since we then keep it fully backwards compatible, not breaking people's existing stuff. It might be that people already made custom PVCEs so we don't want to interfere there when they upgrade to a newer version.
• I would hope not, why would that be?
• The core ones live in: https://github.com/umbraco/Umbraco-CMS/tree/v8/dev/src/Umbraco.Core/PropertyEditors/ValueConverters

Feel free to keep asking questions if you want to work on this, we don't have a grand plan for this so we're hoping for someone to experiment with it and present us with a solution we can give feedback on, a quick prototype could reveal a lot of hairy things that I can't imagine right now.

[bjarnef]

Yes, it should not break existing custom PVCEs used in packages. Not sure if there is a specific way to check this?
Regarding database changes it would only be if it needed to store something specific for this, but I guess it is it possible without database changes (just thinking out loud).
Ahh, I was looking in src/Umbraco.Web/PropertyEditors/ValueConverters not src/Umbraco.Core/PropertyEditors/ValueConverters. Btw what are these differences on these, e.g. the two value converters for textstring (textarea/textbox), which are almost identical.

https://github.com/umbraco/Umbraco-CMS/blob/v8/dev/src/Umbraco.Core/PropertyEditors/ValueConverters/TextStringValueConverter.cs

https://github.com/umbraco/Umbraco-CMS/blob/v8/dev/src/Umbraco.Web/PropertyEditors/ValueConverters/TextStringValueConverter.cs

[nul800sebastiaan]

I'm guessing one of them is in use and the other one is not used anywhere. 🙈

[bjarnef]

Okay, maybe @Shazwazza or @zpqrtbnk know, which one is in use or if both are required?

A thought about the package.manifest, where we e.g. have a INT. But only from this how should we know, if the return value should be e.g. int or bool?

At the moment we have these options for valueType in package.manifest: https://our.umbraco.com/documentation/extending/property-editors/package-manifest#editor

STRING Stores the value as an nvarchar in the database
DATETIME Stores the value as datetime in the database
TEXT Stores the value as ntext in the database
INT Stores the value as a bigint in the database
JSON Stored as ntext and automatically serialized to a dynamic object

It seems there a few more, which maybe has been added later, but not documented?
DATE, DECIMAL, BIGINT, TIME and XML. Would it make sense to add a BOOL here?
https://github.com/umbraco/Umbraco-CMS/blob/853087a75044b814df458457dc9a1f778cc89749/src/Umbraco.Core/PropertyEditors/ValueTypes.cs#L25-L73

For my property editor Switcher I have value type INT, but I would like it to return a boolean (similar to true/false property editor when using e.g. umbracoNaviHide).

I think we either need..

• an additional property e.g. returnType: "BOOL"
• or it should be possible to specify other types in valueType property, e.g. valueType: "BOOL", where this is converted to 0/1 in database (similar to the current value INT, but the property value should be returned as bool instead of an int).

...
alias: "Our.Umbraco.Switcher",
name: "Switcher",
editor:{
     view: "~/App_Plugins/Switcher/switcher.html",
     valueType: "INT"
}
...

Example 1

...
alias: "Our.Umbraco.Switcher",
name: "Switcher",
editor:{
     view: "~/App_Plugins/Switcher/switcher.html",
     valueType: "INT",
     returnType: "BOOL"
}
...

Example 2

...
alias: "Our.Umbraco.Switcher",
name: "Switcher",
editor:{
     view: "~/App_Plugins/Switcher/switcher.html",
     valueType: "BOOL"
}
...

[bjarnef]

For Switcher with valueType: "INT" it reach this part of the code and Type is long? or nullable System.Int64
https://github.com/umbraco/Umbraco-CMS/blob/853087a75044b814df458457dc9a1f778cc89749/src/Umbraco.Core/PropertyEditors/DataValueEditor.cs#L163-L176

but it seems ModelsBuilder generate the property value of type object (Dll mode).

[bjarnef]

@Shazwazza @zpqrtbnk maybe you have any input to this and is there a reason we have two TextStringValueConverter?

Should we extend the package.manifest since this with the current one you might want to return INT or BOOL but in database both are represented as an INT value.

[ronaldbarendse]

@bjarnef Some PVCs within Umbraco.Core are extended/replaced in Umbraco.Web, so the part of the conversion that doesn't depend on web related code/libraries is done in Core and the rest in Web: https://github.com/umbraco/Umbraco-CMS/pull/8249.

I would recommend adding a new (optional) property within the manifest, so you don't end up with multiple registered PVCs (causing an exception).

For optimal extensibility, this should be a type/assembly reference, so you can easily reuse your own PVCs for multiple editors without editing code (open-closed principle).

[ronaldbarendse]

I would recommend adding a new (optional) property within the manifest, so you don't end up with multiple registered PVCs (causing an exception).

This property could either be named returnType (the type returned by ModelsBuilder generated properties) or objectType (referring to the PVC level/type). Umbraco can ship with default PVCs that only register themself for editors specifying this property in the manifest.

For optimal extensibility, this should be a type/assembly reference, so you can easily reuse your own PVCs for multiple editors without editing code (open-closed principle).

Thinking about this some more, this shouldn't be needed and would actually make the PVC registration work backwards (PVCs now register themself, instead of one being assigned to a specific editor).

[ronaldbarendse]

FYI there's already a default PVC for ValueTypes.Json: https://github.com/umbraco/Umbraco-CMS/blob/3bfd9b71e290354744d677440983ecd06c5c0788/src/Umbraco.Core/PropertyEditors/ValueConverters/JsonValueConverter.cs