Skip to content
This repository has been archived by the owner on Oct 4, 2021. It is now read-only.

[NuGet] Show license metadata #9085

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

[NuGet] Show license metadata #9085

wants to merge 12 commits into from

Conversation

mrward
Copy link
Member

@mrward mrward commented Oct 25, 2019

On hold. Accessibility problems with embedded hyperlinks. May involve a re-design/re-think of how complex license expressions are displayed.

  • 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.

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

  • 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. 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:

  1. 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. Worked around this by using the gtk# backend instead.
    Fixed by using separate LinkLabels.
  2. Embedded hyperlinks in complex license expressions are not accessible in Manage Packages dialog (OK in license acceptance dialog)
  3. 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.
    a. 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.
    Fixed
  • Screenshots

MITLicense

LicenseExpression

LicenseWarning

LicenseWarningTooltip

FileLicenseDialog

LicenseAcceptanceDialog

@karann-msft
Copy link

what does selecting "view license" do when the package is coming from a v3 feed like nuget.org?

@mrward
Copy link
Member Author

mrward commented Oct 29, 2019

@karann-msft View License opens the license url in the default browser or the license file (if one is available) in a dialog.


Buttons.Add (Command.Ok);
DefaultCommand = Command.Ok;
var okCommand = new Command ("Ok", Core.GettextCatalog.GetString ("OK"));
Copy link
Contributor

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

Copy link
Member Author

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.

Copy link
Contributor

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"?

Copy link
Member Author

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'.

Copy link
Contributor

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?

@mrward mrward force-pushed the nuget-license-metadata branch 2 times, most recently from 8fb69db to 6e60408 Compare January 16, 2020 11:22
@mrward
Copy link
Member Author

mrward commented Jan 16, 2020

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:

NuGetLicenseDialog-Tearing

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).

@mrward
Copy link
Member Author

mrward commented Jan 16, 2020

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.

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.
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.
Base automatically changed from master to main March 9, 2021 14:17
@akoeplinger akoeplinger changed the base branch from main to master March 15, 2021 17:01
Base automatically changed from master to main March 15, 2021 17:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants