-
Notifications
You must be signed in to change notification settings - Fork 1k
[NuGet] Show license metadata #9085
base: main
Are you sure you want to change the base?
Conversation
what does selecting "view license" do when the package is coming from a v3 feed like nuget.org? |
@karann-msft View License opens the license url in the default browser or the license file (if one is available) in a dialog. |
fdabca2
to
ecfb73c
Compare
|
||
Buttons.Add (Command.Ok); | ||
DefaultCommand = Command.Ok; | ||
var okCommand = new Command ("Ok", Core.GettextCatalog.GetString ("OK")); |
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.
There is Command.Ok already
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.
Yes, originally that was used. However in the UX review there were complaints about showing 'Ok' instead of 'OK' as the button text. I guess I could change the Command.Ok but did not want to make that change.
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.
This is interesting. So this means all "OK" in MD are "Ok"?
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.
Only dialogs that use Xwt and use the Command.Ok will show 'Ok'.
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.
I think it would make sense to reword this globally to "OK". @hbons?
8fb69db
to
6e60408
Compare
Bit stuck with this. Showing the license acceptance dialog with the native toolkit means the hyperlinks do not work on this dialog. You cannot click them. Could not see how to support clickable links in XWT with the NSTextField - some research suggests to either use the NSTextView, since this has support to finding the character based on a mouse click, or maybe adding a bunch of code to register a layout manager with the NSTextField. Using the non-native (gtk) license license acceptance dialog has bad UX. The list scrolls down and you get odd tearing of the UI: Also ended up removing the resize logic when using gtk since you see the dialog resize after it is displayed which does not not look great here. Other problem is that license expressions (with more than one hyperlink in the label) is not accessible. In general NuGet packages only have a single license but NuGet supports expressions. The single license is OK with accessibility (currently you cannot open hyperlinks with Voice Over but that is not specific to this dialog). |
I think I look at changing the license acceptance dialog back to using the native toolkit. Only complicated license expressions cannot be clicked but I have not seen one used anywhere. The other simpler licenses (which happen in general) use a LinkLabel so they work. |
52bf273
to
1f1abf2
Compare
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
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.
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.
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.
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
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.
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.
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.
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.
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.
1f1abf2
to
227dc6c
Compare
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.
On hold. Accessibility problems with embedded hyperlinks. May involve a re-design/re-think of how complex license expressions are displayed.
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.
Test cases on 1004394:
Fixes VSTS #1005394 - Support the new "license" properties in the NuGet
package manager UI
Fixes VSTS #1012071 - Right-align licenses to improve readability and
text wrapping
Fixes VSTS #1012079 - Use tooltip on warning icon to provide more
details
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. Not wrapping should be OK in the majority of cases since the text should fit unless a complex license expression is used. Scrollbars show with a whitebackground unlike the scrollbars when using the GTK# backend.
Known issues:
Hyperlinks in license expressions do not open the url in a browserFixed by using separate LinkLabels.since the Xwt Xamarin.Mac's LabelBackend does not implement support
for this. Worked around this by using the gtk# backend instead.
The File License dialog when using native toolkit does not allow the user to scroll the license text when it does not fit. You have to resize the dialog for this to work.Fixeda. The non-native UI is OK - however cannot use this since the license acceptance dialog is using the native UI and having that dialog open the file license dialog results in it being shown behind the license acceptance dialog.