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

TreeView selection API spec #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kaiguo
Copy link

@kaiguo kaiguo commented Jun 7, 2019

No description provided.

@predavid-zz
Copy link

@MikeHillberg - once you review and approve, we can get this into v.next of WinUI.

* Add some more background for those not as familiar with the current TreeView
* Update the examples to be close to real-world usage.
* Reordered the sections to match the spec template
* Explain the SelectedItems behavior change in the Remarks section
// Set SelectedNode for single selection
treeView.SelectedNode = node;
// Get SelectedNode in single selection
TreeViewNode selectedNode = treeView.SelectedNode;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the value of SelectedNode if SelectedNodes was used to select multiple nodes?

Copy link
Author

@kaiguo kaiguo Aug 5, 2019

Choose a reason for hiding this comment

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

What is the value of SelectedNode if SelectedNodes was used to select multiple nodes?

Currently it returns the first node in SelectedNodes list (maybe it should return the newly selected node which is the last one in list?).

winrt::TreeViewNode TreeView::SelectedNode()
{
    auto nodes = SelectedNodes();
    return nodes.Size() > 0 ? nodes.GetAt(0) : nullptr;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

How does ListView behave?

Copy link
Author

Choose a reason for hiding this comment

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

ListView returns first item in list too so they are consistent.


- Add `SelectedNode`: get/set the selected TreeViewNode for single selection
- Add `SelectedItem`: get/set the selected item for single selection
- Add `SelectedItems`: get the SelectedItems collection.

Choose a reason for hiding this comment

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

What is the reason that SelectedItems cannot be set via binding? As SelectedItem can be set by two-way binding, it seems to make sense to me to give the same functionality to SelectedItems as well.

<TreeView ItemsSource="{x:Bind Descendants}" SelectedItems="{x:Bind CurrentDescendants, Mode=TwoWay}" />

Copy link
Contributor

Choose a reason for hiding this comment

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

This matches ListView.SelectedItems, but I realize that doesn't answer the question. ListView also supports source collections that implement ISelectionInfo, which is a way for the source to have full control over the selection, but it's pretty advanced.

Choose a reason for hiding this comment

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

@kaiguo @MikeHillberg Yeah, I'd say selecting a group of items should be as simple as possible. Right now, looking at the TreeView Selection API, I need to interact with the specific TreeView object to select multiple items. Hence, if I just wanted to mark a group of items in a TreeView as selected, I can't easily do that without somehow referencing an object of the View (as in MVVM). That seems to me like a pretty big oversight and an unnecessary hurdle for the developer.

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.

5 participants