Add custom graph styling support #438
Conversation
The implementation makes use of the webview communication mechanism to switch messages between the webview and the graph. It works as follows: - When the webview is loaded, it now sends a single request to VSCode, the request asks VSCode for the graph style - When VSCode answers with the style, the graph style is updated and the webview loading process continues as normal. The style change does not modify the API, in fact it makes use of the shadiest programming tatic ever, *global variables* to remain compatible. The style object *currently* supports four base fields: - background: string - fontSize: int - highlightedForeground: string - node: object - note: string - nonExistingNote: string - unknown: string
| @@ -300,6 +336,15 @@ try { | |||
| Actions.selectNode(noteId); | |||
| } | |||
| break; | |||
| case "graphStyle": | |||
| const style = message.payload; | |||
| updateStyle(style); | |||
There was a problem hiding this comment.
mmm... I think here you need a slightly different approach, to be in line with the other actions (and I think it will also simplify the code).
Because style is now part of the model (that is, it can be changed by the app), I would:
- add it to the model itself (and the style code in
initDatavizneeds to reference the model, not thestyleglobal) - make
updateStylean action, and in its implementation make the changes to the style in the model
If the design doesn't make sense to you let's chat.
There was a problem hiding this comment.
By model, do you mean this?
foam/packages/foam-vscode/static/graphs/default/graph.js
Lines 39 to 49 in 32f40fa
There was a problem hiding this comment.
yes.
actions are used to update the model.
the model is rendered by the view (which was initialized once in initGraph, the force-graph library doesn't have a clear separation between model and view, but it's still good enough for us)
There was a problem hiding this comment.
New commits fix this, please review
|
That CI fail is weird... |
| @@ -23,6 +33,7 @@ const feature: FoamFeature = { | |||
| noteAddedListener.dispose(); | |||
| noteUpdatedListener.dispose(); | |||
| noteDeletedListener.dispose(); | |||
| panel = undefined; | |||
There was a problem hiding this comment.
| panel = undefined; | |
| panel.dispose(); |
There was a problem hiding this comment.
This is per-VSCode documentation https://code.visualstudio.com/api/extension-guides/webview
There was a problem hiding this comment.
you are totally right, I misunderstood what was happening here
This PR addresses #436
The implementation makes use of the webview communication mechanism to
switch messages between the webview and the graph.
It works as follows:
the request asks VSCode for the graph style
the webview loading process continues as normal.
The style change does not modify the API, in fact it makes use of the
shadiest programming tatic ever, global variables to remain
compatible.
The style object currently supports four base fields:
For an ugly example, see below:
