Skip to content

Add new navigation pattern for Value Tracking#54163

Merged
ryzngard merged 7 commits intodotnet:mainfrom
ryzngard:feature/value_tracking_tree_navigation
Jun 25, 2021
Merged

Add new navigation pattern for Value Tracking#54163
ryzngard merged 7 commits intodotnet:mainfrom
ryzngard:feature/value_tracking_tree_navigation

Conversation

@ryzngard
Copy link
Contributor

@ryzngard ryzngard commented Jun 17, 2021

Value tracking originally used the standard WPF navigation pattern. This would also navigate to the source line based on selection.

This improves the design by manually handling keyboard navigation for most cases. By default, if an item is selected it no longer navigates to source. There are conditions where this is different:

Shortcut Expand Behavior Navigates To Source Previous or Next
Arrow Down False True Next
Arrow Up False True Next
Arrow Left Collapse Only False Neither
Arrow Right Expand Only False Neither
Space Toggle False Neither
Enter False True Neither
F8 Expand Only True Next
Shift + F8 False True Previous
Click False True Neither
Double Click False True Neither

Resolves #53821

@ryzngard ryzngard requested a review from a team as a code owner June 17, 2021 04:20
@ghost ghost added the Area-IDE label Jun 17, 2021
item.Parent = parent;
}
}
};
Copy link
Member

Choose a reason for hiding this comment

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

Call me old, but a 20 line lambda with a local function should probably just be a method 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe... you don't always know when you start writing it :P

{
if (sender is TreeViewItemBase viewModel)
{
SelectItem(viewModel, true);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this also expand, based on your table?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I think the table is wrong here. Imo clicking on an item shouldn't expand at all. Open to feedback on that behavior though

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 table to reflect this behavior as written

Copy link
Member

Choose a reason for hiding this comment

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

As the person who made double click on a project file open the editor, and not expand/collapse, I get the logic of it and support you, but you should probably expect feedback too :)

Copy link
Member

Choose a reason for hiding this comment

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

I believe double-clicking in Test Explorer would do an expansion. And I just know solution explorer does a different thing😮

Co-authored-by: David Wengier <david.wengier@microsoft.com>
@mikadumont
Copy link
Contributor

mikadumont commented Jun 23, 2021

I personally do think arrow up, arrow down, and single click should navigate to source. We should keep behavior consistent with other results windows like Find All References and shouldn't add keyboard friction which can cause the experience to be less seamless for users. Can we revert those options back to true and monitor feedback once we release it in preview 2?

@ryzngard
Copy link
Contributor Author

I've changed the table to match Mika's request and updated the code. If we get more user feedback on wanting this to be different we will reevaluate.

@ryzngard ryzngard merged commit ee996f2 into dotnet:main Jun 25, 2021
@ghost ghost added this to the Next milestone Jun 25, 2021
@RikkiGibson RikkiGibson modified the milestones: Next, 17.0.P2 Jun 29, 2021
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.

Value Tracking should have a toggle button to disable single click navigation

5 participants