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

feat: expose TreeViewItem expansion status to root event. #16984

Merged
merged 3 commits into from
Sep 12, 2024

Conversation

rabbitism
Copy link
Contributor

What does the pull request do?

Expose TreeViewItem expansion change status to a RoutedEvent in root TreeView. Thus developer can easily track the change.

What is the current behavior?

No such a convenient way.

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

Since TreeView instance is already available in TreeViewItem, just manually raise a new defined event.

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0051738-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

/// <summary>
/// Represents the event that is raised when the expansion state of a <see cref="TreeViewItem"/> changes within a <see cref="TreeView"/>.
/// </summary>
public event EventHandler<TreeViewItemExpansionChangedEventArgs> ExpansionChanged
Copy link
Member

Choose a reason for hiding this comment

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

For Expander, we use ExpandedEvent/ExpandingEvent and CollapsedEvent/CollapsingEvent. It would be expected to have at least ExpandedEvent and CollapsedEvent here for consistency, but not ExpansionChanged.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's skip ExpandingEvent and CollapsingEvent events for now, as these are cancellable and more tricky to implement.

ExpandedEvent and CollapsedEvent should be enough for most apps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0051743-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Comment on lines 56 to 66
/// <summary>
/// Defines the <see cref="Expanded"/> event.
/// </summary>
public static readonly RoutedEvent<RoutedEventArgs> ExpandedEvent =
RoutedEvent.Register<TreeView, RoutedEventArgs>(nameof(Expanded), RoutingStrategies.Bubble);

/// <summary>
/// Defines the <see cref="Collapsed"/> event.
/// </summary>
public static readonly RoutedEvent<RoutedEventArgs> CollapsedEvent =
RoutedEvent.Register<TreeView, RoutedEventArgs>(nameof(Collapsed), RoutingStrategies.Bubble);
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I haven't noticed at first, but why does TreeView has these events? And not TreeViewItem? Makes more sense to be on the item.

It's still possible to handle these events on the TreeView or any other higher-position control, since this event bubbles:

treeView.AddHandler(TreeViewItem.CollapsedEvent, Handler);

Also with 11.2, it's possible to subscribe to any routed event:

<TreeView TreeViewItem.CollapsedEvent="Handler" ItemsSource="{Binding Source}" />

Copy link
Contributor Author

@rabbitism rabbitism Sep 12, 2024

Choose a reason for hiding this comment

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

I made the design from user prototype (directly add class handler in TreeView level in xaml) but yes, with the new RoutedEvent update we can add with your second sample. Can I assume this PR will not be backported to 11.1?

Copy link
Member

Choose a reason for hiding this comment

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

Most likely, yes, it will go into 11.2, if merged this week.

/// </summary>
public class TreeViewItemExpansionChangedEventArgs: RoutedEventArgs
{
public TreeViewItem Source { get; }
Copy link
Member

Choose a reason for hiding this comment

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

This property can be removed for consistency and not conflict with object RoutedEventArgs.Source

Copy link
Member

Choose a reason for hiding this comment

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

Actually, with events separation, this custom event args are less useful now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm i forgot to delete it.Thanks.

…ix documentation. 4. Change routing to Bubble|Tunnel as both parent and children may care about this epxanding status.
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0051801-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 added this pull request to the merge queue Sep 12, 2024
Merged via the queue into AvaloniaUI:master with commit a8a8060 Sep 12, 2024
11 checks passed
@rabbitism rabbitism deleted the tree branch November 19, 2024 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants