From ece3bfe68cd906a6ac418bb53c2213319cb8a758 Mon Sep 17 00:00:00 2001 From: Jonatan Gonzalez Date: Wed, 11 Dec 2024 12:00:32 -0800 Subject: [PATCH] Avoid recreating IMarkdownPreview control (#6153) * Avoid recreating markdown control * Remove MarkdownPreview depdency property * Pass IMarkdownPreview into the viewmodel instead of building it there * Add missing using statement * fix merge * Move to singleton * Remove creating instance in tab control * pr comment --- .../Utility/MarkdownPreviewSingleton.cs | 37 ++++++++++++++ .../ViewModels/PackageDetailsTabViewModel.cs | 4 +- .../ViewModels/ReadmePreviewViewModel.cs | 2 + .../Xamls/DetailControl.xaml.cs | 5 ++ .../Xamls/PackageDetailsTabControl.xaml.cs | 1 + .../Xamls/PackageManagerControl.xaml.cs | 1 + .../Xamls/PackageReadmeControl.xaml | 7 ++- .../Xamls/PackageReadmeControl.xaml.cs | 51 +++++++------------ 8 files changed, 68 insertions(+), 40 deletions(-) create mode 100644 src/NuGet.Clients/NuGet.PackageManagement.UI/Utility/MarkdownPreviewSingleton.cs diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Utility/MarkdownPreviewSingleton.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/Utility/MarkdownPreviewSingleton.cs new file mode 100644 index 00000000000..e1aff481d6e --- /dev/null +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Utility/MarkdownPreviewSingleton.cs @@ -0,0 +1,37 @@ +// 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. + +using Microsoft.VisualStudio.Markdown.Platform; + +namespace NuGet.PackageManagement.UI +{ + public class MarkdownPreviewSingleton + { +#pragma warning disable CS0618 // Type or member is obsolete + private static IMarkdownPreview Instance; +#pragma warning restore CS0618 // Type or member is obsolete + +#pragma warning disable CS0618 // Type or member is obsolete + public static IMarkdownPreview GetInstance() +#pragma warning restore CS0618 // Type or member is obsolete + { + if (Instance == null) + { +#pragma warning disable CS0618 // Type or member is obsolete + Instance = new PreviewBuilder().Build(); +#pragma warning restore CS0618 // Type or member is obsolete + } + + return Instance; + } + + public static void ResetInstance() + { + if (Instance != null) + { + Instance.Dispose(); + Instance = null; + } + } + } +} diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageDetailsTabViewModel.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageDetailsTabViewModel.cs index be490248dd6..22c357b7fad 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageDetailsTabViewModel.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageDetailsTabViewModel.cs @@ -38,8 +38,9 @@ public async Task InitializeAsync(DetailControlModel detailControlModel, INuGetP { var nuGetFeatureFlagService = await ServiceLocator.GetComponentModelServiceAsync(); _readmeTabEnabled = await nuGetFeatureFlagService.IsFeatureEnabledAsync(NuGetFeatureFlagConstants.RenderReadmeInPMUI); - +#pragma warning disable CS0618 // Type or member is obsolete ReadmePreviewViewModel = new ReadmePreviewViewModel(nugetPackageFileService, currentFilter, _readmeTabEnabled); +#pragma warning restore CS0618 // Type or member is obsolete DetailControlModel = detailControlModel; if (_readmeTabEnabled) @@ -76,7 +77,6 @@ public void Dispose() } _disposed = true; DetailControlModel.PropertyChanged -= DetailControlModel_PropertyChanged; - foreach (var tab in Tabs) { tab.PropertyChanged -= IsVisible_PropertyChanged; diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/ReadmePreviewViewModel.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/ReadmePreviewViewModel.cs index c28e59d21a2..d741bf803f6 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/ReadmePreviewViewModel.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/ReadmePreviewViewModel.cs @@ -20,7 +20,9 @@ public sealed class ReadmePreviewViewModel : TitledPageViewModelBase private bool _canRenderLocalReadme; private bool _isBusy; +#pragma warning disable CS0618 // Type or member is obsolete public ReadmePreviewViewModel(INuGetPackageFileService packageFileService, ItemFilter itemFilter, bool isReadmeFeatureEnabled) +#pragma warning restore CS0618 // Type or member is obsolete { _nugetPackageFileService = packageFileService ?? throw new ArgumentNullException(nameof(packageFileService)); _canRenderLocalReadme = CanRenderLocalReadme(itemFilter); diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/DetailControl.xaml.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/DetailControl.xaml.cs index 6174e273edd..1dd73856efe 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/DetailControl.xaml.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/DetailControl.xaml.cs @@ -104,6 +104,11 @@ public void Refresh() }).PostOnFailure(nameof(DetailControl)); } + public void Cleanup() + { + _packageDetailsTabControl.Dispose(); + } + private void ProjectInstallButtonClicked(object sender, EventArgs e) { var model = (PackageDetailControlModel)DataContext; diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageDetailsTabControl.xaml.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageDetailsTabControl.xaml.cs index 4bd3925898c..6364b9d4553 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageDetailsTabControl.xaml.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageDetailsTabControl.xaml.cs @@ -41,6 +41,7 @@ protected virtual void Dispose(bool disposing) if (disposing) { PackageDetailsTabViewModel.Dispose(); + MarkdownPreviewSingleton.ResetInstance(); } _disposed = true; } diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs index 5d71167d588..610c47c781a 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs @@ -1580,6 +1580,7 @@ private void CleanUp() _refreshCts?.Dispose(); _cancelSelectionChangedSource?.Dispose(); + _packageDetail.Cleanup(); _detailModel.Dispose(); _packageList.SelectionChanged -= PackageList_SelectionChanged; diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageReadmeControl.xaml b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageReadmeControl.xaml index bd9150b3b04..08ceea526c4 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageReadmeControl.xaml +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageReadmeControl.xaml @@ -28,14 +28,13 @@ + Visibility="{Binding Path=IsReadmeReady, Mode=OneWay, FallbackValue=Collapsed, Converter={StaticResource BooleanToVisibilityConverter}}" /> + Visibility="{Binding Path=IsBusy, Mode=OneWay, FallbackValue=Visible, Converter={StaticResource BooleanToVisibilityConverter}}" > @@ -75,6 +74,6 @@ + Visibility="{Binding Path=ErrorWithReadme, Mode=OneWay, FallbackValue=Collapsed, Converter={StaticResource BooleanToVisibilityConverter}}" /> diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageReadmeControl.xaml.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageReadmeControl.xaml.cs index 542af46e6d5..92ca7e7a280 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageReadmeControl.xaml.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageReadmeControl.xaml.cs @@ -7,6 +7,7 @@ using System.Windows; using System.Windows.Controls; using Microsoft.VisualStudio.Markdown.Platform; +using Microsoft.VisualStudio.Shell; using NuGet.PackageManagement.UI.ViewModels; using NuGet.VisualStudio; using NuGet.VisualStudio.Telemetry; @@ -16,19 +17,17 @@ namespace NuGet.PackageManagement.UI /// /// Interaction logic for PackageReadmeControl.xaml /// - public partial class PackageReadmeControl : UserControl, IDisposable + public partial class PackageReadmeControl : UserControl { #pragma warning disable CS0618 // Type or member is obsolete private IMarkdownPreview _markdownPreview; #pragma warning restore CS0618 // Type or member is obsolete - private bool _disposed = false; public PackageReadmeControl() { InitializeComponent(); -#pragma warning disable CS0618 // Type or member is obsolete - _markdownPreview = new PreviewBuilder().Build(); -#pragma warning restore CS0618 // Type or member is obsolete + _markdownPreview = MarkdownPreviewSingleton.GetInstance(); + descriptionMarkdownPreview.Content = _markdownPreview.VisualElement; } public ReadmePreviewViewModel ReadmeViewModel { get => (ReadmePreviewViewModel)DataContext; } @@ -41,36 +40,10 @@ private void ReadmeViewModel_PropertyChanged(object sender, PropertyChangedEvent } } - public void Dispose() - { - Dispose(disposing: true); - GC.SuppressFinalize(this); - } - - protected virtual void Dispose(bool disposing) - { - if (_disposed) - { - return; - } - - if (disposing) - { - _markdownPreview?.Dispose(); - if (ReadmeViewModel is not null) - { - ReadmeViewModel.PropertyChanged -= ReadmeViewModel_PropertyChanged; - } - } - - _disposed = true; - } - private async Task UpdateMarkdownAsync() { try { - descriptionMarkdownPreview.Content = descriptionMarkdownPreview.Content ?? _markdownPreview.VisualElement; if (!string.IsNullOrWhiteSpace(ReadmeViewModel.ReadmeMarkdown)) { await _markdownPreview.UpdateContentAsync(ReadmeViewModel.ReadmeMarkdown, ScrollHint.None); @@ -88,6 +61,10 @@ private void UserControl_DataContextChanged(object sender, DependencyPropertyCha { if (e.OldValue is ReadmePreviewViewModel oldMetadata) { + ThreadHelper.JoinableTaskFactory.Run(async () => + { + await _markdownPreview.UpdateContentAsync("", ScrollHint.None); + }); oldMetadata.PropertyChanged -= ReadmeViewModel_PropertyChanged; } if (ReadmeViewModel is not null) @@ -98,13 +75,19 @@ private void UserControl_DataContextChanged(object sender, DependencyPropertyCha private void PackageReadmeControl_Unloaded(object sender, RoutedEventArgs e) { - Dispose(); + if (ReadmeViewModel is not null) + { + ThreadHelper.JoinableTaskFactory.Run(async () => + { + await _markdownPreview.UpdateContentAsync("", ScrollHint.None); + }); + ReadmeViewModel.PropertyChanged -= ReadmeViewModel_PropertyChanged; + } } private void PackageReadmeControl_Loaded(object sender, RoutedEventArgs e) { - NuGetUIThreadHelper.JoinableTaskFactory.RunAsync(UpdateMarkdownAsync) - .PostOnFailure(nameof(PackageReadmeControl), nameof(PackageReadmeControl_Loaded)); + NuGetUIThreadHelper.JoinableTaskFactory.Run(UpdateMarkdownAsync); } } }