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

[Feature] Render package README from local storage in the PM UI #6110

Merged
merged 15 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
<PackageVersion Include="Microsoft.Test.Apex.VisualStudio" Version="17.11.35005.70" />
<PackageVersion Include="Microsoft.TestPlatform.Portable" Version="17.1.0" />
<PackageVersion Include="Microsoft.VisualStudio.LanguageServices" Version="4.3.1" />
<PackageVersion Include="Microsoft.VisualStudio.Markdown.Platform" Version="17.11.119-preview" />
<PackageVersion Include="Microsoft.VisualStudio.ProjectSystem" Version="17.4.221-pre" />
<PackageVersion Include="Microsoft.VisualStudio.ProjectSystem.Managed" Version="17.2.0-beta1-20502-01" />
<PackageVersion Include="Microsoft.VisualStudio.ProjectSystem.Managed.VS" Version="17.2.0-beta1-20502-01" />
Expand Down
17 changes: 9 additions & 8 deletions NuGet.Config
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
</auditSources>
<packageSourceMapping>
<clear />
<packageSource key = "dotnet-public">
<packageSource key="dotnet-public">
<package pattern="Azure.Core " />
<package pattern="ben.demystifier" />
<package pattern="castle.core" />
<package pattern="fluentassertions" />
<package pattern="Humanizer.Core" />
<package pattern="ilmerge" />
<package pattern="lucene.net" />
<package pattern="Markdig.Signed" />
<package pattern="messagepack" />
<package pattern="messagepack.annotations" />
<package pattern="messagepackanalyzer" />
Expand All @@ -45,14 +46,14 @@
<package pattern="xunit.*" />
<package pattern="xunitxml.testlogger" />
</packageSource>
<packageSource key = "dotnet-eng">
<packageSource key="dotnet-eng">
<package pattern="Microsoft.DotNet.*" />
</packageSource>
<packageSource key = "nuget-build">
<packageSource key="nuget-build">
<package pattern="nuget.client.endtoend.testdata" />
<package pattern="nugetvalidator" />
</packageSource>
<packageSource key = "vside">
<packageSource key="vside">
<package pattern="envdte" />
<package pattern="envdte100" />
<package pattern="envdte80" />
Expand All @@ -65,7 +66,7 @@
<package pattern="Microsoft.ServiceHub.Framework" />
<package pattern="Microsoft.TeamFoundationServer.ExtendedClient" />
<package pattern="microsoft.test.apex.visualstudio" />
<package pattern="microsoft.visualstudio.*" />
<package pattern="Microsoft.VisualStudio.*" />
<package pattern="Microsoft.VisualStudioEng.MicroBuild.Core" />
<package pattern="Microsoft.VSSDK.BuildTools" />
<package pattern="stdole" />
Expand All @@ -83,13 +84,13 @@
<package pattern="vslangproj90" />
<package pattern="vswebsite.interop" />
</packageSource>
<packageSource key = "myget-legacy@Local">
<packageSource key="myget-legacy@Local">
<package pattern="Microsoft.Extensions.CommandLineUtils.Sources" />
</packageSource>
<packageSource key = "VS">
<packageSource key="VS">
<package pattern="Microsoft.DevDiv.Validation.TestPlatform.Settings.Tasks" />
</packageSource>
<packageSource key = "dotnet-libraries">
<packageSource key="dotnet-libraries">
<package pattern="System.CommandLine" />
</packageSource>
</packageSourceMapping>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
[assembly: SuppressMessage("Build", "CA1501:'PackageManagerControl' has an object hierarchy '9' levels deep within the defining module. If possible, eliminate base classes within the hierarchy to decrease its hierarchy level below '6': 'UserControl, ContentControl, Control, FrameworkElement, UIElement, Visual, DependencyObject, DispatcherObject, Object'", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.PackageManagement.UI.PackageManagerControl")]
[assembly: SuppressMessage("Build", "CA1501:'PackageItemDeprecationLabel' has an object hierarchy '9' levels deep within the defining module. If possible, eliminate base classes within the hierarchy to decrease its hierarchy level below '6': 'UserControl, ContentControl, Control, FrameworkElement, UIElement, Visual, DependencyObject, DispatcherObject, Object'", Justification = "Default WPF Hierarchy", Scope = "type", Target = "~T:NuGet.PackageManagement.UI.PackageItemDeprecationLabel")]
[assembly: SuppressMessage("Build", "CA1501:'PackageManagerTopPanel' has an object hierarchy '9' levels deep within the defining module. If possible, eliminate base classes within the hierarchy to decrease its hierarchy level below '6': 'UserControl, ContentControl, Control, FrameworkElement, UIElement, Visual, DependencyObject, DispatcherObject, Object'", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.PackageManagement.UI.PackageManagerTopPanel")]
[assembly: SuppressMessage("Build", "CA1501:'PackageMetadataControl' has an object hierarchy '9' levels deep within the defining module. If possible, eliminate base classes within the hierarchy to decrease its hierarchy level below '6': 'UserControl, ContentControl, Control, FrameworkElement, UIElement, Visual, DependencyObject, DispatcherObject, Object'", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.PackageManagement.UI.PackageMetadataControl")]
[assembly: SuppressMessage("Build", "CA1501:'PackageMetadataControl' has an object hierarchy '9' levels deep within the defining module. If possible, eliminate base classes within the hierarchy to decrease its hierarchy level below '6': 'UserControl, ContentControl, Control, FrameworkElement, UIElement, Visual, DependencyObject, DispatcherObject, Object'", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.PackageManagement.UI.PackageDetailsTabControl")]
[assembly: SuppressMessage("Build", "CA1501:'PackageRestoreBar' has an object hierarchy '9' levels deep within the defining module. If possible, eliminate base classes within the hierarchy to decrease its hierarchy level below '6': 'UserControl, ContentControl, Control, FrameworkElement, UIElement, Visual, DependencyObject, DispatcherObject, Object'", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.PackageManagement.UI.PackageRestoreBar")]
[assembly: SuppressMessage("Build", "CA1501:'PreviewWindow' has an object hierarchy '11' levels deep within the defining module. If possible, eliminate base classes within the hierarchy to decrease its hierarchy level below '6': 'DialogWindow, DialogWindowBase, Window, ContentControl, Control, FrameworkElement, UIElement, Visual, DependencyObject, DispatcherObject, Object'", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.PackageManagement.UI.PreviewWindow")]
[assembly: SuppressMessage("Build", "CA1501:'PRMigratorBar' has an object hierarchy '9' levels deep within the defining module. If possible, eliminate base classes within the hierarchy to decrease its hierarchy level below '6': 'UserControl, ContentControl, Control, FrameworkElement, UIElement, Visual, DependencyObject, DispatcherObject, Object'", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.PackageManagement.UI.PRMigratorBar")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace NuGet.PackageManagement.UI
/// The base class of PackageDetailControlModel and PackageSolutionDetailControlModel.
/// When user selects an action, this triggers version list update.
/// </summary>
public abstract class DetailControlModel : INotifyPropertyChanged, IDisposable
public abstract class DetailControlModel : TitledPageViewModelBase, IDisposable
{
private static readonly string StarAll = VersionRangeFormatter.Instance.Format("p", VersionRange.Parse("*"), VersionRangeFormatter.Instance);
private static readonly string StarAllFloating = VersionRangeFormatter.Instance.Format("p", VersionRange.Parse("*-*"), VersionRangeFormatter.Instance);
Expand Down Expand Up @@ -70,6 +70,9 @@ protected DetailControlModel(
_options.SelectedChanged += DependencyBehavior_SelectedChanged;

_versions = new ItemsChangeObservableCollection<DisplayVersion>();

Title = Resources.Label_PackageDetails;
IsVisible = true;
}

/// <summary>
Expand Down Expand Up @@ -385,11 +388,9 @@ private async Task<IReadOnlyList<PackageDependency>> GetDependencies(IProjectCon
// Called after package install/uninstall.
public abstract Task RefreshAsync(CancellationToken cancellationToken);

public event PropertyChangedEventHandler PropertyChanged;

protected void OnPropertyChanged(string propertyName)
{
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
RaisePropertyChanged(propertyName);
}

public string Id => _searchResultPackage?.Id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public DetailedPackageMetadata(PackageSearchMetadataContextInfo serverData, Pack
IconUrl = serverData.IconUrl;
LicenseUrl = serverData.LicenseUrl;
ProjectUrl = serverData.ProjectUrl;
ReadmeFileUrl = serverData.ReadmeFileUrl;
ReadmeUrl = serverData.ReadmeUrl;
ReportAbuseUrl = serverData.ReportAbuseUrl;
// Some server implementations send down an array with an empty string, which ends up as an empty string.
Expand Down Expand Up @@ -95,6 +96,8 @@ public DetailedPackageMetadata(PackageSearchMetadataContextInfo serverData, Pack

public Uri? ProjectUrl { get; set; }

public string? ReadmeFileUrl { get; set; }

public Uri? ReadmeUrl { get; set; }

public Uri? ReportAbuseUrl { get; set; }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace NuGet.PackageManagement.UI
{
public enum PackageMetadataTab
{
Readme,
PackageDetails,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Build.Framework" />
<PackageReference Include="Microsoft.NET.StringTools" />
<PackageReference Include="Microsoft.VisualStudio.Markdown.Platform" />
<PackageReference Include="Microsoft.VisualStudio.Sdk" />
</ItemGroup>

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions src/NuGet.Clients/NuGet.PackageManagement.UI/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1076,4 +1076,23 @@ Please see https://aka.ms/troubleshoot_nuget_cache for more help.</value>
<data name="ColumnHeader_PackageLevel" xml:space="preserve">
<value>Package Level</value>
</data>
<data name="Label_PackageDetails" xml:space="preserve">
<value>Package Details</value>
</data>
<data name="Error_UnableToLoadReadme" xml:space="preserve">
<value>An error occurred while trying to load the README.</value>
</data>
<data name="Label_Readme_Tab" xml:space="preserve">
<value>README</value>
</data>
<data name="Text_NoReadme" xml:space="preserve">
<value>Could not read package README from selected package.

Only the package maintainer can add a README.

If you are not the maintainer, please consider filing an issue or contacting the maintainer to request a README.

For instructions on how to add a README, please visit [aka.ms/nuget/readme](https://aka.ms/nuget/readme)</value>
<comment>{Locked="[aka.ms/nuget/readme](https://aka.ms/nuget/readme)"} "[aka.ms/nuget/readme](https://aka.ms/nuget/readme)" this is a URL link and should not be translated</comment>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public UserSettings()
SelectedFilter = ItemFilter.Installed;
DependencyBehavior = DependencyBehavior.Lowest;
FileConflictAction = FileConflictAction.PromptUser;
SelectedPackageMetadataTab = PackageMetadataTab.Readme;
OptionsExpanded = false;
}

Expand All @@ -40,6 +41,8 @@ public UserSettings()

public ItemFilter SelectedFilter { get; set; }

public PackageMetadataTab SelectedPackageMetadataTab { get; set; }

public DependencyBehavior DependencyBehavior { get; set; }

public FileConflictAction FileConflictAction { get; set; }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
using System;
using System.Collections.ObjectModel;
using System.ComponentModel;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.VisualStudio.Shell;
using NuGet.VisualStudio;
using NuGet.VisualStudio.Internal.Contracts;

namespace NuGet.PackageManagement.UI.ViewModels
{
public class PackageDetailsTabViewModel : ViewModelBase, IDisposable
{
private bool _disposed = false;
private bool _readmeTabEnabled;

public ReadmePreviewViewModel ReadmePreviewViewModel { get; private set; }

public DetailControlModel DetailControlModel { get; private set; }
jgonz120 marked this conversation as resolved.
Show resolved Hide resolved

public ObservableCollection<TitledPageViewModelBase> Tabs { get; private set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe last week in your feature branch PR I suggested making this an IReadOnlyList. Can you help me understand why this collection needs to be dynamic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry, if I make the tab a ReadonlyList then I cannot make changes to the items in the tabs, they need to be instantiated in the constructor. Right now, I'm instantiating the VM in the constructor of the view, so I don't have all the components necessary to instantiate the VM. I think once we refactor the DetailControlModel it can handle creating the VM.


private TitledPageViewModelBase _selectedTab;
public TitledPageViewModelBase SelectedTab
{
get => _selectedTab;
set
{
SetAndRaisePropertyChanged(ref _selectedTab, value);
}
jgonz120 marked this conversation as resolved.
Show resolved Hide resolved
}

public PackageDetailsTabViewModel()
{
_readmeTabEnabled = true;
Tabs = new ObservableCollection<TitledPageViewModelBase>();
}

public async Task InitializeAsync(DetailControlModel detailControlModel, INuGetPackageFileService nugetPackageFileService, ItemFilter currentFilter, PackageMetadataTab initialSelectedTab)
{
var nuGetFeatureFlagService = await ServiceLocator.GetComponentModelServiceAsync<INuGetFeatureFlagService>();
_readmeTabEnabled = await nuGetFeatureFlagService.IsFeatureEnabledAsync(NuGetFeatureFlagConstants.RenderReadmeInPMUI);

ReadmePreviewViewModel = new ReadmePreviewViewModel(nugetPackageFileService, currentFilter, _readmeTabEnabled);
DetailControlModel = detailControlModel;

Tabs.Add(ReadmePreviewViewModel);
Tabs.Add(DetailControlModel);

SelectedTab = Tabs.FirstOrDefault(t => t.IsVisible && ConvertFromTabType(t) == initialSelectedTab) ?? Tabs.FirstOrDefault(t => t.IsVisible);

DetailControlModel.PropertyChanged += DetailControlModel_PropertyChanged;

foreach (var tab in Tabs)
{
tab.PropertyChanged += IsVisible_PropertyChanged;
}
}

public static PackageMetadataTab ConvertFromTabType(TitledPageViewModelBase vm)
{
if (vm is DetailControlModel)
{
return PackageMetadataTab.PackageDetails;
}
return PackageMetadataTab.Readme;
}

public async Task SetCurrentFilterAsync(ItemFilter filter)
{
if (_readmeTabEnabled)
{
await ReadmePreviewViewModel.SetCurrentFilterAsync(filter);
}
}

public void Dispose()
{
if (_disposed)
{
return;
}
_disposed = true;
DetailControlModel.PropertyChanged -= DetailControlModel_PropertyChanged;

foreach (var tab in Tabs)
{
tab.PropertyChanged -= IsVisible_PropertyChanged;
}
}

private void IsVisible_PropertyChanged(object sender, PropertyChangedEventArgs e)
{
if (e.PropertyName == nameof(TitledPageViewModelBase.IsVisible))
{
if (SelectedTab == null || (SelectedTab == sender && !SelectedTab.IsVisible))
{
SelectedTab = Tabs.FirstOrDefault(t => t.IsVisible);
}
}
}

private void DetailControlModel_PropertyChanged(object sender, PropertyChangedEventArgs e)
{
ThreadHelper.JoinableTaskFactory.Run(async () =>
{
if (_readmeTabEnabled)
{
await ReadmePreviewViewModel.SetPackageMetadataAsync(DetailControlModel.PackageMetadata, CancellationToken.None);
}
});
}
}
}
Loading
Loading