umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
Other
4.49k stars 2.69k forks source link

Choosing a color picker for the first time doesn't populate the label, consecutive clicks do work #4319

Closed hfloyd closed 5 years ago

hfloyd commented 5 years ago

Umbraco v.7.13.1

I added a new Color Picker data type, checked to use labels, and filled in several options:

image

I am using Models Builder, and was pleased to see on my next model generation that the ColorPicker property was converted to a strongly-typed object with two string properties: "Color" and "Label". I would assume that "Color" would return the HEX code (ex: "28593a") and label would return the label (ex: "green")...

But when debugging, I see that both properties are returning JSON strings for the "value" which is the hex code:

image

Either this is a bug in the PropertyValueConverter code, or I am missing something fundamental...

The code generated by ModelsBuilder:

/// <summary>Static getter for Alert Color</summary>
        public static Umbraco.Core.PropertyEditors.ValueConverters.ColorPickerValueConverter.PickedColor GetAlertColor(ICompAlertMessage that)
        {
            return that.GetPropertyValue<Umbraco.Core.PropertyEditors.ValueConverters.ColorPickerValueConverter.PickedColor>("AlertColor");
        }

Eventually I was able to get the result I wanted by stealing some code from the PropertyValueConverter and adding it to a partial for the Model:

       /// <summary>Static getter for Alert Color</summary>
        public static Umbraco.Core.PropertyEditors.ValueConverters.ColorPickerValueConverter.PickedColor GetAlertColor(ICompAlertMessage that)
        {
            var val = that.GetPropertyValue<string>("AlertColor");

            var cp = GetAllColorOptions().First(n => n.Color ==val);

            return cp;
        }

        private static IEnumerable<ColorPickerValueConverter.PickedColor> GetAllColorOptions()
        {
            var colors = new List<ColorPickerValueConverter.PickedColor>();
            var propertyType = CompAlertMessage.GetModelPropertyType(x => x.AlertColor);
            var preValues = ApplicationContext.Current.Services.DataTypeService.GetPreValuesCollectionByDataTypeId(propertyType.DataTypeId);

            foreach (var preValue in preValues.PreValuesAsDictionary)
            {
                if (preValue.Key != "useLabel")
                {
                    var jo = JsonConvert.DeserializeObject<JObject>(preValue.Value.Value);
                    colors.Add(
                        new ColorPickerValueConverter.PickedColor(jo["value"].ToString(), jo["label"].ToString()));
                }
            }

            return colors;
        }

But I have a strong feeling that this isn't the most efficient way to handle this...

nul800sebastiaan commented 5 years ago

Hey @hfloyd, what mode are you running ModelsBuilder in? I get a perfectly fine label value when I do @Model.Content.MyColors.Label so I'm not sure how to reproduce.

kjac commented 5 years ago

Whoop! PR in #4325

nul800sebastiaan commented 5 years ago

Yep, found out that this is exactly the problem, when you click a color for the very first time, it doesn't populate the label value, selecting another color and then the same color "fixes" it. Will review the PR very soon! 👍

nul800sebastiaan commented 5 years ago

Thanks @kjac for the wonderful help today! This has now been fixed and will be available in 7.13.2 due out Tuesday next week. Thanks for reporting @hfloyd !

hfloyd commented 5 years ago

Thanks @nul800sebastiaan and @kjac for the fast response and detective work! <3

hfloyd commented 5 years ago

Ps. @nul800sebastiaan - RE: ModelsBuilder Mode: I always use the VS Add-in and compile all models in a separate project from the website.

 <add key="Umbraco.ModelsBuilder.Enable" value="true" />
    <add key="Umbraco.ModelsBuilder.ModelsMode" value="" />
    <add key="Umbraco.ModelsBuilder.EnableApi" value="true" />
    <add key="Umbraco.ModelsBuilder.StaticMixinGetters" value="true" />
    <add key="Umbraco.ModelsBuilder.LanguageVersion" value="CSharp6" />