-
Notifications
You must be signed in to change notification settings - Fork 391
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
Snapshot tracks only visible unresolved items #4980
Conversation
Allowing non-visible items to turn this flag 'true' can produce a "Dependencies" node with a warning icon but no children with a corresponding warning to drill into.
""Id"": ""tfm1\\yyy\\dependencyExisting"", | ||
""Name"":""dependencyExisting"", | ||
""Caption"":""DependencyExisting"", | ||
""HasUnresolvedDependency"":""false"", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change gets rid of HasUnresolvedDependency
, which isn't even on IDependency
. For this simple test type I prefer the strongly typed approach.
@@ -78,7 +78,7 @@ internal sealed class DependenciesTreeViewProvider : IDependenciesTreeViewProvid | |||
|
|||
dependenciesTree = CleanupOldNodes(dependenciesTree, currentTopLevelNodes); | |||
|
|||
ProjectImageMoniker rootIcon = _viewModelFactory.GetDependenciesRootIcon(snapshot.HasUnresolvedDependency).ToProjectSystemType(); | |||
ProjectImageMoniker rootIcon = _viewModelFactory.GetDependenciesRootIcon(snapshot.HasVisibleUnresolvedDependency).ToProjectSystemType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where the icon is determined at the root level. This PR means only visible and unresolved descendents of the root can cause the root to have a yellow triangle.
@@ -438,7 +438,7 @@ private void OnDependenciesSnapshotChanged(object sender, SnapshotChangedEventAr | |||
{ | |||
dependenciesNode = await viewProvider.BuildTreeAsync(dependenciesNode, snapshot, cancellationToken); | |||
|
|||
await _treeTelemetryService.ObserveTreeUpdateCompletedAsync(snapshot.HasUnresolvedDependency); | |||
await _treeTelemetryService.ObserveTreeUpdateCompletedAsync(snapshot.HasVisibleUnresolvedDependency); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this change will impact telemetry, in keeping with what's presented on screen.
@@ -183,9 +183,9 @@ void Add(AddDependencyContext context, IDependencyModel dependencyModel) | |||
string.Equals(id, dependency.Id), | |||
"dependenciesWorld dictionary entry keys must match their value's ids."); | |||
|
|||
if (!dependency.Resolved) | |||
if (!dependency.Resolved && dependency.Visible) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the additional constraint.
Allowing non-visible items to turn this flag 'true' can produce a "Dependencies" node with a warning icon, but no children with a corresponding warning to drill into.
Relates to #4362.