New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/auto watermark #2722

Merged
merged 24 commits into from Nov 19, 2016

Conversation

Projects
None yet
3 participants
@xxMUROxx
Contributor

xxMUROxx commented Oct 29, 2016

AutoWatermark is able to get the DisplayAttribute from the bound property in following cases:

  • Binding that is supported
    • "{Binding Path=Property}"
    • "{Binding Path=Property.SubProperty}"
    • "{Binding Path=CollectionProperty}"
    • "{Binding Path=CollectionProperty[0].SubProperty}"
  • Binding that is not supported
    • "{Binding Path=CollectionProperty[0]}"

Tasks

  • Searches for WatermarkAttribute on bound property (like stated above)
  • Decision: Is property name adequate
  • Decision: Should error/warning shown if property does not have DisplayAttribute
  • Error must occur if control does not support AutoWatermark

closes #2712

@punker76

This comment has been minimized.

Show comment
Hide comment
@punker76

punker76 Oct 29, 2016

Member

@xxMUROxx We can do this for .Net 4 too, look at this here which works...

        private static void OnControlWithAutoWatermarkSupportLoaded(object o, RoutedEventArgs routedEventArgs)
        {
            FrameworkElement obj = (FrameworkElement)o;
            obj.Loaded -= OnControlWithAutoWatermarkSupportLoaded;

            DependencyProperty dependencyProperty;

            if (!AutoWatermarkPropertyMapping.TryGetValue(obj.GetType(), out dependencyProperty))
            {
                throw new NotSupportedException($"{nameof(AutoWatermarkProperty)} is not supported for {obj.GetType()}");
            }
            var binding = obj.GetBindingExpression(dependencyProperty);

            if (binding != null)
            {
                var dataItem = binding.DataItem.GetType();
#if NET4_5
                var propertyName = binding.ResolvedSourcePropertyName;
#else
                var propertyName = binding.ParentBinding?.Path?.Path;
#endif
                if (propertyName != null)
                {
                    var property = dataItem.GetProperty(propertyName, BindingFlags.GetProperty | BindingFlags.Public | BindingFlags.Instance);
                    if (property != null)
                    {
#if NET4_5
                        var attribute = property.GetCustomAttribute<WatermarkAttribute>();
#else
                        var attribute = property.GetCustomAttributes(typeof(WatermarkAttribute), false).FirstOrDefault() as WatermarkAttribute;
#endif
                        if (attribute != null)
                        {
                            obj.SetValue(WatermarkProperty, attribute.Caption);
                        }
                    }
                }
            }
        }
#endif

But we must make a note to the users, that this feature uses reflection, which can have performance issues if you use this on many controls with watermark in a UI.

Member

punker76 commented Oct 29, 2016

@xxMUROxx We can do this for .Net 4 too, look at this here which works...

        private static void OnControlWithAutoWatermarkSupportLoaded(object o, RoutedEventArgs routedEventArgs)
        {
            FrameworkElement obj = (FrameworkElement)o;
            obj.Loaded -= OnControlWithAutoWatermarkSupportLoaded;

            DependencyProperty dependencyProperty;

            if (!AutoWatermarkPropertyMapping.TryGetValue(obj.GetType(), out dependencyProperty))
            {
                throw new NotSupportedException($"{nameof(AutoWatermarkProperty)} is not supported for {obj.GetType()}");
            }
            var binding = obj.GetBindingExpression(dependencyProperty);

            if (binding != null)
            {
                var dataItem = binding.DataItem.GetType();
#if NET4_5
                var propertyName = binding.ResolvedSourcePropertyName;
#else
                var propertyName = binding.ParentBinding?.Path?.Path;
#endif
                if (propertyName != null)
                {
                    var property = dataItem.GetProperty(propertyName, BindingFlags.GetProperty | BindingFlags.Public | BindingFlags.Instance);
                    if (property != null)
                    {
#if NET4_5
                        var attribute = property.GetCustomAttribute<WatermarkAttribute>();
#else
                        var attribute = property.GetCustomAttributes(typeof(WatermarkAttribute), false).FirstOrDefault() as WatermarkAttribute;
#endif
                        if (attribute != null)
                        {
                            obj.SetValue(WatermarkProperty, attribute.Caption);
                        }
                    }
                }
            }
        }
