gregwiechec / Alloy.HideTabs

Allow to hide tab and properties based on conditions
Apache License 2.0
4 stars 5 forks source link

Incorrect property gets hidden/shown when using attributes for controling property display #4

Open miroslav-mihaljevic-luminos opened 1 year ago

miroslav-mihaljevic-luminos commented 1 year ago

With current logic, the property that gets affected is the one used for attribute condition and not the one on which is the attribute.

...
[Display(
     Name = "Form Format",
     GroupName = Constants.ContentGroupNames.Form,
     Order = 46)]
   [RequiresLayoutRefresh]
   [Enum(typeof(FormFormat))]
   //This field will get hidden (which is wrong)
   public virtual FormFormat FormFormat { get; set; }

[Display(
    Name = "Extra Form Field",
    GroupName = Constants.ContentGroupNames.Form,
    Order = 60)]
[ShowPropertyWhenValueEquals(nameof(FormFormat), (int) FormFormat.TwoSteps)]
//This is the field I want to actually hide/show
public virtual ContentArea ExtraFormField { get; set; }
...

The problematic code is from AttributeBasedLayoutVisibilityResolver:

               if (attribute is ShowPropertyWhenValueEqualsAttribute showAttr)
                {
                    if (showAttr.Value.Equals(content.Property[property.Name /*Correct is showAttr.PropertyName */].Value) == false)
                    {
                        result.Add(showAttr.PropertyName); //Correct is property.Name
                    }
                    continue;
                }
                if (attribute is HidePropertyWhenValueEquals hideAttr)
                {
                    if (hideAttr.Value.Equals(content.Property[property.Name /*Correct is hideAttr.PropertyName*/].Value) == true)
                    {
                        result.Add(hideAttr.PropertyName); //Correct is property.Name
                    }
                }

My proposal is that the whole GetHiddenProperties() method gets replaced with this version:

    public IEnumerable<string> GetHiddenProperties(IContent content)
    {
        List<string> propertiesToHide = new();

        Type contentType = content.GetOriginalType();
        var allContentProperties = contentType.GetProperties(BindingFlags.Public | BindingFlags.Instance);

        foreach (PropertyInfo currentProperty in allContentProperties)
        {
            var visibilityAttributes =
                currentProperty.GetCustomAttributes().OfType<ILayoutElementVisibilityAttribute>();
            foreach (ILayoutElementVisibilityAttribute attribute in visibilityAttributes)
            {
                if (attribute is ShowPropertyWhenValueEqualsAttribute showAttr)
                {
                    var conditionProperty = content.Property[showAttr.PropertyName];
                    var targetValue = showAttr.Value;

                    if (targetValue.Equals(conditionProperty.Value) == false)
                        propertiesToHide.Add(currentProperty.Name);
                }
                else if (attribute is HidePropertyWhenValueEquals hideAttr)
                {
                    var conditionProperty = content.Property[hideAttr.PropertyName];
                    var targetValue = hideAttr.Value;

                    if (targetValue.Equals(conditionProperty.Value) == true)
                        propertiesToHide.Add(currentProperty.Name);
                }
            }
        }

        return propertiesToHide.GroupBy(x => x).Select(x => x.FirstOrDefault());
    }
miroslav-mihaljevic-luminos commented 1 year ago

I just figured out that the current logic is the intended behavior, although it is kind of misleading, I expected to put the attribute on the field that I want to hide and not on the field that should control the hiding, so I guess the above code can be treated as a possible extra feature.