Skip to content
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

Fix graph issue when deleting node #401

Merged
merged 6 commits into from
Dec 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/foam-core/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"esModuleInterop": true,
"importHelpers": true,
"downlevelIteration": true,
"target": "es2019",
// commonjs module format is used so that the incremental
// tsc build-mode ran during development can replace individual
// files (as opposed to generate the .cjs.development.js bundle.
Expand Down
6 changes: 5 additions & 1 deletion packages/foam-vscode/src/features/dataviz.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as vscode from "vscode";
import * as path from "path";
import { FoamFeature } from "../types";
import { Foam } from "foam-core";
import { Foam, Logger } from "foam-core";
import { TextDecoder } from "util";
import { getTitleMaxLength } from "../settings";
import { isSome } from "../utils";
Expand Down Expand Up @@ -120,6 +120,10 @@ async function createGraphPanel(foam: Foam, context: vscode.ExtensionContext) {
vscode.window.showTextDocument(doc, vscode.ViewColumn.One);
});
break;

case "error":
Logger.error("An error occurred in the graph view", message.payload);
break;
}
},
undefined,
Expand Down
27 changes: 23 additions & 4 deletions packages/foam-vscode/static/graphs/default/graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function getStyle(name, fallback) {

const style = {
background: getStyle(`--vscode-panel-background`, "#202020"),
fontSize: parseInt(getStyle(`--vscode-font-size`, 12)),
fontSize: parseInt(getStyle(`--vscode-font-size`, 12)) - 2,
highlightedForeground: getStyle(
"--vscode-list-highlightForeground",
"#f9c74f"
Expand All @@ -27,7 +27,7 @@ const style = {
const sizeScale = d3
.scaleLinear()
.domain([0, 30])
.range([1, 3])
.range([0.5, 2])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick for the future, changes like these add noise to the original PR
I also suffer from this but I think we can all each other get better

.clamp(true);

const labelAlpha = d3
Expand Down Expand Up @@ -97,6 +97,12 @@ const Actions = {
});
m.data.links = links; // links can be swapped out without problem

// check that selected/hovered nodes are still valid (see #397)
m.hoverNode = remaining.has(m.hoverNode) ? m.hoverNode : null;
m.selectedNodes = new Set(
Array.from(m.selectedNodes).filter(nId => remaining.has(nId))
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rebased my branch for #394 on top of this -- I'm noticing that when I save a file the graph loses track of which node is selected / being hovered over. Could that be related to my change of not deleting the node anymore on save?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just reproduced this on master, so it's unrelated to your changes - I am investigating what's causing this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you rebase from master the issue is now fixed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

// annoying we need to call this function, but I haven't found a good workaround
graph.graphData(m.data);
}),
Expand Down Expand Up @@ -124,7 +130,7 @@ function initDataviz(channel) {
.d3Force("x", d3.forceX())
.d3Force("y", d3.forceY())
.d3Force("collide", d3.forceCollide(graph.nodeRelSize()))
.linkWidth(0.5)
.linkWidth(0.2)
.linkDirectionalParticles(1)
.linkDirectionalParticleWidth(link =>
getLinkState(link, model) === "highlighted" ? 1 : 0
Expand All @@ -142,7 +148,7 @@ function initDataviz(channel) {
const label = info.title;

Draw(ctx)
.circle(node.x, node.y, size + 0.5, border)
.circle(node.x, node.y, size + 0.2, border)
.circle(node.x, node.y, size, fill)
.text(label, node.x, node.y + size + 1, fontSize, textColor);
})
Expand Down Expand Up @@ -260,6 +266,19 @@ try {
type: "webviewDidLoad"
});
};
window.addEventListener("error", error => {
vscode.postMessage({
type: "error",
payload: {
message: error.message,
filename: error.filename,
lineno: error.lineno,
colno: error.colno,
error: error.error
}
});
});

window.addEventListener("message", event => {
const message = event.data;

Expand Down
2 changes: 2 additions & 0 deletions tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
"declaration": true,
"declarationMap": true,
"baseUrl": ".",
"module": "commonjs",
"target": "ES2019",
"paths": {
"foam-core": ["./packages/foam-core/src"],
"foam-cli": ["./packages/foam-cli/src"],
Expand Down