#endif

But we must make a note to the users, that this feature uses reflection, which can have performance issues if you use this on many controls with watermark in a UI.

@punker76

By the way, can you revert the Xaml style changes? Thx

Show outdated Hide outdated src/MahApps.Metro/MahApps.Metro.Shared/Controls/Helper/TextBoxHelper.cs
FrameworkElement element = d as FrameworkElement;
if (element != null)
{
element.Loaded += OnControlWithAutoWatermarkSupportLoaded;

This comment has been minimized.

@punker76

punker76 Oct 29, 2016

Member

Possible memory leak...

@punker76

punker76 Oct 29, 2016

Member

Possible memory leak...

This comment has been minimized.

@xxMUROxx

xxMUROxx Oct 29, 2016

Contributor

Because of the event subscription? Shouldn't it be enough to unsubscribe directly after the event was raised the first time?

@xxMUROxx

xxMUROxx Oct 29, 2016

Contributor

Because of the event subscription? Shouldn't it be enough to unsubscribe directly after the event was raised the first time?

Show outdated Hide outdated ...MahApps.Metro/MahApps.Metro.Shared/Controls/Helper/WatermarkAttribute.cs
public string Caption { get; set; }
}
#endif
}

This comment has been minimized.

@punker76

punker76 Oct 29, 2016

Member

Why is it necessary to introduce a new attribute? Is the Display attribute too bad?

@punker76

punker76 Oct 29, 2016

Member

Why is it necessary to introduce a new attribute? Is the Display attribute too bad?

This comment has been minimized.

@xxMUROxx

xxMUROxx Oct 29, 2016

Contributor

No, of course, it is not.

@xxMUROxx

xxMUROxx Oct 29, 2016

Contributor

No, of course, it is not.

@xxMUROxx

This comment has been minimized.

Show comment
Hide comment
@xxMUROxx

xxMUROxx Oct 31, 2016

Contributor

@punker76 XAML-Style change was introduces unintentionally. I will revert them

Contributor

xxMUROxx commented Oct 31, 2016

@punker76 XAML-Style change was introduces unintentionally. I will revert them

@xxMUROxx

This comment has been minimized.

Show comment
Hide comment
@xxMUROxx

xxMUROxx Nov 1, 2016

Contributor

@punker76 reverted xaml, added .net4 support and hopefully removed memory leak

Contributor

xxMUROxx commented Nov 1, 2016

@punker76 reverted xaml, added .net4 support and hopefully removed memory leak

@punker76

This comment has been minimized.

Show comment
Hide comment
@punker76

punker76 Nov 18, 2016

Member

@xxMUROxx It seems this feature is done. So can I merge it?

Member

punker76 commented Nov 18, 2016

@xxMUROxx It seems this feature is done. So can I merge it?

@xxMUROxx

This comment has been minimized.

Show comment
Hide comment
@xxMUROxx

xxMUROxx Nov 18, 2016

Contributor

@punker76 yes

Contributor

xxMUROxx commented Nov 18, 2016

@punker76 yes

@punker76 punker76 added this to the 1.4.0 milestone Nov 18, 2016

@punker76 punker76 merged commit 2056eb3 into MahApps:develop Nov 19, 2016

1 check passed

continuous-integration/teamcity Finished TeamCity Build MahApps.Metro PullRequest :: MahApps.Metro PullRequests : Tests passed: 61
Details
@kamilkosek

This comment has been minimized.

Show comment
Hide comment
@kamilkosek

kamilkosek Nov 19, 2016

Thanks guys

kamilkosek commented Nov 19, 2016

Thanks guys

@xxMUROxx xxMUROxx deleted the xxMUROxx:feature/AutoWatermark branch Feb 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment