Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have the following Code to get ImageColorPicker child:

foreach (CustomTabItem customTabItem in SelectedWindowsTabControl.Items)
{
    TabItem ti = tabControl.ItemContainerGenerator.ContainerFromItem(customTabItem) as TabItem;
    Popup popup = (Helpers.FindVisualChild<Popup>(ti) as Popup);
    ImageColorPicker icp = (popup.Child as StackPanel).Children[0] as ImageColorPicker;

    ...
}

public class Helpers
{
    /// <summary>
    /// Return the first visual child of element by type.
    /// </summary>
    /// <typeparam name="T">The type of the Child</typeparam>
    /// <param name="obj">The parent Element</param>
    public static T FindVisualChild<T>(DependencyObject obj) where T : DependencyObject
    {
        for (int i = 0; i < VisualTreeHelper.GetChildrenCount(obj); i++)
        {
            DependencyObject child = VisualTreeHelper.GetChild(obj, i);
            if (child != null && child is T)
                return (T)child;
            else
            {
                T childOfChild = FindVisualChild<T>(child);
                if (childOfChild != null)
                    return childOfChild;
            }
        }
        return null;
    }
}

Here's the Template of the TabItem (part of it, the important part):

<ControlTemplate TargetType="{x:Type local:CustomTabItem}">
    <Grid Height="26" Background="{TemplateBinding Background}">
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="Auto" />
            <ColumnDefinition Width="Auto" />
        </Grid.ColumnDefinitions>
        <ContentPresenter Margin="5,0" HorizontalAlignment="Left" VerticalAlignment="Center" ContentSource="Header">
        </ContentPresenter>
        <StackPanel Grid.Column="1" Height="16" Margin="0,0,1,0" HorizontalAlignment="Center" VerticalAlignment="Center" Orientation="Horizontal">
            <ToggleButton x:Name="Edit" Width="16" Content="&#xE104;" Style="{StaticResource CustomizedMetroTabItemToggleButton}" ToolTip="Edit" />
            <Popup HorizontalOffset="{Binding Width, ElementName=Edit}" IsOpen="{Binding IsChecked, Mode=TwoWay, ElementName=Edit}" Placement="Left" PlacementTarget="{Binding ElementName=Edit}" PopupAnimation="Slide" StaysOpen="False" VerticalOffset="{Binding ActualHeight, ElementName=Edit}">
                <StackPanel>
                    <local:ImageColorPicker x:Name="ColorPicker" Width="100" Height="100" HorizontalAlignment="Center" Source="Images/ColorWheel.png" />
                </StackPanel>
            </Popup>
        </StackPanel>
    </Grid>
</ControlTemplate>

Is there a better way to get the ImageColorPicker then what I've done? (Getting the TabItem, then the Popup and then the ImageColorPicker, I am sure there's shorten)

share|improve this question
5  
You should try to find a woman that also wants a child this will increase chances. Now seriously: Try to find more descriptive (and less ambiguous) titles :) –  Nobody Mar 19 at 15:05
2  
nah, title is ok - funny titles draw attention ;) (pill.Forget(); would be a good way!) –  Mat's Mug Mar 19 at 15:07
1  
We do have a parenting site on StackExchange for those interested. –  user1306322 Mar 19 at 19:12
2  
Hot Network Questions pitcher plant title of the week! –  Jonathan Van Matre Mar 19 at 20:11

1 Answer 1

up vote 8 down vote accepted

I don't like a class that's just called Helpers - that's generally a code smell, something that ends up a big dumping ground for anything that doesn't quite fit anywhere else. Be more specific when naming things, perhaps VisualHierarchyHelper would be a better name?

