Adds hovers to the Graph (wip) #3321
Conversation
| "https-proxy-agent": "5.0.1", | ||
| "iconv-lite": "0.6.3", | ||
| "lit": "3.1.3", | ||
| "marked": "12.0.2", |
There was a problem hiding this comment.
That was on purpose to avoid conflicts. I think that is something we should do generally, because it just causes issues when merging. Though it is needed for the ultimate merge so it adds another merge step.
There was a problem hiding this comment.
Isn't it a good thing that it causes conflicts (i.e. wouldn't we want to catch/fix these conflicts ahead of time, rather than on merge?) or am I missing something?
eamodio
force-pushed
the
feature/graph-hovers
branch
from
b41456a to
640d080
Compare
sergeibbb
changed the title
sergeibbb
changed the title
eamodio
force-pushed
the
feature/graph-hovers
branch
from
640d080 to
854ab02
Compare
|
Hovers are cached (in both the webview and in the exthost -- except for WIP as it needs to be able to update with changes). We also re-use the stats if the "Changes" column is loaded, though we aren't invalidating the hover caches if that changes. I've also move the hover to be below the row (and offset from the mouse so you can more easily move up/down and mouse wheel scroll, but if the hover inverts right now all bets are off, so maybe @d13 can look more into that. |
axosoft-ramint
left a comment
axosoft-ramint
left a comment
There was a problem hiding this comment.
LGTM - definitely still some funkiness with scrolling, but it's still a better experience than current.
Once this dependency PR is merged, I will bump the graph component version here and add the new property we need to suppress the graph's internal tooltips, and then we can merge.
Adds hovers to the Graph. This is still a work-in-progress and should probably be moved into the graph component itself.
See GLVSC-524