-
-
Notifications
You must be signed in to change notification settings - Fork 81
License clues section in Licenses explorer #570
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
Conversation
Signed-off-by: Omkar Phansopkar <[email protected]>
…pe defs Signed-off-by: Omkar Phansopkar <[email protected]>
Signed-off-by: Omkar Phansopkar <[email protected]>
AyanSinhaMahapatra
left a comment
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.
@OmkarPh looks good thanks++
I've tested out with the scan in the issue and this works as expected. Couple of follow ups:
-
Possible UI improvements?

The horizontal and vertical scrollbars are present always here, on the left pane.

In packages view these scrollbars are only present when we have a lot of packages/dependencies or we expand the dependencies and there is scrolling.
IMHO the scrollbars should only be present when scrolling is possible.
This is also present in other views like the packages explorer. -
Grouping same license clues together.
This has to happen from the toolkit side, I'm thinking of adding a new field in the license clues : for_detection as a refinement to be able to group these together. This will be useful where there are a bunch of repeating license clues. We'll then do the needful here to have them similarly as the matches table in license detections then.
Signed-off-by: Omkar Phansopkar <[email protected]>
|
I couldn't reproduce the ugly empty scroll panes on my system (maybe it's OS specific 🤔 ) |
AyanSinhaMahapatra
left a comment
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.
LGTM! Thanks!
thanks 😄 |


Fixes #565