I'm using a very similar method - the main difference is essentially the number ot return statements, and the childName parameter; I found this code on Stack Overflow a little while ago:

    /// <summary>
    /// Finds a Child of a given item in the visual tree. 
    /// </summary>
    /// <param name="parent">A direct parent of the queried item.</param>
    /// <typeparam name="T">The type of the queried item.</typeparam>
    /// <param name="childName">x:Name or Name of child. </param>
    /// <returns>The first parent item that matches the submitted type parameter. 
    /// If not matching item can be found, 
    /// a null parent is being returned.</returns>
    /// <remarks>
    /// http://stackoverflow.com/a/1759923/1188513
    /// </remarks>
    public static T FindChild<T>(this DependencyObject parent, string childName)
       where T : DependencyObject
    {
        if (parent == null) return null;

        T foundChild = null;

        var childrenCount = VisualTreeHelper.GetChildrenCount(parent);
        for (var i = 0; i < childrenCount; i++)
        {
            var child = VisualTreeHelper.GetChild(parent, i);
            var childType = child as T;
            if (childType == null)
            {
                foundChild = FindChild<T>(child, childName);
                if (foundChild != null) break;
            }
            else if (!string.IsNullOrEmpty(childName))
            {
                var frameworkElement = child as FrameworkElement;
                if (frameworkElement != null && frameworkElement.Name == childName)
                {
                    foundChild = (T)child;
                    break;
                }
            }
            else
            {
                foundChild = (T)child;
                break;
            }
        }

        return foundChild;
    }

Notice the guard clause preventing a NullReferenceException that your method would throw if obj was null. I think this is a pretty neat way of finding a child node in the visual tree.

That said, it might be personal preference, but I think the readability of your code could benefit from implicit typing (var), especially in cases like this where the type is already pretty obvious:

TabItem ti = tabControl.ItemContainerGenerator.ContainerFromItem(customTabItem) as TabItem;
Popup popup = (Helpers.FindVisualChild<Popup>(ti) as Popup);
ImageColorPicker icp = (popup.Child as StackPanel).Children[0] as ImageColorPicker;

Becomes:

var ti = tabControl.ItemContainerGenerator.ContainerFromItem(customTabItem) as TabItem;
var popup = (Helpers.FindVisualChild<Popup>(ti) as Popup);
var icp = (popup.Child as StackPanel).Children[0] as ImageColorPicker;

And here you would have to make sure popup isn't null before accessing its Child member, if you want to avoid that possible NullReferenceException.

Also, you're casting too much - T should be of the type you've specified, so the return type of FindChild<ImageColorPicker> is ImageColorPicker, casting it to ImageColorPicker is redundant.


Update

The ImageColorPicker child has a Popup parent, which has a StackPanel parent, which has a Grid parent, which has a TabItem parent.

You're not fully using the recursiveness of your function when you're getting the color picker. I'd believe you could get it like this:

var tab = tabControl.ItemContainerGenerator.ContainerFromItem(customTabItem) as TabItem;
var picker = VisualHierarchyHelper.FindChild<ImageColorPicker>(tab, "ColorPicker");

That should work, because the search is recursive; you don't need to get everything in-between.

share|improve this answer
    
Thats not exactly what I meant. Right now I use 3 lines to get the ImageColorPicker (btw, the Popup will never be null). Why cant I just do something like TabItem ti = tabControl.ItemContainerGenerator.ContainerFromItem(customTabItem) as TabItem; ImageColorPicker icp = Helpers.FindVisualChild<ImageColorPicker(ti) as ImageColorPicker; –  Ron Mar 19 at 15:40
    
There's one ImageColorPicker control in the TabItem but still, the picker variable is null, why is that? –  Ron Mar 19 at 15:54
    
Even with this FindVisualChild<T> implementation? (I'm investigating it right now) –  Mat's Mug Mar 19 at 15:55
    
Yes, even with the FindVisualChild<T> implementation. Something is weird and I cant understand what –  Ron Mar 19 at 15:58
1  
From MSDN: When you add content to a Popup control, the Popup control becomes the logical parent to the content. Similarly, the Popup content is considered to be the logical child of the Popup. The child content is not added to the visual tree that contains the Popup control. Instead, the child content is rendered in a separate window that has its own visual tree when the IsOpen property is set to true. –  Ron Mar 19 at 16:42

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.