Skip to content

Improve extensibility #134

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

Merged
merged 7 commits into from
May 16, 2023
Merged

Conversation

SuperJMN
Copy link
Contributor

@SuperJMN SuperJMN commented Oct 6, 2022

I wanted to create a feature for TreeDataGrid and found that it was almost impossible to do it without having at least:

  • A way to get the items in a Source
  • A way to get the children in a Source (if any).

Without those, people like me would have to force the type system beyond reasonable in some scenarios.

Take this as an example:
https://github.com/SuperJMN-Zafiro/Zafiro.Avalonia/blob/master/Zafiro.Avalonia/ScrollToSelectedItemBehavior.cs

As you see, this behavior is not straightforward to use because:

  1. It needs a type parameter (T).
  2. It needs to know how to get the children for T, hence the constructor argument that uses reflection on T to get the given property.

Sample usage would be:

<TreeDataGrid Source="{Binding Source}">
    <i:Interaction.Behaviors>
        <global:ScrollToSelectedItemBehavior x:TypeArguments="vm:Model">
        <x:Arguments>
            <x:String>Children</x:String>
        </x:Arguments>
        </global:ScrollToSelectedItemBehavior>
    </i:Interaction.Behaviors>
</TreeDataGrid>

T being:

class Model 
{ 
    ...
    IEnumerable<Model> Children { get; } 
    ...
}

With the changes I'm suggesting, the Behavior would stop requiring type parameters and reflection to know how to get the children.

IMHO, this is beneficial from an extensibility standpoint.

I hope you accept these changes :)

@dnfadmin
Copy link

dnfadmin commented Oct 6, 2022

CLA assistant check
All CLA requirements met.

@SuperJMN SuperJMN force-pushed the improve-extensibility branch from 672cde2 to c3fb825 Compare October 6, 2022 22:17
@SuperJMN SuperJMN requested a review from maxkatz6 October 7, 2022 08:16
@maxkatz6 maxkatz6 enabled auto-merge October 10, 2022 02:16
Copy link
Member

@grokys grokys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of nits you may want to fix, but LGTM!

auto-merge was automatically disabled October 19, 2022 15:40

Head branch was pushed to by a user without write access

@SuperJMN
Copy link
Contributor Author

Nits included :)

@grokys grokys merged commit 251f990 into AvaloniaUI:master May 16, 2023
@SuperJMN SuperJMN deleted the improve-extensibility branch May 16, 2023 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants