leekelleher / umbraco-contentment

Contentment for Umbraco - a state of happiness and satisfaction
https://marketplace.umbraco.com/package/umbraco.community.contentment
Mozilla Public License 2.0
157 stars 72 forks source link

Datalist returns null from block settings #111

Closed bjarnef closed 3 years ago

bjarnef commented 3 years ago

What is the current behaviour?

We have a custom data source to select some devices / screen resolutions to hide some block elements, where we use some block settings.

public class DeviceDataSource : IDataListSource, IDataListSourceValueConverter
{
    private readonly Dictionary<string, DeviceType> _lookup;

    public DeviceDataSource()
    {
        var devices = new List<DeviceType>
        {
            new DeviceType {
                Name = "Mobile",
                Key = "mobile",
                Icon = "icon-iphone",
            },
            new DeviceType {
                Name = "Tablet",
                Key = "tablet",
                Icon = "icon-ipad",
            },
            new DeviceType {
                Name = "Desktop",
                Key = "desktop",
                Icon = "icon-display",
            },
        };
        _lookup = devices.ToDictionary(x => x.Key.ToString());
    }

    public string Name => "Devices";

    public string Description => "Data source for devices.";

    public string Icon => "icon-brick";

    public OverlaySize OverlaySize => OverlaySize.Small;

    public Dictionary<string, object> DefaultValues => default;

    public IEnumerable<ConfigurationField> Fields => default;

    public IEnumerable<DataListItem> GetItems(Dictionary<string, object> config)
    {
        return _lookup
            .Select(x => new DataListItem
            {
                Name = x.Value.Name,
                Value = x.Key,
                Icon = x.Value.Icon,
                Description = ""
            });
    }

    public Type GetValueType(Dictionary<string, object> config) => typeof(DeviceType);

    public object ConvertValue(Type type, string value) => _lookup.ContainsKey(value) == true ? _lookup[value] : null;
}

public class DeviceType
{
    public string Name { get; set; }
    public string Key { get; set; }
    public string Icon { get; set; }
}

In this use-case the content types also vary by culture, but not the element types or block settings.

This works fine when using the property directly on a Settings node (which vary by culture). However if accessing the property from a Settings element type on a block element placed in Block List on a Standard Page (which vary by culture) the property returns null instead of the selected items.

What is the expected behaviour?

The property on either block content or block settings to return the selected items.

Please tell us about your set-up:

Anything else?

Settings node

image

Block element settings

image

Block element

image

bjarnef commented 3 years ago

It does however work correct with these settings based on enum values, so I wonder if it is specific related to a custom data source?

public enum TextWidth
{
    [EnumMember(Value = "small")]
    Small,

    [EnumMember(Value = "medium")]
    Medium,

    [EnumMember(Value = "large")]
    Large
}

public enum TextPosition
{
    [EnumMember(Value = "left")]
    Left,

    [EnumMember(Value = "center")]
    Center,

    [EnumMember(Value = "right")]
    Right
}

public enum TextSize
{
    [EnumMember(Value = "small")]
    Small,

    [EnumMember(Value = "medium")]
    Medium,

    [EnumMember(Value = "large")]
    Large
}

image

leekelleher commented 3 years ago

@bjarnef I haven't looked into the issue with the Block List Settings yet, but wanted to pick up on the use of the EnumMember attribute - that the Enum data-source does not take it into account, so the value stored is the Enum's name.

I wasn't even aware of the EnumMember attribute before seeing your example above. ๐Ÿ˜ฎ

At present, the Enum data-source is only aware of the DataListItemAttribute and DescriptionAttribute. https://github.com/leekelleher/umbraco-contentment/blob/1.4.2/src/Umbraco.Community.Contentment/DataEditors/DataList/DataSources/EnumDataListSource.cs#L80-L86


re: Block List, I haven't actually used it on any projects myself yet. I'll get around to looking at it.

bjarnef commented 3 years ago

