Fix vb event generation in navbars#53876
Conversation
| var workspace = document.Project.Solution.Workspace; | ||
| var navigationService = VSTypeScriptDocumentNavigationServiceWrapper.Create(workspace); | ||
| navigationService.TryNavigateToPosition(workspace, document.Id, item.Spans[0].Start, virtualSpace: 0, options: null, cancellationToken: cancellationToken); | ||
| return true; |
There was a problem hiding this comment.
note: i'm preserving behavior for TS and F#. Neither of which have any sort of special 'type list item is selected, so update the member list' behavior like VB has.
| // list. So selecting such an item should only update the member list, and we do not want a refresh | ||
| // to wipe that out. | ||
| if (!await navBarService.TryNavigateToItemAsync(document, item, view, cancellationToken).ConfigureAwait(true)) | ||
| return; |
There was a problem hiding this comment.
this regressed in: https://github.com/dotnet/roslyn/pull/52530/files#diff-902dbb57c78c6bbd71f7409a96a3db1dbd4813fb5658a069ef3b3b33da063a8aL305
THe reason for this is that the old code effectively would say: ok, something was selected. However the semantic version of the solution is the same, so i won't refresh anything.
The approach here is cleaner. If trying to navigate failed, there's no need for us to have our controller change anything, and we'll instead just let the view update accordingly.
| Return True | ||
| End If | ||
|
|
||
| Return False |
There was a problem hiding this comment.
this is the critical part of the change. We didn't navigate or generate, so this means the user effectively only picked the item just to get the refreshed navbars.
| // ConfigureAwait(true) as we have to come back to UI thread in order to kick of the refresh task | ||
| // below. Note that we only want to refresh if selecting the item had an effect (either navigating | ||
| // or generating). If nothing happened to don't want to refresh. This is important as some items | ||
| // exist in the type list that are only there to show a set a particular set of items in the member |
There was a problem hiding this comment.
| // exist in the type list that are only there to show a set a particular set of items in the member | |
| // exist in the type list that are only there to show a particular set of items in the member |
There was a problem hiding this comment.
WIll do in next PR :)
We need integration tests for this. That is tracked by: #53875