Add icons and main badge to Model Details View#2683
Conversation
norascheuch
force-pushed
the
nora/model-details-view-styling
branch
from
2448f3c to
df88049
Compare
| this.dataProvider.setState(externalApiUsages, databaseItem); | ||
| this.treeView.badge = { | ||
| value: externalApiUsages.length, | ||
| tooltip: "Number of unmodeled external APIs", |
There was a problem hiding this comment.
I think this tooltip isn't strictly true. We're showing all external APIs and not just the unmodeled ones.
There was a problem hiding this comment.
That's true, but a problem. We should only show unmodeled APIs in this view! Shall I make the change in this PR or create a new one, since the problem wasn't introduced here?
|
Talked about this in person with @norascheuch and we decided to:
|
| } else { | ||
| return { | ||
| label: item.label, | ||
| label: `${item.label} at ${item.url.uri} [${item.url.startLine}, ${item.url.endLine}]`, |
There was a problem hiding this comment.
This looks good, but there's a couple of ways we can make it so it matches the planned designs a little better. I made a couple of tweaks and ended up with:
I feel it's easiest to explain by just putting up the code for it, so you can have a look and see if you want to use this approach: https://github.com/github/vscode-codeql/compare/nora/model-details-view-styling...robertbrignull/nora/model-details-view-styling?expand=1
The two changes are:
- If we use the
descriptionfield for the path and line number, then it's rendered in grey and slightly smaller. - It would be great if we could not show the
file:/prefix and also not show absolute paths. Instead we should show the relative path inside the database source archive because this is going to be shorter and more understandable for users. Unfortunately it turned out to be a little annoying to compute this relative path. ThesourceLocationPrefixis calculated from theDatabaseItemthat we already have, but it needs a CodeQL CLI server to do so and is an async operation. I think the best thing to do is to pre-calculate it whenever the database item changes.
There was a problem hiding this comment.
Oh I'm sorry 🤦♀️ you mentioned the description thing but I forgot. Thanks for making the changes! I was wondering about how to get the relative file path but didn't think as far as to add the cliServer and make all the following changes. Looks very good now!
There was a problem hiding this comment.
I was wondering about how to get the relative file path but didn't think as far as to add the
cliServerand make all the following changes. Looks very good now!
I was hoping it would be easier than that. I basically just tried every field on DatabaseItem until I found the one that did what I wanted 😅 . Sadly that then required passing the cliServer around 😞 .
robertbrignull
left a comment
robertbrignull
left a comment
There was a problem hiding this comment.
LGTM and works for me locally
2 participants
Adds styling to the new Mode Details View.
Checklist
ready-for-doc-reviewlabel there.