@leekelleher ahh okay, not a bit issue though as I compare with the enum and not specifically the value in EnumMember. However if re-using it in other part of code, EnumMember is useful for serialization of data.

bjarnef commented 3 years ago

@leekelleher I am sure you are busy with many other projects and v2.0 of Contentment ๐Ÿ˜Ž I think it may be something inside the DataListValueConverter. https://github.com/leekelleher/umbraco-contentment/blob/develop/src/Umbraco.Community.Contentment/DataEditors/DataList/DataListValueConverter.cs

leekelleher commented 3 years ago

@bjarnef I've tried taking a look into this over the past hour or so... I can reproduce the issue, but it looks to be something how Umbraco is calling the ValueConverter methods. This issue goes beyond Contentment/DataList.

Here's where I got to with my debugging...

On a Block List item, when accessing a .Content's property value, it will call the ConvertSourceToIntermediate() method first, then it will call the ConvertIntermediateToObject() method. However when accessing a .Settings' property value, it will only call the ConvertIntermediateToObject() method - it does not call ConvertSourceToIntermediate(). ๐Ÿ˜ข

This means that the inter value being passed in to DataList's ValueConverter is in the source format, in this particular case, a JArray, so the DataList's ConvertSourceToIntermediate() method doesn't know what to do with it. (A workaround would be for it to support JArray objects too. But that's a workaround, not a fix to the underlying issue.)

I started to dig around Umbraco's source, to see what I could find. I got as far as here: PublishedElementPropertyBase's GetInterValue() method, this does have an initialized flag, so I'm thinking that somehow that's already being set (somehow? ๐Ÿคท) when the .Settings element is being used/called.

At this point, I don't have much time to start debugging Umbraco core, (sorry). All I know is that would impact other property-editor value-converters, not only Contentment/DataList. ๐Ÿ˜ฌ

bjarnef commented 3 years ago

@leekelleher thanks for looking into this - really appreciated ๐Ÿ˜Š๐Ÿ™Œ Okay, so if sound a workaround could be to add the property as Content on the block element type instead of Settings element.

Maybe @warrenbuckley has some deeper insights how the value converter works regarding the Block List and in this case Settings element?

leekelleher commented 3 years ago

@bjarnef I've put a workaround fix in commit e03cb2114e8691f06af359a9b06017bc8197002a, currently in the v2.0 branch. I'll look to backport it for a v1.4.3 patch release.

bjarnef commented 3 years ago

@leekelleher cool, would be a great workaround for now. Would be great to investigate further how the .Settings could return the correct type though, but this workaround might be good enough for now ๐Ÿ˜

leekelleher commented 3 years ago

@bjarnef FYI, released v1.4.3.

bjarnef commented 3 years ago

@leekelleher working on a project using Contentment v3.0 and noticed there's a Buttons list editor as well, which also support icons. I wonder if we can specify specific icons for enum values?

For example:

public enum Alignment
{
     Left,
     Center,
     Right
}

Or do we need to create a custom data source for this like above?

Maybe a custom Contentment attribute if not something in .NET ๐Ÿค”

public enum Alignment
{
     [ContentmentMember(Icon = "align-left")
     Left,

     [ContentmentMember(Icon = "align-center")
     Center,

     [ContentmentMember(Icon = "align-right")
     Right
}
leekelleher commented 3 years ago

@bjarnef The DataListItemAttribute already has an Icon field. ๐Ÿ‘

bjarnef commented 3 years ago

@leekelleher oh, that is beautiful, just what I need ๐Ÿ˜Ž Combined with custom SVG icons that's a pretty cool feature. #h5yr ๐Ÿ™Œ๐Ÿ‘

public enum Alignment
{
    [DataListItem(Icon = "align-left")]
    [EnumMember(Value = "left")]
    Left,

    [DataListItem(Icon = "align-center")]
    [EnumMember(Value = "center")]
    Center,

    [DataListItem(Icon = "align-right")]
    [EnumMember(Value = "right")]
    Right
}

image