Skip to content
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] Improve/extend the visual tree extensions #4300

Open
Sergio0694 opened this issue Oct 6, 2021 · 5 comments · Fixed by #4333
Open

[Feature] Improve/extend the visual tree extensions #4300

Sergio0694 opened this issue Oct 6, 2021 · 5 comments · Fixed by #4333
Milestone

Comments

@Sergio0694
Copy link
Member

Sergio0694 commented Oct 6, 2021

Describe the problem this feature would solve

The DependencyObjectExtensions exposes APIs that offer a very convenient (and efficient) way to interact, query and traverse items in the visual tree linked to a given object. There are currently some limitations though that force developers (eg. us in the Store team) to have to implement some additional features manually. I think it would make sense to address them with a single implementation in the Toolkit that we and others could then use as well. This would reduce complexity from other codebases and offer a streamlined and consistent way to perform all these more advanced operations when working with visual trees.

The current limitations are as follows, in no particular order:

  • There isn't an API to easily just get the collection of children for a given item. VisualTreeHelper exposes APIs to get the children count and then get children at a specified offset. We're missing a helper to just put these two together.
  • Especially when working with large visual trees, the default DFS search can be quite inefficient, and developers might want to optionally choose to use a different exploration mode, specifically BFS. This isn't currently available in the Toolkit, and it also has the issue of being relatively tricky to implement efficiently (as one would want to pool the temporary children buffers to avoid creating and throwing away arrays every single time a traversal is executed and the target list grows).
  • The FindAscendants and FindDescendants methods lack a corresponding OrSelf version like other APIs.

Describe the solution

I propose to add the following set of new APIs to solve the issues mentioned above:

namespace Microsoft.Toolkit.Uwp.UI
{
    public enum SearchType
    {
        DepthFirst,
        BreadthFirst
    }

    public static class DependencyObjectExtensions
    {
        // Get the children
        public static DependencyObject[] GetChildren(this DependencyObject element);
        public static T[] GetChildren<T>(this DependencyObject element) where T : notnull, DependencyObject;

        // Overloads with search type
        public static FrameworkElement? FindDescendant(this DependencyObject element, string name, StringComparison comparisonType, SearchType searchType);
        public static T? FindDescendant<T>(this DependencyObject element, SearchType searchType) where T : notnull, DependencyObject;
        public static DependencyObject? FindDescendant(this DependencyObject element, Type type, SearchType searchType);
        public static T? FindDescendant<T>(this DependencyObject element, Func<T, bool> predicate, SearchType searchType) where T : notnull, DependencyObject;
        public static T? FindDescendant<T, TState>(this DependencyObject element, TState state, Func<T, TState, bool> predicate, SearchType searchType) where T : notnull, DependencyObject;
        public static FrameworkElement? FindDescendantOrSelf(this DependencyObject element, string name, StringComparison comparisonType, SearchType searchType);
        public static T? FindDescendantOrSelf<T>(this DependencyObject element, SearchType searchType) where T : notnull, DependencyObject;
        public static DependencyObject? FindDescendantOrSelf(this DependencyObject element, Type type, SearchType searchType);
        public static T? FindDescendantOrSelf<T>(this DependencyObject element, Func<T, bool> predicate, SearchType searchType) where T : notnull, DependencyObject;
        public static T? FindDescendantOrSelf<T, TState>(this DependencyObject element, TState state, Func<T, TState, bool> predicate, SearchType searchType) where T : notnull, DependencyObject;
        public static IEnumerable<DependencyObject> FindDescendants(this DependencyObject element, SearchType searchType);

        // Find Ascendants/Descendants with self (and search type)
        public static IEnumerable<DependencyObject> FindDescendantsOrSelf(this DependencyObject element);
        public static IEnumerable<DependencyObject> FindDescendantsOrSelf(this DependencyObject element, SearchType searchType);
        public static IEnumerable<DependencyObject> FindAscendantsOrSelf(this DependencyObject element);
    }
}

Describe alternatives you've considered

Do nothing and let each developer reimplement the extensions needed manually. As mentioned above, this is not ideal as it adds complexity, increases the API surface to test and maintain for everyone, and likely will result in developers not implementing these APIs in the most efficient way possible (because everyone has limited time, or might not be experienced enough, or care).

Open questions

  • Should GetChildren return an IEnumerable<T> instead of an array?
  • Should we also add an extension to check whether an item is an ascendant/descendant of another?
  • Should we also add these corresponding extensions to the FrameworkElementExtensions type?
@Sergio0694 Sergio0694 added improvements ✨ extensions ⚡ feature request 📬 A request for new changes to improve functionality labels Oct 6, 2021
@Sergio0694 Sergio0694 added this to the 7.2/8.0? milestone Oct 6, 2021
@ghost
Copy link

ghost commented Oct 6, 2021

Hello, 'Sergio0694! Thanks for submitting a new feature request. I've automatically added a vote 👍 reaction to help get things started. Other community members can vote to help us prioritize this feature in the future!

@JustinXinLiu
Copy link
Contributor

Would be great if the Toolkit could support this!

@JohnnyWestlake
Copy link
Contributor

One I've found quite useful is something like FindFirstLevelDescendantsOfType<T>. Take the case of an ItemsControl with a button as the item template, running itemsControl.FindFirstLevelDescendantsOfType<Button>() would do a depth first search, but stop each downwards branch on the first time it comes across a button, return that button, and move onto the next branch.

@michael-hawker
Copy link
Member

  1. Right now we use Child/Parent for Logical functions and Ascendant/Descendant for Visual. Would it make sense for the GetChildren to be another parameter to FindDescendants which limits the depth of the tree, so like 1 would just get the immediate children? Or we just create a FindFirstLevelDescendants method for the immediate visual 'children'. I think sticking with IEnumerable makes sense, though I suppose if we make a dedicated method we could do an array? (Not sure of specific pros/cons here as we have the predicate versions for filtering while searching as well.)

  2. That could be interesting. Would it be something like parent.IsAscendantOf(child) and child.IsDescendantOf(parent)?

  3. I think it makes sense for us to mirror any of these across our logical/visual tree helpers, though the visual ones are probably the more useful of the bunch. I know for things we do in the Toolkit or like XAML Studio is where logical helpers become more interesting, though as we've learned some of the logical properties (Parent) aren't even set until something is in the visual tree... 🤦‍♂️ So, the FindChildren based ones are the more useful to mirror. I think based on this info, finding parent vs. finding ascendant should be similar...? So at least those 3/4s are useful vs. all if anything?

@ghost ghost added the In-PR 🚀 label Oct 21, 2021
@ghost ghost added Completed 🔥 and removed In-PR 🚀 labels Jan 4, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 5, 2022
@michael-hawker michael-hawker modified the milestones: 8.0, 7.1.3 Aug 23, 2022
@michael-hawker michael-hawker modified the milestones: 7.1.3, 8.0 Oct 20, 2022
@michael-hawker
Copy link
Member

Talked to Sergio and we decided we should keep these for 8.0 instead of including in the hotfix release that we decided to ship. Will revert the commit for now and then we'll work to port these all into the new Labs-type infrastructure and add these back along with some other helpers/tweaks we've discovered along the way like #4771 and the depth parameter.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants