-
Notifications
You must be signed in to change notification settings - Fork 676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds deprecation links in PM UI #4144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this looks pretty reasonable, but I don't have great understanding about why it works just yet.
Did we consider the fact that alternate package metadata has a version range included as well?
src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/DeprecationControl.xaml
Outdated
Show resolved
Hide resolved
9b838de
to
c51d9bf
Compare
29f7f5f
to
ce6969d
Compare
src/NuGet.Clients/NuGet.PackageManagement.UI/Resources/Commands.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Resources/Commands.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemDeprecationLabel.xaml.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemDeprecationLabel.xaml.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemDeprecationLabel.xaml.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs
Outdated
Show resolved
Hide resolved
I think |
src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageItemViewModel.cs
Outdated
Show resolved
Hide resolved
@kartheekp-ms The link provides the user with a shortcut to the alternative package without having to click on the package to see the details so it saves them clicks. Since this code is complying with the design spec that resulted after user studies and was under review and approved by stakeholders and the UX board, I would recommend keeping it in the code. |
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemDeprecationLabel.xaml.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemDeprecationLabel.xaml.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemControl.xaml
Outdated
Show resolved
Hide resolved
This is awesome! I do have a few questions though - When you click on the alternative package link, is there a way to only show that package in the package list rather than initiating a regular search for the ID? It seems weird that we surface a full list of search results when the alternative package is a specific package. I know for nuget.org packages, we can use "PackageId: " to get a single result. That's what I use in my proposal for VS package deep links: NuGet/Home#11047 The drawback is that it only works for nuget.org packages. That being said, I'm not sure it's likely that a user would have another feed that supports deprecation, but maybe @joelverhagen and @loic-sharma might know better. Regardless of how we do it, I think this would be a much better experience if we only surfaced the specific alternative package. AFAIK It's not necessarily even guaranteed that searching for an exact ID will surface the ID matched package as the first result. For reference:
Also, if you click on the alternative package link from the Installed or Updates tab, does it go to the Browse tab? |
ce6969d
to
6ff3580
Compare
@nkolev92 I tried including the version on search boxes, but, nuget.org shows no results. That's why i'm only using package ID. |
@chgill-MSFT Yes, it switches to Browse tab to look for packages. Also, added |
008cdee
to
be9d15c
Compare
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs
Outdated
Show resolved
Hide resolved
...Get.PackageManagement.UI.Test/Converters/DeprecationToDeprecationLabelStateConverterTests.cs
Show resolved
Hide resolved
test/NuGet.Core.Tests/NuGet.Versioning.Test/VersionRangeTests.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemDeprecationLabel.xaml
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemControl.xaml
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemDeprecationLabel.xaml
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/FormatedStringPartConverter.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/FormatedStringPartConverter.cs
Outdated
Show resolved
Hide resolved
be9d15c
to
50edf06
Compare
src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/FormattedStringPartConverter.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/FormattedStringPartConverter.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/FormattedStringPartConverter.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/FormattedStringPartConverter.cs
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs
Outdated
Show resolved
Hide resolved
...Clients/NuGet.PackageManagement.UI/Converters/DeprecationToDeprecationLabelStateConverter.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Converters/FormattedStringPartConverter.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageItemDeprecationLabel.xaml
Outdated
Show resolved
Hide resolved
6275503
to
1f52be5
Compare
...Clients/NuGet.PackageManagement.UI/Converters/DeprecationToDeprecationLabelStateConverter.cs
Show resolved
Hide resolved
…ationToDeprecationLabelStateConverter.cs Co-authored-by: Jean-Pierre Briedé <jebriede@microsoft.com>
Co-authored-by: Kartheek Penagamuri <52756182+kartheekp-ms@users.noreply.github.com>
Wording and else Co-authored-by: Christopher R. Gill <chrisraygill@gmail.com> Co-authored-by: Kartheek Penagamuri <52756182+kartheekp-ms@users.noreply.github.com>
5f72d4e
to
a8ccacd
Compare
Bug
Fixes: NuGet/Home#10996
Regression? Last working version: No
Spec: https://github.com/NuGet/Home/blob/8fb8d219a76b99c2a3ce79f8364c946283d4a19b/proposed/2021/VSdeprecationfeature.md
Description
Adds three messages in PM UI:
Demos
LInk in Package list
Link in Details pane
From Installed tab
PR Checklist
PR has a meaningful title
PR has a linked issue.
Described changes
Tests
[ ] Test exception[ ] N/ADocumentation
[ ] Documentation PR or issue filledAccessibility Tests
Test in Package List
Test in Details Pane
Known issues
This PR is affected by NuGet/Home#11055 ; that can be fixed separately.