From c1acfb3aab69e62c5131f82f4758b77ff2fc029b Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Wed, 23 Oct 2019 19:09:25 +0100 Subject: [PATCH 01/12] [NuGet] Show license metadata in Manage Packages dialog If the NuGet package source has license metadata then show this information instead of the View License link. License metadata can either be an license expression or a file. If the license is a file then clicking View License will open a separate dialog with the license read from the file. Fixes VSTS #1005394 - Support the new "license" properties in the NuGet package manager UI --- .../LicenseFileDialog.cs | 81 +++++++++ .../ManagePackagesDialog.UI.cs | 21 ++- .../ManagePackagesDialog.cs | 172 +++++++++++++++++- .../MonoDevelop.PackageManagement.csproj | 7 + .../ManagePackagesSearchResultViewModel.cs | 26 +++ .../DetailedPackageMetadata.cs | 22 +++ .../Models/FreeText.cs | 15 ++ .../Models/IText.cs | 10 + .../Models/LicenseFileText.cs | 78 ++++++++ .../Models/LicenseText.cs | 19 ++ .../Models/WarningText.cs | 15 ++ .../PackageLicenseUtilities.cs | 134 ++++++++++++++ 12 files changed, 594 insertions(+), 6 deletions(-) create mode 100644 main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs create mode 100644 main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/FreeText.cs create mode 100644 main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/IText.cs create mode 100644 main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/LicenseFileText.cs create mode 100644 main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/LicenseText.cs create mode 100644 main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/WarningText.cs create mode 100644 main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/PackageLicenseUtilities.cs diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs new file mode 100644 index 00000000000..c84bcad96ba --- /dev/null +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs @@ -0,0 +1,81 @@ +// +// LicenseFileDialog.cs +// +// Author: +// Matt Ward +// +// Copyright (c) 2019 Microsoft Corporation +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +using System.ComponentModel; +using NuGet.PackageManagement.UI; +using Xwt; +using Xwt.Formats; + +namespace MonoDevelop.PackageManagement.Gui +{ + class LicenseFileDialog : Dialog + { + RichTextView textView; + LicenseFileText licenseFileText; + + public LicenseFileDialog (LicenseFileText licenseFileText) + { + this.licenseFileText = licenseFileText; + + Build (); + LoadText (); + + licenseFileText.PropertyChanged += LicenseFileTextPropertyChanged; + licenseFileText.LoadLicenseFile (); + } + + void Build () + { + Height = 450; + Width = 450; + Title = licenseFileText.LicenseHeader; + + textView = new RichTextView (); + Content = textView; + + Buttons.Add (Command.Ok); + DefaultCommand = Command.Ok; + } + + void LicenseFileTextPropertyChanged (object sender, PropertyChangedEventArgs e) + { + LoadText (); + } + + void LoadText () + { + textView.LoadText (licenseFileText.LicenseText, TextFormat.Plain); + } + + protected override void Dispose (bool disposing) + { + base.Dispose (disposing); + if (disposing) { + licenseFileText.PropertyChanged -= LicenseFileTextPropertyChanged; + } + } + } +} diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.UI.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.UI.cs index 5a23a14c4d9..a6ab18375c8 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.UI.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.UI.cs @@ -47,7 +47,11 @@ internal partial class ManagePackagesDialog : ExtendedTitleBarDialog Label packageAuthor; Label packagePublishedDate; Label packageDownloads; + Label packageLicenseLabel; LinkLabel packageLicenseLink; + VBox packageLicenseMetadataWarningsVBox; + HBox packageLicenseMetadataHBox; + Label packageLicenseMetadataLabel; LinkLabel packageProjectPageLink; Label packageDependenciesList; HBox packageDependenciesHBox; @@ -346,7 +350,7 @@ void Build () var packageLicenseHBox = new HBox (); packageInfoVBox.PackStart (packageLicenseHBox); - var packageLicenseLabel = new Label (); + packageLicenseLabel = new Label (); packageLicenseLabel.Text = GettextCatalog.GetString ("License"); packageLicenseLabel.Font = packageInfoBoldFont; packageLicenseHBox.PackStart (packageLicenseLabel); @@ -356,6 +360,21 @@ void Build () packageLicenseLink.Font = packageInfoSmallFont; packageLicenseHBox.PackEnd (packageLicenseLink); + packageLicenseMetadataWarningsVBox = new VBox (); + packageLicenseMetadataWarningsVBox.Visible = false; + packageInfoVBox.PackStart (packageLicenseMetadataWarningsVBox); + + packageLicenseMetadataHBox = new HBox (); + packageLicenseMetadataHBox.Visible = false; + packageInfoVBox.PackStart (packageLicenseMetadataHBox); + + packageLicenseMetadataLabel = new Label (); + packageLicenseMetadataLabel.Wrap = WrapMode.Word; + packageLicenseMetadataLabel.MarginLeft = 5; + packageLicenseMetadataLabel.Font = packageInfoSmallFont; + packageLicenseMetadataLabel.Accessible.LabelWidget = packageLicenseLabel; + packageLicenseMetadataHBox.PackStart (packageLicenseMetadataLabel, true); + // Package project page. var packageProjectPageHBox = new HBox (); packageInfoVBox.PackStart (packageProjectPageHBox); diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs index b5b1fac718b..958f2af2db9 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs @@ -27,10 +27,13 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Security; using MonoDevelop.Components.AtkCocoaHelper; using MonoDevelop.Core; using MonoDevelop.Ide; +using MonoDevelop.PackageManagement.Gui; using MonoDevelop.Projects; +using NuGet.PackageManagement.UI; using NuGet.Versioning; using Xwt; using Xwt.Drawing; @@ -98,6 +101,8 @@ public ManagePackagesDialog ( LoadViewModel (initialSearch); closeButton.Clicked += CloseButtonClicked; + packageLicenseLink.NavigateToUrl += PackageLicenseNavigateToUrl; + packageLicenseMetadataLabel.LinkClicked += PackageLicenseLinkClicked; showPrereleaseCheckBox.Clicked += ShowPrereleaseCheckBoxClicked; packageSourceComboBox.SelectionChanged += PackageSourceChanged; addPackagesButton.Clicked += AddPackagesButtonClicked; @@ -147,6 +152,8 @@ void UpdateTabAccessibility () protected override void Dispose (bool disposing) { closeButton.Clicked -= CloseButtonClicked; + packageLicenseLink.NavigateToUrl -= PackageLicenseNavigateToUrl; + packageLicenseMetadataLabel.LinkClicked -= PackageLicenseLinkClicked; currentPackageVersionLabel.BoundsChanged -= PackageVersionLabelBoundsChanged; showPrereleaseCheckBox.Clicked -= ShowPrereleaseCheckBoxClicked; @@ -477,6 +484,10 @@ void ShowPackageInformation (ManagePackagesSearchResultViewModel packageViewMode { bool consolidate = viewModel.IsConsolidatePageSelected; + foreach (Widget child in packageInfoVBox.Children) { + child.Visible = !consolidate; + } + if (consolidate) { projectsListViewLabel.Text = GettextCatalog.GetString ("Select projects and a version for a consolidation."); } else { @@ -491,7 +502,7 @@ void ShowPackageInformation (ManagePackagesSearchResultViewModel packageViewMode this.packageId.Visible = packageViewModel.HasNoGalleryUrl; ShowUri (this.packageIdLink, packageViewModel.GalleryUrl, packageViewModel.Id); ShowUri (this.packageProjectPageLink, packageViewModel.ProjectUrl); - ShowUri (this.packageLicenseLink, packageViewModel.LicenseUrl); + ShowLicense (packageViewModel); PopulatePackageDependencies (packageViewModel); } @@ -511,10 +522,6 @@ void ShowPackageInformation (ManagePackagesSearchResultViewModel packageViewMode packageVersionsHBox.Visible = true; } - foreach (Widget child in packageInfoVBox.Children) { - child.Visible = !consolidate; - } - if (consolidate) { PopulateProjectList (); } else { @@ -1042,6 +1049,7 @@ void SelectedPackageViewModelChanged (object sender, PropertyChangedEventArgs e) } else { if (!viewModel.IsConsolidatePageSelected) { packagePublishedDate.Text = viewModel.SelectedPackage.GetLastPublishedDisplayText (); + ShowLicense (viewModel.SelectedPackage); PopulatePackageDependencies (viewModel.SelectedPackage); } } @@ -1284,5 +1292,159 @@ void ProjectCheckBoxCellViewToggled (object sender, WidgetEventArgs e) UpdateAddPackagesButton (); } + + void ShowLicense (ManagePackagesSearchResultViewModel packageViewModel) + { + if (packageViewModel.HasLicenseMetadata) { + ShowLicenseMetadata (packageViewModel); + } else { + packageLicenseMetadataHBox.Visible = false; + packageLicenseMetadataWarningsVBox.Visible = false; + ShowUri (packageLicenseLink, packageViewModel.LicenseUrl, GettextCatalog.GetString ("View License")); + } + } + + static string GetUriMarkup (Uri uri, string text) + { + return string.Format ( + "{1}", + uri != null ? SecurityElement.Escape (uri.ToString ()) : string.Empty, + SecurityElement.Escape (text)); + } + + void PackageLicenseNavigateToUrl (object sender, NavigateToUrlEventArgs e) + { + if (ShowLicenseFile (e.Uri)) { + e.SetHandled (); + } + } + + bool ShowLicenseFile (Uri uri) + { + if (!uri.IsFile) + return false; + + if (uri.Fragment?.Length > 0) { + if (int.TryParse (uri.Fragment.Substring (1), out int fileNumber)) { + ShowLicenseFile (fileNumber); + return true; + } + } + return false; + } + + void PackageLicenseLinkClicked (object sender, LinkEventArgs e) + { + if (ShowLicenseFile (e.Target)) { + e.SetHandled (); + } + } + + void ShowLicenseMetadata (ManagePackagesSearchResultViewModel packageViewModel) + { + List warnings = null; + var textLinks = packageViewModel.GetLicenseLinks (); + + if (textLinks.Count == 0) { + packageLicenseLink.Visible = false; + packageLicenseMetadataHBox.Visible = false; + packageLicenseMetadataWarningsVBox.Visible = false; + return; + } + + // Single link - show this on the same line as the License label. + if (textLinks.Count == 1) { + packageLicenseLink.Visible = true; + packageLicenseMetadataHBox.Visible = false; + packageLicenseMetadataWarningsVBox.Visible = false; + + IText textLink = textLinks [0]; + if (textLink is LicenseText licenseText) { + packageLicenseLink.Text = licenseText.Text; + packageLicenseLink.Uri = licenseText.Link; + return; + } else if (textLink is LicenseFileText licenseFileText) { + packageLicenseLink.Text = GettextCatalog.GetString ("View License"); + packageLicenseLink.Uri = CreateLicenseFileUri (licenseFileText, 1); + return; + } else { + // Warning or plain text - handled below. + } + } + + // Multiple text links. We need to allow these to wrap so show these below the license label. + var markupBuilder = StringBuilderCache.Allocate (); + + int fileLicenseCount = 0; // Should be one but handle multiple. + foreach (IText textLink in textLinks) { + if (textLink is LicenseText licenseText) { + markupBuilder.Append (GetUriMarkup (licenseText.Link, licenseText.Text)); + } else if (textLink is LicenseFileText licenseFileText) { + fileLicenseCount++; + markupBuilder.Append (GetUriMarkup (CreateLicenseFileUri (licenseFileText, fileLicenseCount), licenseFileText.Text)); + } else if (textLink is WarningText warning) { + warnings ??= new List (); + warnings.Add (warning); + } else { + markupBuilder.Append (textLink.Text); + } + } + + packageLicenseLink.Visible = false; + packageLicenseMetadataHBox.Visible = true; + packageLicenseMetadataLabel.Markup = StringBuilderCache.ReturnAndFree (markupBuilder); + + if (warnings != null) { + AddWarnings (warnings); + } else { + packageLicenseMetadataWarningsVBox.Visible = false; + } + } + + Uri CreateLicenseFileUri (LicenseFileText licenseFileText, int licenseCount) + { + return new Uri ($"file:///{licenseFileText.Text}#{licenseCount}"); + } + + void AddWarnings (List warnings) + { + foreach (Widget child in packageLicenseMetadataWarningsVBox.Children.ToArray ()) { + packageLicenseMetadataWarningsVBox.Remove (child); + child.Dispose (); + } + + foreach (WarningText warning in warnings) { + var hbox = new HBox (); + var image = new ImageView { + Image = ImageService.GetIcon ("md-warning", Gtk.IconSize.Menu), + MarginLeft = packageLicenseMetadataLabel.MarginLeft, + VerticalPlacement = WidgetPlacement.Start, + }; + image.Accessible.RoleDescription = GettextCatalog.GetString ("Warning Icon"); + hbox.PackStart (image); + + var label = new Label { + Text = warning.Text, + Font = packageLicenseMetadataLabel.Font, + Wrap = WrapMode.Word + }; + image.Accessible.LabelWidget = label; + label.Accessible.LabelWidget = packageLicenseLabel; + hbox.PackStart (label, true, true); + + packageLicenseMetadataWarningsVBox.PackStart (hbox); + } + + packageLicenseMetadataWarningsVBox.Visible = true; + } + + void ShowLicenseFile (int fileNumber) + { + LicenseFileText licenseFileText = viewModel.SelectedPackage.GetLicenseFile (fileNumber); + if (licenseFileText != null) { + var dialog = new LicenseFileDialog (licenseFileText); + dialog.Run (this); + } + } } } \ No newline at end of file diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.csproj b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.csproj index 0cee2f7ffa7..2ea256d7b42 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.csproj +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.csproj @@ -369,6 +369,13 @@ + + + + + + + diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/ManagePackagesSearchResultViewModel.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/ManagePackagesSearchResultViewModel.cs index 6aa0d2c97a1..eb94d0439fe 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/ManagePackagesSearchResultViewModel.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/ManagePackagesSearchResultViewModel.cs @@ -47,6 +47,7 @@ internal class ManagePackagesSearchResultViewModel : ViewModelBase licenseLinks; string summary; bool isChecked; @@ -383,6 +384,7 @@ void OnPackageMetadataLoaded (Task task) if (metadata != null) { viewModel.Published = metadata.Published; dependencies = GetCompatibleDependencies ().ToArray (); + licenseLinks = PackageLicenseUtilities.GenerateLicenseLinks (metadata); OnPropertyChanged ("Dependencies"); } } @@ -467,6 +469,30 @@ public string GetCurrentPackageVersionAdditionalText () { return parent.GetCurrentPackageVersionAdditionalText (Id); } + + public bool HasLicenseMetadata { + get { return packageDetailModel?.PackageMetadata?.LicenseMetadata != null; } + } + + public IReadOnlyList GetLicenseLinks () + { + licenseLinks ??= new List (); + return licenseLinks; + } + + public LicenseFileText GetLicenseFile (int fileNumber) + { + int currentFileNumber = 0; + foreach (IText text in GetLicenseLinks ()) { + if (text is LicenseFileText licenseFileText) { + currentFileNumber++; + if (currentFileNumber == fileNumber) { + return licenseFileText; + } + } + } + return null; + } } } diff --git a/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/DetailedPackageMetadata.cs b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/DetailedPackageMetadata.cs index e16eddcd532..91ab22e8090 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/DetailedPackageMetadata.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/DetailedPackageMetadata.cs @@ -4,6 +4,8 @@ using System; using System.Collections.Generic; using System.Linq; +using NuGet.Packaging; +using NuGet.Protocol; using NuGet.Protocol.Core.Types; using NuGet.Versioning; @@ -17,6 +19,7 @@ public DetailedPackageMetadata() public DetailedPackageMetadata(IPackageSearchMetadata serverData, long? downloadCount) { + Id = serverData.Identity.Id; Version = serverData.Identity.Version; Summary = serverData.Summary; Description = serverData.Description; @@ -34,9 +37,16 @@ public DetailedPackageMetadata(IPackageSearchMetadata serverData, long? download ?? new PackageDependencySetMetadata[] { }; HasDependencies = DependencySets.Any( dependencySet => dependencySet.Dependencies != null && dependencySet.Dependencies.Count > 0); + LicenseMetadata = serverData.LicenseMetadata; + localMetadata = serverData as LocalPackageSearchMetadata; } + readonly LocalPackageSearchMetadata localMetadata; + + public string Id { get; set; } + public NuGetVersion Version { get; set; } + public string Summary { get; set; } public string Description { get; set; } @@ -63,5 +73,17 @@ public DetailedPackageMetadata(IPackageSearchMetadata serverData, long? download // This property is used by data binding to display text "No dependencies" public bool HasDependencies { get; set; } + + public LicenseMetadata LicenseMetadata { get; set; } + + public IReadOnlyList LicenseLinks => PackageLicenseUtilities.GenerateLicenseLinks (this); + + public string LoadFileAsText (string path) + { + if (localMetadata != null) { + return localMetadata.LoadFileAsText (path); + } + return null; + } } } \ No newline at end of file diff --git a/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/FreeText.cs b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/FreeText.cs new file mode 100644 index 00000000000..80508374086 --- /dev/null +++ b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/FreeText.cs @@ -0,0 +1,15 @@ +// 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 +{ + internal class FreeText : IText + { + public FreeText (string text) + { + Text = text; + } + + public string Text { get; } + } +} \ No newline at end of file diff --git a/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/IText.cs b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/IText.cs new file mode 100644 index 00000000000..afbd4ddc043 --- /dev/null +++ b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/IText.cs @@ -0,0 +1,10 @@ +// 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 interface IText + { + string Text { get; } + } +} \ No newline at end of file diff --git a/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/LicenseFileText.cs b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/LicenseFileText.cs new file mode 100644 index 00000000000..43f53cc98d9 --- /dev/null +++ b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/LicenseFileText.cs @@ -0,0 +1,78 @@ +// 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 System; +using System.ComponentModel; +using System.Threading; +using System.Threading.Tasks; +using MonoDevelop.Core; + +namespace NuGet.PackageManagement.UI +{ + internal class LicenseFileText : IText, INotifyPropertyChanged + { + string _text; + string _licenseText; + string _licenseHeader; + readonly string _licenseFileLocation; + private Func _loadFileFromPackage; + + int _initialized; + + internal LicenseFileText (string text, string licenseFileHeader, Func loadFileFromPackage, string licenseFileLocation) + { + _text = text; + _licenseHeader = licenseFileHeader; + _licenseText = GettextCatalog.GetString ("Loading license file…"); + _loadFileFromPackage = loadFileFromPackage; + _licenseFileLocation = licenseFileLocation; + } + + internal void LoadLicenseFile () + { + if (Interlocked.CompareExchange (ref _initialized, 1, 0) == 0) { + if (_loadFileFromPackage != null) { + Task.Run (async () => { + string content = _loadFileFromPackage (_licenseFileLocation); + await Runtime.RunInMainThread (() => { + LicenseText = content; + }); + }).Ignore (); + } + } + } + + public string LicenseHeader { + get => _licenseHeader; + set { + _licenseHeader = value; + OnPropertyChanged ("LicenseHeader"); + } + } + + public string Text { + get => _text; + set { + _text = value; + OnPropertyChanged ("Text"); + } + } + + public string LicenseFileLocation => _licenseFileLocation; + + public string LicenseText { + get => _licenseText; + set { + _licenseText = value; + OnPropertyChanged ("LicenseText"); + } + } + + public event PropertyChangedEventHandler PropertyChanged; + + protected void OnPropertyChanged (string name) + { + PropertyChanged?.Invoke (this, new System.ComponentModel.PropertyChangedEventArgs (name)); + } + } +} \ No newline at end of file diff --git a/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/LicenseText.cs b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/LicenseText.cs new file mode 100644 index 00000000000..41b470657cf --- /dev/null +++ b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/LicenseText.cs @@ -0,0 +1,19 @@ +// 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 System; + +namespace NuGet.PackageManagement.UI +{ + internal class LicenseText : IText + { + public LicenseText (string text, Uri link) + { + Text = text; + Link = link; + } + + public string Text { get; set; } + public Uri Link { get; set; } + } +} \ No newline at end of file diff --git a/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/WarningText.cs b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/WarningText.cs new file mode 100644 index 00000000000..dff77a14aa0 --- /dev/null +++ b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/Models/WarningText.cs @@ -0,0 +1,15 @@ +// 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 +{ + internal class WarningText : IText + { + public WarningText (string text) + { + Text = text; + } + + public string Text { get; } + } +} \ No newline at end of file diff --git a/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/PackageLicenseUtilities.cs b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/PackageLicenseUtilities.cs new file mode 100644 index 00000000000..30a0ad015d4 --- /dev/null +++ b/main/src/addins/MonoDevelop.PackageManagement/NuGet.PackageManagement.UI/PackageLicenseUtilities.cs @@ -0,0 +1,134 @@ +// 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 System; +using System.Collections.Generic; +using System.Globalization; +using MonoDevelop.Core; +using NuGet.Packaging; +using NuGet.Packaging.Licenses; +using NuGet.Protocol; +using NuGet.Protocol.Core.Types; + +namespace NuGet.PackageManagement.UI +{ + internal class PackageLicenseUtilities + { + internal static IReadOnlyList GenerateLicenseLinks (DetailedPackageMetadata metadata) + { + return GenerateLicenseLinks (metadata.LicenseMetadata, metadata.LicenseUrl, GettextCatalog.GetString ("{0} License", metadata.Id), metadata.LoadFileAsText); + } + + internal static IReadOnlyList GenerateLicenseLinks (IPackageSearchMetadata metadata) + { + if (metadata is LocalPackageSearchMetadata localMetadata) { + return GenerateLicenseLinks (metadata.LicenseMetadata, metadata.LicenseUrl, GettextCatalog.GetString ("{0} License", metadata.Identity.Id), localMetadata.LoadFileAsText); + } + return GenerateLicenseLinks (metadata.LicenseMetadata, metadata.LicenseUrl, metadata.Identity.Id, null); + } + + internal static IReadOnlyList GenerateLicenseLinks (LicenseMetadata licenseMetadata, Uri licenseUrl, string licenseFileHeader, Func loadFile) + { + if (licenseMetadata != null) { + return GenerateLicenseLinks (licenseMetadata, licenseFileHeader, loadFile); + } else if (licenseUrl != null) { + return new List () { new LicenseText (GettextCatalog.GetString ("View License"), licenseUrl) }; + } + return new List (); + } + + //internal static Paragraph[] GenerateParagraphs (string licenseContent) + //{ + // var textParagraphs = licenseContent.Split ( + // new[] { "\n\n", "\r\n\r\n" }, // Take care of paragraphs regardless of the name ending. It's a best effort, so weird line ending combinations might not work too well. + // StringSplitOptions.None); + + // var paragraphs = new Paragraph[textParagraphs.Length]; + // for (var i = 0; i < textParagraphs.Length; i++) { + // paragraphs[i] = new Paragraph (new Run (textParagraphs[i])); + // } + // return paragraphs; + //} + + // Internal for testing purposes. + internal static IReadOnlyList GenerateLicenseLinks (LicenseMetadata metadata, string licenseFileHeader, Func loadFile) + { + var list = new List (); + + if (metadata.WarningsAndErrors != null) { + list.Add (new WarningText (string.Join (Environment.NewLine, metadata.WarningsAndErrors))); + } + + switch (metadata.Type) { + case LicenseType.Expression: + + if (metadata.LicenseExpression != null && !metadata.LicenseExpression.IsUnlicensed ()) { + var identifiers = new List (); + PopulateLicenseIdentifiers (metadata.LicenseExpression, identifiers); + + var licenseToBeProcessed = metadata.License; + + foreach (var identifier in identifiers) { + var licenseStart = licenseToBeProcessed.IndexOf (identifier); + if (licenseStart != 0) { + list.Add (new FreeText (licenseToBeProcessed.Substring (0, licenseStart))); + } + var license = licenseToBeProcessed.Substring (licenseStart, identifier.Length); + list.Add (new LicenseText (license, new Uri (string.Format (LicenseMetadata.LicenseServiceLinkTemplate, license)))); + licenseToBeProcessed = licenseToBeProcessed.Substring (licenseStart + identifier.Length); + } + + if (licenseToBeProcessed.Length != 0) { + list.Add (new FreeText (licenseToBeProcessed)); + } + } else { + list.Add (new FreeText (metadata.License)); + } + + break; + + case LicenseType.File: + list.Add (new LicenseFileText (metadata.License, licenseFileHeader, loadFile, metadata.License)); + break; + + default: + break; + } + + return list; + } + + private static void PopulateLicenseIdentifiers (NuGetLicenseExpression expression, IList identifiers) + { + switch (expression.Type) { + case LicenseExpressionType.License: + var license = (NuGetLicense)expression; + identifiers.Add (license.Identifier); + break; + + case LicenseExpressionType.Operator: + var licenseOperator = (LicenseOperator)expression; + switch (licenseOperator.OperatorType) { + case LicenseOperatorType.LogicalOperator: + var logicalOperator = (LogicalOperator)licenseOperator; + PopulateLicenseIdentifiers (logicalOperator.Left, identifiers); + PopulateLicenseIdentifiers (logicalOperator.Right, identifiers); + break; + + case LicenseOperatorType.WithOperator: + var withOperator = (WithOperator)licenseOperator; + identifiers.Add (withOperator.License.Identifier); + identifiers.Add (withOperator.Exception.Identifier); + break; + + default: + break; + } + break; + + default: + break; + } + } + } +} \ No newline at end of file From 08ddf782fcbe914736e5ff1c061b82348e1beb2b Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Thu, 24 Oct 2019 20:15:11 +0100 Subject: [PATCH 02/12] [NuGet] Show license metadata in License Acceptance dialog If the NuGet package has associated license metadata then show this in the dialog. License metadata can be an expression or a file. A license file will now be displayed in a dialog if the View License link is clicked. Warning icons and associated message will also be displayed for deprecated licenses. There are problems here. Could not get the text to wrap so ended up making the dialog resizable and always showing the scrollbars. Hyperlinks in license expressions do not open the url in a browser since the Xwt Xamarin.Mac's LabelBackend does not implement support for this. --- .../LicenseAcceptanceDialog.cs | 116 ++++++++++++++---- .../LicenseFileDialog.cs | 45 ++++++- .../LicenseFileTextExtensions.cs | 39 ++++++ .../LicenseLinkMarkupBuilder.cs | 74 +++++++++++ .../ManagePackagesDialog.cs | 73 +++-------- .../MonoDevelop.PackageManagement.csproj | 2 + .../ManagePackagesSearchResultViewModel.cs | 14 --- .../NuGetPackageLicense.cs | 4 + .../PackageLicenseViewModel.cs | 7 ++ 9 files changed, 273 insertions(+), 101 deletions(-) create mode 100644 main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileTextExtensions.cs create mode 100644 main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseLinkMarkupBuilder.cs diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs index 852aa83080d..a0a88c097b9 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs @@ -30,7 +30,8 @@ using System; using System.Linq; using MonoDevelop.Core; -using MonoDevelop.Ide; +using MonoDevelop.Ide; +using NuGet.PackageManagement.UI; using Xwt; namespace MonoDevelop.PackageManagement @@ -46,7 +47,6 @@ internal class LicenseAcceptanceDialog : Dialog public LicenseAcceptanceDialog (LicenseAcceptanceViewModel viewModel) { Height = 350; - Resizable = false; Padding = 0; Title = GettextCatalog.GetString ("License Acceptance"); this.viewModel = viewModel; @@ -67,7 +67,7 @@ public LicenseAcceptanceDialog (LicenseAcceptanceViewModel viewModel) packagesList.Spacing = 0; scroll = new ScrollView (packagesList); - scroll.HorizontalScrollPolicy = ScrollPolicy.Never; + scroll.HorizontalScrollPolicy = ScrollPolicy.Automatic; scroll.VerticalScrollPolicy = ScrollPolicy.Automatic; scroll.BorderVisible = false; scroll.BackgroundColor = Ide.Gui.Styles.BackgroundColor; @@ -107,23 +107,6 @@ void AddPackages () } } - protected override void OnShown () - { - var count = packagesList.Children.Count (); - if (count > 0 && count < 4) { - scroll.VerticalScrollPolicy = ScrollPolicy.Never; - var firstRow = packagesList.Children.First (); - var rowHeight = firstRow.Size.Height; - Height -= (rowHeight + firstRow.MarginTop + firstRow.MarginBottom) * (4 - count); - } else if (count == 4) { - scroll.VerticalScrollPolicy = ScrollPolicy.Never; - } else if (count > 4) { - scroll.VerticalScrollPolicy = ScrollPolicy.Automatic; - Height += rowMargin.Top + rowMargin.Bottom; - } - base.OnShown (); - } - void AddPackage (PackageLicenseViewModel package) { var titleBox = new VBox (); @@ -131,11 +114,8 @@ void AddPackage (PackageLicenseViewModel package) titleBox.MarginBottom = 4; titleBox.PackStart (new Label { Markup = string.Format ("{0} – {1}", package.Id, package.Author), - }); - var licenseLabel = new LinkLabel (GettextCatalog.GetString ("View License")); - licenseLabel.Uri = package.LicenseUrl; - licenseLabel.LinkClicked += (sender, e) => IdeServices.DesktopService.ShowUrl (e.Target.AbsoluteUri); - titleBox.PackStart (licenseLabel); + }); + AddLicenseInfo (package, titleBox); var rowBox = new HBox (); rowBox.Margin = rowMargin; @@ -151,6 +131,85 @@ void AddPackage (PackageLicenseViewModel package) packagesList.PackStart (rowBox); } + static void AddLicenseInfo (PackageLicenseViewModel package, VBox parentVBox) + { + if (package.LicenseLinks.Any ()) { + if (package.LicenseLinks.Count == 1) { + IText textLink = package.LicenseLinks [0]; + if (textLink is LicenseText licenseText) { + AddLicenseLinkLabel (licenseText.Link, licenseText.Text, parentVBox); + return; + } else if (textLink is LicenseFileText licenseFileText) { + AddFileLicenseLinkLabel (licenseFileText, parentVBox); + return; + } else { + // Warning or free text fallback to showing an expression which will handle this. + } + } + AddLicenseExpressionLabel (package, parentVBox); + } else { + AddLicenseLinkLabel (package.LicenseUrl, GettextCatalog.GetString ("View License"), parentVBox); + } + } + + static void AddLicenseExpressionLabel (PackageLicenseViewModel package, VBox parentVBox) + { + var licenseLabel = new Label (); + var builder = new LicenseLinkMarkupBuilder (); + licenseLabel.Markup = builder.GetMarkup (package.LicenseLinks); + licenseLabel.Wrap = WrapMode.None; + + // Does not work. LabelBackend for Xamarin.Mac implementation does not ILabelEventSink.OnLinkClicked + licenseLabel.LinkClicked += (sender, e) => IdeServices.DesktopService.ShowUrl (e.Target.AbsoluteUri); + + foreach (WarningText warning in builder.Warnings) { + AddLicenseWarningLabel (warning, parentVBox); + } + + var hbox = new HBox (); + hbox.PackStart (licenseLabel, true); + + parentVBox.PackStart (hbox); + } + + static void AddLicenseWarningLabel (WarningText warning, VBox parentVBox) + { + var hbox = new HBox (); + var image = new ImageView { + Image = ImageService.GetIcon ("md-warning", Gtk.IconSize.Menu), + VerticalPlacement = WidgetPlacement.Start, + }; + image.Accessible.RoleDescription = GettextCatalog.GetString ("Warning Icon"); + + hbox.PackStart (image); + + var label = new Label { + Text = warning.Text, + Wrap = WrapMode.None + }; + image.Accessible.LabelWidget = label; + hbox.PackStart (label, true); + + parentVBox.PackStart (hbox); + } + + static void AddFileLicenseLinkLabel (LicenseFileText licenseFileText, VBox parentVBox) + { + var licenseLabel = new LinkLabel (GettextCatalog.GetString ("View License")); + licenseLabel.Uri = licenseFileText.CreateLicenseFileUri (1); + licenseLabel.Tag = licenseFileText; + licenseLabel.NavigateToUrl += (sender, e) => ShowFileDialog ((LinkLabel)sender); + parentVBox.PackStart (licenseLabel); + } + + static void AddLicenseLinkLabel (Uri licenseUrl, string labelText, VBox parentVBox) + { + var licenseLabel = new LinkLabel (labelText); + licenseLabel.Uri = licenseUrl; + licenseLabel.LinkClicked += (sender, e) => IdeServices.DesktopService.ShowUrl (e.Target.AbsoluteUri); + parentVBox.PackStart (licenseLabel); + } + public new bool Run (WindowFrame parentWindow) { return base.Run (parentWindow) == Command.Ok; @@ -162,6 +221,13 @@ protected override void Dispose (bool disposing) imageLoader.Dispose (); base.Dispose (disposing); } + + static void ShowFileDialog (LinkLabel label) + { + var licenseFileText = (LicenseFileText)label.Tag; + var dialog = new LicenseFileDialog (licenseFileText); + dialog.Run (label.ParentWindow); + } } } diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs index c84bcad96ba..83b8e590b63 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs @@ -24,14 +24,16 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. +using System; +using System.Collections.Generic; using System.ComponentModel; -using NuGet.PackageManagement.UI; +using NuGet.PackageManagement.UI; using Xwt; using Xwt.Formats; -namespace MonoDevelop.PackageManagement.Gui +namespace MonoDevelop.PackageManagement { - class LicenseFileDialog : Dialog + sealed class LicenseFileDialog : Dialog { RichTextView textView; LicenseFileText licenseFileText; @@ -77,5 +79,42 @@ protected override void Dispose (bool disposing) licenseFileText.PropertyChanged -= LicenseFileTextPropertyChanged; } } + + public static bool ShowDialog (Uri uri, IReadOnlyList licenseLinks, Dialog parent) + { + if (!uri.IsFile) + return false; + + if (uri.Fragment?.Length > 0) { + if (int.TryParse (uri.Fragment.Substring (1), out int fileNumber)) { + ShowLicenseFile (fileNumber, licenseLinks, parent); + return true; + } + } + return false; + } + + static void ShowLicenseFile (int fileNumber, IReadOnlyList licenseLinks, Dialog parent) + { + LicenseFileText licenseFileText = GetLicenseFile (fileNumber, licenseLinks); + if (licenseFileText != null) { + var dialog = new LicenseFileDialog (licenseFileText); + dialog.Run (parent); + } + } + + static LicenseFileText GetLicenseFile (int fileNumber, IReadOnlyList licenseLinks) + { + int currentFileNumber = 0; + foreach (IText text in licenseLinks) { + if (text is LicenseFileText licenseFileText) { + currentFileNumber++; + if (currentFileNumber == fileNumber) { + return licenseFileText; + } + } + } + return null; + } } } diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileTextExtensions.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileTextExtensions.cs new file mode 100644 index 00000000000..677279ab5fc --- /dev/null +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileTextExtensions.cs @@ -0,0 +1,39 @@ +// +// LicenseFileTextExtensions.cs +// +// Author: +// Matt Ward +// +// Copyright (c) 2019 Microsoft Corporation +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +using System; +using NuGet.PackageManagement.UI; + +namespace MonoDevelop.PackageManagement +{ + static class LicenseFileTextExtensions + { + public static Uri CreateLicenseFileUri (this LicenseFileText licenseFileText, int licenseCount) + { + return new Uri ($"file:///{licenseFileText.Text}#{licenseCount}"); + } + } +} diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseLinkMarkupBuilder.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseLinkMarkupBuilder.cs new file mode 100644 index 00000000000..96f0f9d90e8 --- /dev/null +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseLinkMarkupBuilder.cs @@ -0,0 +1,74 @@ +// +// LicenseLinkMarkupBuilder.cs +// +// Author: +// Matt Ward +// +// Copyright (c) 2019 Microsoft Corporation +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security; +using MonoDevelop.Core; +using NuGet.PackageManagement.UI; + +namespace MonoDevelop.PackageManagement +{ + sealed class LicenseLinkMarkupBuilder + { + List warnings; + + public string GetMarkup (IReadOnlyList textLinks) + { + var markupBuilder = StringBuilderCache.Allocate (); + + int fileLicenseCount = 0; // Should be one but handle multiple. + foreach (IText textLink in textLinks) { + if (textLink is LicenseText licenseText) { + markupBuilder.Append (GetUriMarkup (licenseText.Link, licenseText.Text)); + } else if (textLink is LicenseFileText licenseFileText) { + fileLicenseCount++; + markupBuilder.Append (GetUriMarkup (licenseFileText.CreateLicenseFileUri (fileLicenseCount), licenseFileText.Text)); + } else if (textLink is WarningText warning) { + warnings ??= new List (); + warnings.Add (warning); + } else { + markupBuilder.Append (textLink.Text); + } + } + + return StringBuilderCache.ReturnAndFree (markupBuilder); + } + + public IEnumerable Warnings { + get { return warnings ?? Enumerable.Empty (); } + } + + static string GetUriMarkup (Uri uri, string text) + { + return string.Format ( + "{1}", + uri != null ? SecurityElement.Escape (uri.ToString ()) : string.Empty, + SecurityElement.Escape (text)); + } + } +} diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs index 958f2af2db9..1fdb61d0348 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs @@ -31,7 +31,6 @@ using MonoDevelop.Components.AtkCocoaHelper; using MonoDevelop.Core; using MonoDevelop.Ide; -using MonoDevelop.PackageManagement.Gui; using MonoDevelop.Projects; using NuGet.PackageManagement.UI; using NuGet.Versioning; @@ -1304,14 +1303,6 @@ void ShowLicense (ManagePackagesSearchResultViewModel packageViewModel) } } - static string GetUriMarkup (Uri uri, string text) - { - return string.Format ( - "{1}", - uri != null ? SecurityElement.Escape (uri.ToString ()) : string.Empty, - SecurityElement.Escape (text)); - } - void PackageLicenseNavigateToUrl (object sender, NavigateToUrlEventArgs e) { if (ShowLicenseFile (e.Uri)) { @@ -1319,20 +1310,6 @@ void PackageLicenseNavigateToUrl (object sender, NavigateToUrlEventArgs e) } } - bool ShowLicenseFile (Uri uri) - { - if (!uri.IsFile) - return false; - - if (uri.Fragment?.Length > 0) { - if (int.TryParse (uri.Fragment.Substring (1), out int fileNumber)) { - ShowLicenseFile (fileNumber); - return true; - } - } - return false; - } - void PackageLicenseLinkClicked (object sender, LinkEventArgs e) { if (ShowLicenseFile (e.Target)) { @@ -1340,9 +1317,16 @@ void PackageLicenseLinkClicked (object sender, LinkEventArgs e) } } + bool ShowLicenseFile (Uri uri) + { + return LicenseFileDialog.ShowDialog ( + uri, + viewModel.SelectedPackage.GetLicenseLinks (), + this); + } + void ShowLicenseMetadata (ManagePackagesSearchResultViewModel packageViewModel) { - List warnings = null; var textLinks = packageViewModel.GetLicenseLinks (); if (textLinks.Count == 0) { @@ -1365,7 +1349,7 @@ void ShowLicenseMetadata (ManagePackagesSearchResultViewModel packageViewModel) return; } else if (textLink is LicenseFileText licenseFileText) { packageLicenseLink.Text = GettextCatalog.GetString ("View License"); - packageLicenseLink.Uri = CreateLicenseFileUri (licenseFileText, 1); + packageLicenseLink.Uri = licenseFileText.CreateLicenseFileUri (1); return; } else { // Warning or plain text - handled below. @@ -1373,40 +1357,20 @@ void ShowLicenseMetadata (ManagePackagesSearchResultViewModel packageViewModel) } // Multiple text links. We need to allow these to wrap so show these below the license label. - var markupBuilder = StringBuilderCache.Allocate (); - - int fileLicenseCount = 0; // Should be one but handle multiple. - foreach (IText textLink in textLinks) { - if (textLink is LicenseText licenseText) { - markupBuilder.Append (GetUriMarkup (licenseText.Link, licenseText.Text)); - } else if (textLink is LicenseFileText licenseFileText) { - fileLicenseCount++; - markupBuilder.Append (GetUriMarkup (CreateLicenseFileUri (licenseFileText, fileLicenseCount), licenseFileText.Text)); - } else if (textLink is WarningText warning) { - warnings ??= new List (); - warnings.Add (warning); - } else { - markupBuilder.Append (textLink.Text); - } - } + var markupBuilder = new LicenseLinkMarkupBuilder (); + packageLicenseMetadataLabel.Markup = markupBuilder.GetMarkup (textLinks); packageLicenseLink.Visible = false; packageLicenseMetadataHBox.Visible = true; - packageLicenseMetadataLabel.Markup = StringBuilderCache.ReturnAndFree (markupBuilder); - if (warnings != null) { - AddWarnings (warnings); + if (markupBuilder.Warnings.Any ()) { + AddWarnings (markupBuilder.Warnings); } else { packageLicenseMetadataWarningsVBox.Visible = false; } } - Uri CreateLicenseFileUri (LicenseFileText licenseFileText, int licenseCount) - { - return new Uri ($"file:///{licenseFileText.Text}#{licenseCount}"); - } - - void AddWarnings (List warnings) + void AddWarnings (IEnumerable warnings) { foreach (Widget child in packageLicenseMetadataWarningsVBox.Children.ToArray ()) { packageLicenseMetadataWarningsVBox.Remove (child); @@ -1437,14 +1401,5 @@ void AddWarnings (List warnings) packageLicenseMetadataWarningsVBox.Visible = true; } - - void ShowLicenseFile (int fileNumber) - { - LicenseFileText licenseFileText = viewModel.SelectedPackage.GetLicenseFile (fileNumber); - if (licenseFileText != null) { - var dialog = new LicenseFileDialog (licenseFileText); - dialog.Run (this); - } - } } } \ No newline at end of file diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.csproj b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.csproj index 2ea256d7b42..50f0d17b01f 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.csproj +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.csproj @@ -376,6 +376,8 @@ + + diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/ManagePackagesSearchResultViewModel.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/ManagePackagesSearchResultViewModel.cs index eb94d0439fe..8234f3ddf5d 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/ManagePackagesSearchResultViewModel.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/ManagePackagesSearchResultViewModel.cs @@ -479,20 +479,6 @@ public IReadOnlyList GetLicenseLinks () licenseLinks ??= new List (); return licenseLinks; } - - public LicenseFileText GetLicenseFile (int fileNumber) - { - int currentFileNumber = 0; - foreach (IText text in GetLicenseLinks ()) { - if (text is LicenseFileText licenseFileText) { - currentFileNumber++; - if (currentFileNumber == fileNumber) { - return licenseFileText; - } - } - } - return null; - } } } diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/NuGetPackageLicense.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/NuGetPackageLicense.cs index 3bbb43c9e9e..f4db7130cb7 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/NuGetPackageLicense.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/NuGetPackageLicense.cs @@ -25,6 +25,8 @@ // THE SOFTWARE. using System; +using System.Collections.Generic; +using NuGet.PackageManagement.UI; using NuGet.Packaging.Core; using NuGet.Protocol.Core.Types; @@ -40,6 +42,7 @@ public NuGetPackageLicense (IPackageSearchMetadata metadata) PackageAuthor = metadata.Authors; LicenseUrl = metadata.LicenseUrl; IconUrl = metadata.IconUrl; + LicenseLinks = PackageLicenseUtilities.GenerateLicenseLinks (metadata); } public PackageIdentity PackageIdentity { get; private set; } @@ -48,6 +51,7 @@ public NuGetPackageLicense (IPackageSearchMetadata metadata) public string PackageAuthor { get; private set; } public Uri LicenseUrl { get; private set; } public Uri IconUrl { get; private set; } + public IReadOnlyList LicenseLinks { get; } } } diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/PackageLicenseViewModel.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/PackageLicenseViewModel.cs index cad8322ad45..ec72fb70db0 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/PackageLicenseViewModel.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/PackageLicenseViewModel.cs @@ -27,6 +27,9 @@ // using System; +using System.Collections.Generic; +using System.Linq; +using NuGet.PackageManagement.UI; namespace MonoDevelop.PackageManagement { @@ -54,5 +57,9 @@ public string Author { public Uri IconUrl { get { return packageLicense.IconUrl; } } + + public IReadOnlyList LicenseLinks { + get { return packageLicense.LicenseLinks; } + } } } From 617951fdd4f28def2d61797c81c64c6de59b412a Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 25 Oct 2019 10:07:49 +0100 Subject: [PATCH 03/12] [NuGet] Simplify logic for license files It is not possible to have more than one license file defined for a NuGet package and it is not possible to use one in an expression. This means the markup builder does not need to handle this and simplifies the logic here. --- .../LicenseAcceptanceDialog.cs | 2 +- .../LicenseFileDialog.cs | 39 +------------------ .../LicenseFileTextExtensions.cs | 4 +- .../LicenseLinkMarkupBuilder.cs | 5 +-- .../ManagePackagesDialog.cs | 26 ++++--------- 5 files changed, 13 insertions(+), 63 deletions(-) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs index a0a88c097b9..0536df89c01 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs @@ -196,7 +196,7 @@ static void AddLicenseWarningLabel (WarningText warning, VBox parentVBox) static void AddFileLicenseLinkLabel (LicenseFileText licenseFileText, VBox parentVBox) { var licenseLabel = new LinkLabel (GettextCatalog.GetString ("View License")); - licenseLabel.Uri = licenseFileText.CreateLicenseFileUri (1); + licenseLabel.Uri = licenseFileText.CreateLicenseFileUri (); licenseLabel.Tag = licenseFileText; licenseLabel.NavigateToUrl += (sender, e) => ShowFileDialog ((LinkLabel)sender); parentVBox.PackStart (licenseLabel); diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs index 83b8e590b63..86b065645ec 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs @@ -26,7 +26,7 @@ using System; using System.Collections.Generic; -using System.ComponentModel; +using System.ComponentModel; using NuGet.PackageManagement.UI; using Xwt; using Xwt.Formats; @@ -79,42 +79,5 @@ protected override void Dispose (bool disposing) licenseFileText.PropertyChanged -= LicenseFileTextPropertyChanged; } } - - public static bool ShowDialog (Uri uri, IReadOnlyList licenseLinks, Dialog parent) - { - if (!uri.IsFile) - return false; - - if (uri.Fragment?.Length > 0) { - if (int.TryParse (uri.Fragment.Substring (1), out int fileNumber)) { - ShowLicenseFile (fileNumber, licenseLinks, parent); - return true; - } - } - return false; - } - - static void ShowLicenseFile (int fileNumber, IReadOnlyList licenseLinks, Dialog parent) - { - LicenseFileText licenseFileText = GetLicenseFile (fileNumber, licenseLinks); - if (licenseFileText != null) { - var dialog = new LicenseFileDialog (licenseFileText); - dialog.Run (parent); - } - } - - static LicenseFileText GetLicenseFile (int fileNumber, IReadOnlyList licenseLinks) - { - int currentFileNumber = 0; - foreach (IText text in licenseLinks) { - if (text is LicenseFileText licenseFileText) { - currentFileNumber++; - if (currentFileNumber == fileNumber) { - return licenseFileText; - } - } - } - return null; - } } } diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileTextExtensions.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileTextExtensions.cs index 677279ab5fc..154255e3ce0 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileTextExtensions.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileTextExtensions.cs @@ -31,9 +31,9 @@ namespace MonoDevelop.PackageManagement { static class LicenseFileTextExtensions { - public static Uri CreateLicenseFileUri (this LicenseFileText licenseFileText, int licenseCount) + public static Uri CreateLicenseFileUri (this LicenseFileText licenseFileText) { - return new Uri ($"file:///{licenseFileText.Text}#{licenseCount}"); + return new Uri ($"file:///{licenseFileText.Text}"); } } } diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseLinkMarkupBuilder.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseLinkMarkupBuilder.cs index 96f0f9d90e8..3a6149526d1 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseLinkMarkupBuilder.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseLinkMarkupBuilder.cs @@ -41,13 +41,12 @@ public string GetMarkup (IReadOnlyList textLinks) { var markupBuilder = StringBuilderCache.Allocate (); - int fileLicenseCount = 0; // Should be one but handle multiple. foreach (IText textLink in textLinks) { if (textLink is LicenseText licenseText) { markupBuilder.Append (GetUriMarkup (licenseText.Link, licenseText.Text)); } else if (textLink is LicenseFileText licenseFileText) { - fileLicenseCount++; - markupBuilder.Append (GetUriMarkup (licenseFileText.CreateLicenseFileUri (fileLicenseCount), licenseFileText.Text)); + // Should not happen. Building an expression should not contain a license file. + LoggingService.LogError ("Unexpected LicenseFileText when building markup {0}", licenseFileText.Text); } else if (textLink is WarningText warning) { warnings ??= new List (); warnings.Add (warning); diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs index 1fdb61d0348..b7561c2ec07 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs @@ -101,7 +101,6 @@ public ManagePackagesDialog ( closeButton.Clicked += CloseButtonClicked; packageLicenseLink.NavigateToUrl += PackageLicenseNavigateToUrl; - packageLicenseMetadataLabel.LinkClicked += PackageLicenseLinkClicked; showPrereleaseCheckBox.Clicked += ShowPrereleaseCheckBoxClicked; packageSourceComboBox.SelectionChanged += PackageSourceChanged; addPackagesButton.Clicked += AddPackagesButtonClicked; @@ -152,7 +151,6 @@ protected override void Dispose (bool disposing) { closeButton.Clicked -= CloseButtonClicked; packageLicenseLink.NavigateToUrl -= PackageLicenseNavigateToUrl; - packageLicenseMetadataLabel.LinkClicked -= PackageLicenseLinkClicked; currentPackageVersionLabel.BoundsChanged -= PackageVersionLabelBoundsChanged; showPrereleaseCheckBox.Clicked -= ShowPrereleaseCheckBoxClicked; @@ -1294,6 +1292,7 @@ void ProjectCheckBoxCellViewToggled (object sender, WidgetEventArgs e) void ShowLicense (ManagePackagesSearchResultViewModel packageViewModel) { + packageLicenseLink.Tag = null; if (packageViewModel.HasLicenseMetadata) { ShowLicenseMetadata (packageViewModel); } else { @@ -1305,26 +1304,14 @@ void ShowLicense (ManagePackagesSearchResultViewModel packageViewModel) void PackageLicenseNavigateToUrl (object sender, NavigateToUrlEventArgs e) { - if (ShowLicenseFile (e.Uri)) { + var licenseFileText = packageLicenseLink.Tag as LicenseFileText; + if (licenseFileText != null) { e.SetHandled (); + var dialog = new LicenseFileDialog (licenseFileText); + dialog.Run (this); } } - void PackageLicenseLinkClicked (object sender, LinkEventArgs e) - { - if (ShowLicenseFile (e.Target)) { - e.SetHandled (); - } - } - - bool ShowLicenseFile (Uri uri) - { - return LicenseFileDialog.ShowDialog ( - uri, - viewModel.SelectedPackage.GetLicenseLinks (), - this); - } - void ShowLicenseMetadata (ManagePackagesSearchResultViewModel packageViewModel) { var textLinks = packageViewModel.GetLicenseLinks (); @@ -1349,7 +1336,8 @@ void ShowLicenseMetadata (ManagePackagesSearchResultViewModel packageViewModel) return; } else if (textLink is LicenseFileText licenseFileText) { packageLicenseLink.Text = GettextCatalog.GetString ("View License"); - packageLicenseLink.Uri = licenseFileText.CreateLicenseFileUri (1); + packageLicenseLink.Uri = licenseFileText.CreateLicenseFileUri (); + packageLicenseLink.Tag = licenseFileText; return; } else { // Warning or plain text - handled below. From 9f1114c1a077833bbace6eac9756a35800c16a8a Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 25 Oct 2019 12:11:25 +0100 Subject: [PATCH 04/12] [NuGet] Fix license expression links in License Acceptance dialog Using the non-native XWT toolkit to display the dialog allows the hyperlinks to be clicked. It also fixes the scrollbars showing a whitebackground, even when not used. Looked at adding the resize logic back, which would reduce the height of the dialog based on the contents. However using the XWT toolkit which is non-native (GTK#) the dialog is displayed first and then resized so you can see it shrink. Instead the dialog does not resize to fit its contents which leaves some empty space. --- .../LicenseAcceptanceService.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs index 57d89949145..3518ccfae03 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs @@ -51,11 +51,13 @@ Task ShowLicenseAcceptanceDialog (IEnumerable license { var res = new TaskCompletionSource (); IdeApp.RunWhenIdle (() => { - Xwt.Toolkit.NativeEngine.Invoke (delegate { + // Disable use of native toolkit since hyperlinks cannot be clicked with the Xamarin.Mac LabelBackend + // Also scrollbars are always visible even when they are not needed. + //Xwt.Toolkit.NativeEngine.Invoke (delegate { using (LicenseAcceptanceDialog dialog = CreateLicenseAcceptanceDialog (licenses)) { res.SetResult (dialog.Run (MessageService.RootWindow)); } - }); + //}); }); return res.Task; } From 2f412e643666cd6d9e8806432dc57997bbf9d717 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 1 Nov 2019 15:16:46 +0000 Subject: [PATCH 05/12] [NuGet] Right align license metadata and show warnings as tooltip Manage NuGet packages dialog changes: Show license expression on the same line as the License label in the Manage NuGet packages dialog. Right align and wrap the license expression. Move the license warnings to a tooltip for the warning icon instead of showing the text directly in the dialog. Move the warning icon so it is on the right hand side of the license expression text. License acceptance dialog changes: Show license warnings in the warning icon tooltip instead of directly on the dialog. Show license warning icon on the same line as the license expression, on the left hand side of the expression. Fixes VSTS #1012071 - Right-align licenses to improve readability and text wrapping Fixes VSTS #1012079 - Use tooltip on warning icon to provide more details --- .../LicenseAcceptanceDialog.cs | 43 +++++++-------- .../ManagePackagesDialog.UI.cs | 36 ++++++------- .../ManagePackagesDialog.cs | 54 +++++-------------- 3 files changed, 49 insertions(+), 84 deletions(-) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs index 0536df89c01..9fadbf99422 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs @@ -162,33 +162,28 @@ static void AddLicenseExpressionLabel (PackageLicenseViewModel package, VBox par // Does not work. LabelBackend for Xamarin.Mac implementation does not ILabelEventSink.OnLinkClicked licenseLabel.LinkClicked += (sender, e) => IdeServices.DesktopService.ShowUrl (e.Target.AbsoluteUri); - foreach (WarningText warning in builder.Warnings) { - AddLicenseWarningLabel (warning, parentVBox); - } - var hbox = new HBox (); - hbox.PackStart (licenseLabel, true); - parentVBox.PackStart (hbox); - } + if (builder.Warnings.Any ()) { + var warningTextBuilder = StringBuilderCache.Allocate (); + foreach (WarningText warning in builder.Warnings) { + warningTextBuilder.Append (warning.Text); + warningTextBuilder.Append (' '); + } - static void AddLicenseWarningLabel (WarningText warning, VBox parentVBox) - { - var hbox = new HBox (); - var image = new ImageView { - Image = ImageService.GetIcon ("md-warning", Gtk.IconSize.Menu), - VerticalPlacement = WidgetPlacement.Start, - }; - image.Accessible.RoleDescription = GettextCatalog.GetString ("Warning Icon"); - - hbox.PackStart (image); - - var label = new Label { - Text = warning.Text, - Wrap = WrapMode.None - }; - image.Accessible.LabelWidget = label; - hbox.PackStart (label, true); + var warningWidget = new MonoDevelop.Components.InformationPopoverWidget (); + warningWidget.Severity = Ide.Tasks.TaskSeverity.Warning; + warningWidget.Message = StringBuilderCache.ReturnAndFree (warningTextBuilder).TrimEnd (); + + // Disable focus. With focus enabled the info popup ends up being displayed and focused + // if it is the first package license on opening the dialog. Unable to set focus to a + // dialog button. + warningWidget.CanGetFocus = false; + + hbox.PackStart (warningWidget); + } + + hbox.PackStart (licenseLabel, true); parentVBox.PackStart (hbox); } diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.UI.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.UI.cs index a6ab18375c8..8c594e2a57d 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.UI.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.UI.cs @@ -49,9 +49,8 @@ internal partial class ManagePackagesDialog : ExtendedTitleBarDialog Label packageDownloads; Label packageLicenseLabel; LinkLabel packageLicenseLink; - VBox packageLicenseMetadataWarningsVBox; - HBox packageLicenseMetadataHBox; - Label packageLicenseMetadataLabel; + InformationPopoverWidget packageLicenseMetadataWarningInfoPopoverWidget; + Label packageLicenseMetadataLinkLabel; LinkLabel packageProjectPageLink; Label packageDependenciesList; HBox packageDependenciesHBox; @@ -353,27 +352,24 @@ void Build () packageLicenseLabel = new Label (); packageLicenseLabel.Text = GettextCatalog.GetString ("License"); packageLicenseLabel.Font = packageInfoBoldFont; - packageLicenseHBox.PackStart (packageLicenseLabel); + packageLicenseHBox.PackStart (packageLicenseLabel, vpos: WidgetPlacement.Start); packageLicenseLink = new LinkLabel (); packageLicenseLink.Text = GettextCatalog.GetString ("View License"); packageLicenseLink.Font = packageInfoSmallFont; - packageLicenseHBox.PackEnd (packageLicenseLink); - - packageLicenseMetadataWarningsVBox = new VBox (); - packageLicenseMetadataWarningsVBox.Visible = false; - packageInfoVBox.PackStart (packageLicenseMetadataWarningsVBox); - - packageLicenseMetadataHBox = new HBox (); - packageLicenseMetadataHBox.Visible = false; - packageInfoVBox.PackStart (packageLicenseMetadataHBox); - - packageLicenseMetadataLabel = new Label (); - packageLicenseMetadataLabel.Wrap = WrapMode.Word; - packageLicenseMetadataLabel.MarginLeft = 5; - packageLicenseMetadataLabel.Font = packageInfoSmallFont; - packageLicenseMetadataLabel.Accessible.LabelWidget = packageLicenseLabel; - packageLicenseMetadataHBox.PackStart (packageLicenseMetadataLabel, true); + packageLicenseLink.TextAlignment = Alignment.End; + packageLicenseHBox.PackStart (packageLicenseLink, true); + + packageLicenseMetadataLinkLabel = new Label (); + packageLicenseMetadataLinkLabel.Wrap = WrapMode.Word; + packageLicenseMetadataLinkLabel.Font = packageInfoSmallFont; + packageLicenseMetadataLinkLabel.Accessible.LabelWidget = packageLicenseLabel; + packageLicenseMetadataLinkLabel.TextAlignment = Alignment.End; + packageLicenseHBox.PackStart (packageLicenseMetadataLinkLabel, true, vpos: WidgetPlacement.Start); + + packageLicenseMetadataWarningInfoPopoverWidget = new InformationPopoverWidget (); + packageLicenseMetadataWarningInfoPopoverWidget.Severity = Ide.Tasks.TaskSeverity.Warning; + packageLicenseHBox.PackStart (packageLicenseMetadataWarningInfoPopoverWidget, vpos: WidgetPlacement.Start, hpos: WidgetPlacement.End); // Package project page. var packageProjectPageHBox = new HBox (); diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs index b7561c2ec07..87eff1d4d8d 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs @@ -1296,8 +1296,7 @@ void ShowLicense (ManagePackagesSearchResultViewModel packageViewModel) if (packageViewModel.HasLicenseMetadata) { ShowLicenseMetadata (packageViewModel); } else { - packageLicenseMetadataHBox.Visible = false; - packageLicenseMetadataWarningsVBox.Visible = false; + packageLicenseMetadataLinkLabel.Visible = false; ShowUri (packageLicenseLink, packageViewModel.LicenseUrl, GettextCatalog.GetString ("View License")); } } @@ -1314,20 +1313,18 @@ void PackageLicenseNavigateToUrl (object sender, NavigateToUrlEventArgs e) void ShowLicenseMetadata (ManagePackagesSearchResultViewModel packageViewModel) { - var textLinks = packageViewModel.GetLicenseLinks (); + packageLicenseLink.Visible = false; + packageLicenseMetadataLinkLabel.Visible = false; + packageLicenseMetadataWarningInfoPopoverWidget.Visible = false; + var textLinks = packageViewModel.GetLicenseLinks (); if (textLinks.Count == 0) { - packageLicenseLink.Visible = false; - packageLicenseMetadataHBox.Visible = false; - packageLicenseMetadataWarningsVBox.Visible = false; return; } // Single link - show this on the same line as the License label. if (textLinks.Count == 1) { packageLicenseLink.Visible = true; - packageLicenseMetadataHBox.Visible = false; - packageLicenseMetadataWarningsVBox.Visible = false; IText textLink = textLinks [0]; if (textLink is LicenseText licenseText) { @@ -1346,48 +1343,25 @@ void ShowLicenseMetadata (ManagePackagesSearchResultViewModel packageViewModel) // Multiple text links. We need to allow these to wrap so show these below the license label. var markupBuilder = new LicenseLinkMarkupBuilder (); - packageLicenseMetadataLabel.Markup = markupBuilder.GetMarkup (textLinks); - - packageLicenseLink.Visible = false; - packageLicenseMetadataHBox.Visible = true; + packageLicenseMetadataLinkLabel.Markup = markupBuilder.GetMarkup (textLinks); + packageLicenseMetadataLinkLabel.Visible = true; if (markupBuilder.Warnings.Any ()) { AddWarnings (markupBuilder.Warnings); } else { - packageLicenseMetadataWarningsVBox.Visible = false; + packageLicenseMetadataWarningInfoPopoverWidget.Visible = false; } } void AddWarnings (IEnumerable warnings) { - foreach (Widget child in packageLicenseMetadataWarningsVBox.Children.ToArray ()) { - packageLicenseMetadataWarningsVBox.Remove (child); - child.Dispose (); - } - + var warningTextBuilder = StringBuilderCache.Allocate (); foreach (WarningText warning in warnings) { - var hbox = new HBox (); - var image = new ImageView { - Image = ImageService.GetIcon ("md-warning", Gtk.IconSize.Menu), - MarginLeft = packageLicenseMetadataLabel.MarginLeft, - VerticalPlacement = WidgetPlacement.Start, - }; - image.Accessible.RoleDescription = GettextCatalog.GetString ("Warning Icon"); - hbox.PackStart (image); - - var label = new Label { - Text = warning.Text, - Font = packageLicenseMetadataLabel.Font, - Wrap = WrapMode.Word - }; - image.Accessible.LabelWidget = label; - label.Accessible.LabelWidget = packageLicenseLabel; - hbox.PackStart (label, true, true); - - packageLicenseMetadataWarningsVBox.PackStart (hbox); - } - - packageLicenseMetadataWarningsVBox.Visible = true; + warningTextBuilder.Append (warning.Text); + warningTextBuilder.Append (' '); + } + packageLicenseMetadataWarningInfoPopoverWidget.Message = StringBuilderCache.ReturnAndFree (warningTextBuilder).TrimEnd (); + packageLicenseMetadataWarningInfoPopoverWidget.Visible = true; } } } \ No newline at end of file From 0c9667c6093162fb57be8b1afbaae134eb6d1f01 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 1 Nov 2019 17:22:36 +0000 Subject: [PATCH 06/12] [NuGet] Improve license file dialog 1. Fix button label so it reads OK instead of Ok 2. Show license file text in scroll view. 3. Adding scroll view gives the text a one pixel border. 4. Make text view read-only. --- .../LicenseFileDialog.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs index 86b065645ec..4667c69f415 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs @@ -24,8 +24,6 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. -using System; -using System.Collections.Generic; using System.ComponentModel; using NuGet.PackageManagement.UI; using Xwt; @@ -55,11 +53,16 @@ void Build () Width = 450; Title = licenseFileText.LicenseHeader; + var scrollView = new ScrollView (); + Content = scrollView; + textView = new RichTextView (); - Content = textView; + textView.ReadOnly = true; + scrollView.Content = textView; - Buttons.Add (Command.Ok); - DefaultCommand = Command.Ok; + var okCommand = new Command ("Ok", Core.GettextCatalog.GetString ("OK")); + Buttons.Add (); + DefaultCommand = okCommand; } void LicenseFileTextPropertyChanged (object sender, PropertyChangedEventArgs e) From 57997dd05877f48a91c337e3c82be01c4f59e5f9 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Thu, 16 Jan 2020 14:26:07 +0000 Subject: [PATCH 07/12] [NuGet] Switch back to native toolkit to display license dialog Use Xwt.Toolkit.NativeEngine to show the License Acceptance dialog. Briefly switched to not using this because complicated license expressions cannot have their hyperlinks clicked. However these do not seem to be used in practice. The native version fixes odd tearing of the list view in the UI, also the list scrolling to the end, and not being able to resize the UI without displaying it first. Downside to this change is that hyperlinks in complicated license expressions cannot be clicked since this is not supported by XWT's Xamarin.Mac implementation of the Label. --- .../LicenseAcceptanceDialog.cs | 19 ++++++++++++++++++- .../LicenseAcceptanceService.cs | 8 ++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs index 9fadbf99422..fa7a1951e9a 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs @@ -67,7 +67,7 @@ public LicenseAcceptanceDialog (LicenseAcceptanceViewModel viewModel) packagesList.Spacing = 0; scroll = new ScrollView (packagesList); - scroll.HorizontalScrollPolicy = ScrollPolicy.Automatic; + scroll.HorizontalScrollPolicy = ScrollPolicy.Never; scroll.VerticalScrollPolicy = ScrollPolicy.Automatic; scroll.BorderVisible = false; scroll.BackgroundColor = Ide.Gui.Styles.BackgroundColor; @@ -107,6 +107,23 @@ void AddPackages () } } + protected override void OnShown () + { + var count = packagesList.Children.Count (); + if (count > 0 && count < 4) { + scroll.VerticalScrollPolicy = ScrollPolicy.Never; + var firstRow = packagesList.Children.First (); + var rowHeight = firstRow.Size.Height; + Height -= (rowHeight + firstRow.MarginTop + firstRow.MarginBottom) * (4 - count); + } else if (count == 4) { + scroll.VerticalScrollPolicy = ScrollPolicy.Never; + } else if (count > 4) { + scroll.VerticalScrollPolicy = ScrollPolicy.Automatic; + Height += rowMargin.Top + rowMargin.Bottom; + } + base.OnShown (); + } + void AddPackage (PackageLicenseViewModel package) { var titleBox = new VBox (); diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs index 3518ccfae03..b0131874d78 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs @@ -51,13 +51,13 @@ Task ShowLicenseAcceptanceDialog (IEnumerable license { var res = new TaskCompletionSource (); IdeApp.RunWhenIdle (() => { - // Disable use of native toolkit since hyperlinks cannot be clicked with the Xamarin.Mac LabelBackend - // Also scrollbars are always visible even when they are not needed. - //Xwt.Toolkit.NativeEngine.Invoke (delegate { + // Note that using native toolkit means hyperlinks from complicated license expressions cannot be + // clicked with the Xamarin.Mac LabelBackend + Xwt.Toolkit.NativeEngine.Invoke (delegate { using (LicenseAcceptanceDialog dialog = CreateLicenseAcceptanceDialog (licenses)) { res.SetResult (dialog.Run (MessageService.RootWindow)); } - //}); + }); }); return res.Task; } From fe8a8523d9f5cf8364891e8a55c368c3ecf9f42e Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 17 Jan 2020 12:38:58 +0000 Subject: [PATCH 08/12] [NuGet] Ensure the file license dialog looks the same when displayed Showing the file license dialog from the License Acceptance dialog was showing the dialog with the native Xamarin.Mac toolkit but the Manage NuGet packages dialog was using the non-native toolkit (GTK#). Now the file license dialog is always launched with the native toolkit. Also removed the horizontal scrollbar since this seems to cause the text to unwrap using the native window. --- .../LicenseAcceptanceDialog.cs | 7 +++++-- .../MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs | 1 + .../ManagePackagesDialog.cs | 7 +++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs index fa7a1951e9a..ff9ba68067e 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs @@ -237,8 +237,11 @@ protected override void Dispose (bool disposing) static void ShowFileDialog (LinkLabel label) { var licenseFileText = (LicenseFileText)label.Tag; - var dialog = new LicenseFileDialog (licenseFileText); - dialog.Run (label.ParentWindow); + Xwt.Toolkit.NativeEngine.Invoke (delegate { + using (var dialog = new LicenseFileDialog (licenseFileText)) { + dialog.Run (label.ParentWindow); + } + }); } } } diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs index 4667c69f415..3a04ca67483 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs @@ -54,6 +54,7 @@ void Build () Title = licenseFileText.LicenseHeader; var scrollView = new ScrollView (); + scrollView.HorizontalScrollPolicy = ScrollPolicy.Never; Content = scrollView; textView = new RichTextView (); diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs index 87eff1d4d8d..af960723bbb 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/ManagePackagesDialog.cs @@ -1306,8 +1306,11 @@ void PackageLicenseNavigateToUrl (object sender, NavigateToUrlEventArgs e) var licenseFileText = packageLicenseLink.Tag as LicenseFileText; if (licenseFileText != null) { e.SetHandled (); - var dialog = new LicenseFileDialog (licenseFileText); - dialog.Run (this); + Toolkit.NativeEngine.Invoke (delegate { + using (var dialog = new LicenseFileDialog (licenseFileText)) { + dialog.Run (this); + } + }); } } From 33b2ae8e6e2ef530aedadf0900ac9753b2915c7d Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 17 Jan 2020 12:42:28 +0000 Subject: [PATCH 09/12] [NuGet] Fix warning logged when clicking file license link Clicking the View License link in the License Acceptance dialog was logging a warning about failing to start the url. The problem was that the custom url used to open the file license was being handled by the default LinkLabel implementation. Now the NavigateToUrlEventArgs is set as handled so this does not happen. --- .../LicenseAcceptanceDialog.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs index ff9ba68067e..e76fea8e4ff 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs @@ -210,7 +210,10 @@ static void AddFileLicenseLinkLabel (LicenseFileText licenseFileText, VBox paren var licenseLabel = new LinkLabel (GettextCatalog.GetString ("View License")); licenseLabel.Uri = licenseFileText.CreateLicenseFileUri (); licenseLabel.Tag = licenseFileText; - licenseLabel.NavigateToUrl += (sender, e) => ShowFileDialog ((LinkLabel)sender); + licenseLabel.NavigateToUrl += (sender, e) => { + e.SetHandled (); + ShowFileDialog ((LinkLabel)sender); + }; parentVBox.PackStart (licenseLabel); } From 227dc6cdb7d1f4edda403a584810a47059d899a0 Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Thu, 23 Jan 2020 15:27:52 +0000 Subject: [PATCH 10/12] [NuGet] Fix file license dialog scrollbar not always enabled When the loading of the file license text happens after the dialog has been displayed the vertical scrollbar for the rich text view would not be enabled unless the dialog was resized by the user. This seems to happen only when using the native toolkit with XWT. To workaround this the dialog's OnReallocate method is called when the license text is loaded after the dialog is shown. This enables the vertical scrollbar if the license text is too long to be displayed. --- .../MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs index 3a04ca67483..4ed7da4312b 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseFileDialog.cs @@ -69,6 +69,10 @@ void Build () void LicenseFileTextPropertyChanged (object sender, PropertyChangedEventArgs e) { LoadText (); + + // Need to refresh the dialog after the license text has been loaded. Otherwise when using the + // native toolkit the vertical scrollbar is not enabled unless you re-size the dialog. + OnReallocate (); } void LoadText () From 16d65e7ffa95d79d78799856fdc33c72dfc912ac Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 24 Jan 2020 12:33:41 +0000 Subject: [PATCH 11/12] [NuGet] Fix license expression accessibility on license accept dialog License expressions on the license acceptance dialog had hyperlinks that could not be clicked or tabbed to. Being unable to click the link was specific to using the native toolkit. Tabbing was not specific to the native toolkit. Voice Over would also not identify the links. To fix this the license expression parts are added with individual labels and link labels. The parent hbox has its accessibility role set to group and given the name 'License Expression'. The hyperlinks can now be clicked and tabbed to. Voice over will read each part of the license expression separately and recognises the links. Only problem here is that using the native XWT toolkit means the text has more spacing around it than using the GTK# XWT toolkit. --- .../LicenseAcceptanceDialog.cs | 58 ++++++++++++++----- .../LicenseAcceptanceService.cs | 2 - 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs index e76fea8e4ff..52cdbf0a72d 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs @@ -28,6 +28,7 @@ // using System; +using System.Collections.Generic; using System.Linq; using MonoDevelop.Core; using MonoDevelop.Ide; @@ -171,19 +172,26 @@ static void AddLicenseInfo (PackageLicenseViewModel package, VBox parentVBox) static void AddLicenseExpressionLabel (PackageLicenseViewModel package, VBox parentVBox) { - var licenseLabel = new Label (); - var builder = new LicenseLinkMarkupBuilder (); - licenseLabel.Markup = builder.GetMarkup (package.LicenseLinks); - licenseLabel.Wrap = WrapMode.None; - - // Does not work. LabelBackend for Xamarin.Mac implementation does not ILabelEventSink.OnLinkClicked - licenseLabel.LinkClicked += (sender, e) => IdeServices.DesktopService.ShowUrl (e.Target.AbsoluteUri); - + // Using the HBox as an accessibility group since the license will have multiple parts. + // Cannot use a single label with markup for the license expression since it is not accessible, + // also when using the native toolkit the hyperlinks cannot be clicked. var hbox = new HBox (); + hbox.Spacing = 0; + hbox.Accessible.Role = Xwt.Accessibility.Role.Group; + hbox.Accessible.Label = GettextCatalog.GetString ("License Expression"); + + // Get any warnings. + List warnings = null; + foreach (IText textLink in package.LicenseLinks) { + if (textLink is WarningText warning) { + warnings ??= new List (); + warnings.Add (warning); + } + } - if (builder.Warnings.Any ()) { + if (warnings?.Any () == true) { var warningTextBuilder = StringBuilderCache.Allocate (); - foreach (WarningText warning in builder.Warnings) { + foreach (WarningText warning in warnings) { warningTextBuilder.Append (warning.Text); warningTextBuilder.Append (' '); } @@ -191,18 +199,36 @@ static void AddLicenseExpressionLabel (PackageLicenseViewModel package, VBox par var warningWidget = new MonoDevelop.Components.InformationPopoverWidget (); warningWidget.Severity = Ide.Tasks.TaskSeverity.Warning; warningWidget.Message = StringBuilderCache.ReturnAndFree (warningTextBuilder).TrimEnd (); - - // Disable focus. With focus enabled the info popup ends up being displayed and focused - // if it is the first package license on opening the dialog. Unable to set focus to a - // dialog button. + warningWidget.MarginRight = 6; + + // Disable focus. With focus enabled the info popup ends up being displayed and focused + // if it is the first package license on opening the dialog. Unable to set focus to a + // dialog button. warningWidget.CanGetFocus = false; hbox.PackStart (warningWidget); } - hbox.PackStart (licenseLabel, true); + foreach (IText textLink in package.LicenseLinks) { + if (textLink is LicenseText licenseText) { + var label = new LinkLabel (licenseText.Text); + label.TextAlignment = Alignment.Start; + label.Uri = licenseText.Link; + label.LinkClicked += (sender, e) => IdeServices.DesktopService.ShowUrl (e.Target.AbsoluteUri); + hbox.PackStart (label, false, false); + } else if (textLink is WarningText warning) { + // Ignore. + } else if (textLink is LicenseFileText licenseFileText) { + // Should not happen. A license expression should not contain a license file. + LoggingService.LogError ("Unexpected LicenseFileText when building licence expression {0}", licenseFileText.Text); + } else { + var label = new Label (textLink.Text); + label.TextAlignment = Alignment.Start; + hbox.PackStart (label, false, false); + } + } - parentVBox.PackStart (hbox); + parentVBox.PackStart (hbox, false, false); } static void AddFileLicenseLinkLabel (LicenseFileText licenseFileText, VBox parentVBox) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs index b0131874d78..57d89949145 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs @@ -51,8 +51,6 @@ Task ShowLicenseAcceptanceDialog (IEnumerable license { var res = new TaskCompletionSource (); IdeApp.RunWhenIdle (() => { - // Note that using native toolkit means hyperlinks from complicated license expressions cannot be - // clicked with the Xamarin.Mac LabelBackend Xwt.Toolkit.NativeEngine.Invoke (delegate { using (LicenseAcceptanceDialog dialog = CreateLicenseAcceptanceDialog (licenses)) { res.SetResult (dialog.Run (MessageService.RootWindow)); From d8adac599c1d984dca2a88f85e3ebf77a908c12b Mon Sep 17 00:00:00 2001 From: Matt Ward Date: Fri, 24 Jan 2020 12:57:08 +0000 Subject: [PATCH 12/12] [NuGet] Add accessibility label to image in license accept dialog --- .../LicenseAcceptanceDialog.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs index 52cdbf0a72d..0e1a16751ea 100644 --- a/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs +++ b/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement.Gui/LicenseAcceptanceDialog.cs @@ -127,18 +127,20 @@ protected override void OnShown () void AddPackage (PackageLicenseViewModel package) { + var label = new Label { + Markup = string.Format ("{0} – {1}", package.Id, package.Author), + }; var titleBox = new VBox (); titleBox.Spacing = 0; titleBox.MarginBottom = 4; - titleBox.PackStart (new Label { - Markup = string.Format ("{0} – {1}", package.Id, package.Author), - }); + titleBox.PackStart (label); AddLicenseInfo (package, titleBox); var rowBox = new HBox (); rowBox.Margin = rowMargin; var icon = new ImageView (ImageService.GetIcon ("md-package", Gtk.IconSize.Dnd)); + icon.Accessible.LabelWidget = label; if (package.IconUrl != null && !string.IsNullOrEmpty (package.IconUrl.AbsoluteUri)) imageLoader.LoadFrom (package.IconUrl, icon);