prjseal / Giphy-Property-Editor

A Giphy Property Editor for Umbraco
MIT License
6 stars 4 forks source link

Missing property value converter #4

Closed abjerner closed 5 years ago

abjerner commented 5 years ago

Hi Paul,

As there currently is no property value converter, ModelsBuilder will generate a property of the type object instead string. Eg. as shown here:

///<summary>
/// Giphy Image: Search for a nice gif
///</summary>
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Umbraco.ModelsBuilder", "8.0.4")]
[ImplementPropertyType("giphyImage")]
public object GiphyImage => this.Value("giphyImage");

With a property value converter, you can tell ModelsBuilder what type the property should be. It's a minor thing, but would improve intellisense 😄

Let me know if you need any help with this 😉

prjseal commented 5 years ago

Thanks for raising this issue @abjerner but in the template it lets me use @Model.GiphyImage without a problem and it renders it out just fine.

abjerner commented 5 years ago

Like I said, this is a minor thing. And rendering out in your example works fine - because it's still a string under the hood.

But let's say I want to check whether the property has a value, I can't use:

if (Model.GiphyImage.IsNullOrWhiteSpace())
{

}

This is because the property type is object. Telling Models Builder (via a property value converter) that the property value is actually a string would allow developers to use string methods and extension methods.

prjseal commented 5 years ago

You make a good case there. It would also be good to have a very simple example of a property value converter in this project. Would you be able to submit a PR for it please if you have time?

abjerner commented 5 years ago

Yes, I can look into that in the evening 😉

prjseal commented 5 years ago

Thanks mate.

abjerner commented 5 years ago

I hadn't noticed that you don't already have a dependency for UmbracoCms.Core. I'm not sure how you wish to handle dependencies, so I haven't made a pull request.

But the property value converter should look something like:

using System;
using Umbraco.Core.Models.PublishedContent;
using Umbraco.Core.PropertyEditors;

namespace GiphyPropertyEditor
{

    public class GiphyPropertyValueConverter : PropertyValueConverterBase
    {

        /// <summary>
        /// Gets a value indicating whether the converter supports a property type.
        /// </summary>
        /// <param name="propertyType">The property type.</param>
        /// <returns>A value indicating whether the converter supports a property type.</returns>
        public override bool IsConverter(PublishedPropertyType propertyType)
        {
            return propertyType.EditorAlias == "GiphyPropertyEditor";
        }

        /// <summary>
        /// Gets the type of values returned by the converter.
        /// </summary>
        /// <param name="propertyType">The property type.</param>
        /// <returns>The CLR type of values returned by the converter.</returns>
        public override Type GetPropertyValueType(PublishedPropertyType propertyType)
        {
            return typeof(string);
        }

    }

}

The PropertyValueConverterBase has a few more virtual methods, but these are mostly for converting from a string to another format, so I don't think we need these here.

prjseal commented 5 years ago

Thanks for this mate. I will add it in tonight.

prjseal commented 5 years ago

@abjerner this should be resolved now. Please check I did it correctly if and when you have chance.