fix: flowchart rendering issues #61
Conversation
This commit modifies the computeEdgePositions function in utils.ts to keep points that are on a straight line in the same direction. The filter condition is updated to only remove points that have both the same x and y coordinates as the previous point. This ensures that points on a straight line in the same direction are maintained.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@ad1992 It was also mentioned about escaping HTML. Does it make sense for mermaid-to-excalidraw to bring this as well? I see that this is a bit out of the scope of this PR, it has to be something a bit more global for all diagrams. |
src/parser/flowchart.ts
Outdated
| @@ -228,9 +240,12 @@ export const parseMermaidFlowChartDiagram = ( | |||
| Object.keys(vertices).forEach((id) => { | |||
| vertices[id] = parseVertex(vertices[id], containerEl); | |||
| }); | |||
|
|
|||
| const processedEdges = new Set<string>(); | |||
There was a problem hiding this comment.
thought: I feel there's a better way to do this but I haven't figured it out yet, the same happens in the state diagram.
There was a problem hiding this comment.
I have made into a function closure for better encapsulation
There was a problem hiding this comment.
I added a map with the aim of avoiding a for loop inside parseEdges, and the only way I thought to encapsulate it was to create a kind of factory haha.
function createEdgeParser() {
const edgeCountMap = new Map<string, number>();
return function parseEdge(data: any, containerEl: HTMLElement) {
const edgeIdentifier = `${data.start}-${data.end}`;
const count = edgeCountMap.get(edgeIdentifier) || 0;
edgeCountMap.set(edgeIdentifier, count + 1);
// Rest of parseEdge implementation...
return {
};
};
}
There was a problem hiding this comment.
Thanks for the fix @igorwessel
I have turned processedEdges into a function closure for better encapsulation
The arrows seems to be breaking in this PR 👇🏻
I guess you meant font awesome support ? we don't support it yet, it's out of scope of this PR. We can look into that later. |
No but basic support for some HTML tags, for example: In this issue #45, we have some entities using HTML tags to format labels, although I believe we should not support this. |
@ad1992 Apparently, the problem is that the penultimate point is very close to the last point. It may be solved by calculating the distance between the penultimate point and the last point, and only including the penultimate point when it is greater than some limit we define. 
|
@igorwessel this was working earlier for all arrows but not anymore. Since it is breaking post the changes in this PR, we should be able to fix it in this PR itself. |
|
@ad1992 I implemented two solutions but I'm still undecided about which is the best approach:
I sent the commit with both solutions, both have their pros/cons but should work in the majority of cases. Only comment the check for distance for 1 solution (testing some values I think I reached the ideal of 50.) And we could check the closure because the problem of repeated edges has returned 😢 |
|
I'm will pull the updates from main, and create some tests to ensure. |
…into fix/45/flowchart_rendering_issues
|
@ad1992 friendly reminder, you think this distance calculation solution is enough? |
Hi @igorwessel sorry for the delayed response! I am gonna check this soon either tonight or tomorrow morning |
There was a problem hiding this comment.
@igorwessel looks good!
I have pushed some tweaks and improvements in e7e9f65, let me know if this looks good and we can update the tests then accordingly.
|
@ad1992 it's great! I didn't remember there was a function for hypotenuse in Math haha |
|
Pushed a fix for excluding point when its same as prev point |
Closes #45
Revision
8d58faa
There are cases where one entity can have multiple relationships to another entity, we were only considering that there was just one relationship.
31fea66
The filter condition was updated to include points that change in either the x or y direction.
This ensures that points on a straight line in the same direction are included in the reflectionPoints array, and ensures line commands when only changes direction.
By ignoring the directions that a line command can make we were only having straight lines.
Testcases