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

Make TreeView SelectedItem a DP #2523

Merged
merged 4 commits into from
May 28, 2020

Conversation

marcelwgn
Copy link
Collaborator

Description

Makes the SelectedItem a DP and adds interaction test to verify two way binding.

Motivation and Context

Closes #2458

How Has This Been Tested?

Add new interaction test

Screenshots (if appropriate):

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label May 21, 2020
@marcelwgn marcelwgn changed the title Make SelectedItem a DP Make TreeView SelectedItem a DP May 21, 2020
@ranjeshj ranjeshj added area-TreeView team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels May 27, 2020
@@ -292,6 +292,29 @@ void TreeView::OnListControlSelectionChanged(const winrt::IInspectable& sender,
if (SelectionMode() == winrt::TreeViewSelectionMode::Single)
{
RaiseSelectionChanged(args.AddedItems(), args.RemovedItems());

const auto newSelectedItem = [this,args]() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this [](start = 38, length = 4)

Is the this capture needed here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No it wasn't needed, removed it.

// Checking if new value is different to the currently internally selected item
if (args.NewValue() != selected)
{
if (auto listControl = ListControl())
Copy link
Contributor

Choose a reason for hiding this comment

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

auto [](start = 16, length = 4)

nit: const

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

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

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-TreeView team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Make TreeView.SelectedItem a DependencyProperty
4 participants