Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Details Pane shows Package Owner profile links on Browse Tab #5961

Merged
merged 9 commits into from
Aug 9, 2024

Conversation

donnie-msft
Copy link
Contributor

@donnie-msft donnie-msft commented Aug 7, 2024

Bug

Fixes: NuGet/Home#13686
Fixes: NuGet/Home#10666
Spec

Description

Render Owner profile hyperlinks consistent with how it's done on the packages list.
image

  • Plumb the KnownOwnerViewModels from the package item into the detail pane's model.
  • Tooltips are shown with the actual URL
    image
  • Author is unchanged & still shows if available.
  • Disable showing Owner from nuspec on any tab (not just Browse tab).
    The only scenario where this could happen was with a local package source with an owner specified in the nuspec. It did not occur for remote/V3 package sources.
    image

Other notes:

  • Enabled nullable on a couple existing classes.
  • I attempted to refactor existing ImmutableList to ImmutableArray, but this caused too many new issues, so I'll do that separately.
  • No software design pattern or architectural improvements/refactoring is being done to existing Views or ViewModels in this PR. I'm happy to contribute to that type of refactoring work in a separate effort.

Accessibility

  • Accessibility Insights reports no errors:
    image
  • I did not test Psuedo-localization (PLOC) since the reflow design of the packages list is already handled - that is, it wraps within the grid column.
  • Keyboard navigation visits each hyperlink
  • Tested with Narrator & JAWS and verified the links announce similar to other hyperlinks.
  • Follow-up: Add explanatory text into the ToolTip or explain that these profile URLs are owners from a particular source. Known Owner URL ToolTips don't mention the originating Package Source Home#13414

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@donnie-msft donnie-msft requested a review from a team as a code owner August 7, 2024 21:34
Copy link
Contributor

@jebriede jebriede left a comment

Choose a reason for hiding this comment

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

🚀

@@ -1729,6 +1731,51 @@ public void SetInstalledOrUpdateButtonIsEnabled_AfterPackageSourceMappingChanges
nameof(PackageSolutionDetailControlModel.CanUninstall) + " should have raised a PropertyChanged when calling "
+ nameof(DetailControlModel.SetInstalledOrUpdateButtonIsEnabled) + " and the value should become true.");
}

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove extra blank line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll get this in my next iteration

@donnie-msft donnie-msft merged commit 61df591 into dev Aug 9, 2024
28 checks passed
@donnie-msft donnie-msft deleted the dev-donnie-msft-ownerAuthorDetailsPane branch August 9, 2024 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants