Skip to content
This repository was archived by the owner on Jul 29, 2019. It is now read-only.

Network: edge-endpoint 'circle' #2066

Merged
merged 4 commits into from
Oct 17, 2016
Merged

Network: edge-endpoint 'circle' #2066

merged 4 commits into from
Oct 17, 2016

Conversation

mojoaxel
Copy link
Member

@mojoaxel mojoaxel commented Sep 4, 2016

Make it possible to also define an type of edge-endpoint (arrow or circle):

image

implements #1993

@mojoaxel
Copy link
Member Author

mojoaxel commented Oct 2, 2016

Can anybody please review this!

Copy link
Member

@Tooa Tooa left a comment

Choose a reason for hiding this comment

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

Thanks for this enhancement 👍

@@ -206,9 +206,9 @@ if (typeof CanvasRenderingContext2D !== 'undefined') {


/**
* Draw an arrow point (no line)
* Draw an arrow an the end of an line with the given angle.
Copy link
Member

Choose a reason for hiding this comment

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

There are two typos: "an the end of an" -> "at the end of a"

@@ -491,8 +495,13 @@ class EdgeBase {
ctx.fillStyle = ctx.strokeStyle;
ctx.lineWidth = this.getLineWidth(selected, hover);

// draw arrow at the end of the line
ctx.arrow(arrowData.point.x, arrowData.point.y, arrowData.angle, arrowData.length);
if (arrowData.type && arrowData.type.toLowerCase() === 'circle') {
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: Should we better have something like an enum type holding the different edge types (circle, arrow, ...)? Then the types would be stored in one place rather than distributed through the whole code as strings. However, for now this isn't a problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good Idea! Let'd do that if more edge types are implemented.

Copy link
Contributor

Choose a reason for hiding this comment

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

agree

@mojoaxel
Copy link
Member Author

@yotamberk Could you please have a look at this!? I consider this a basis to implement further edge-types and not a completed new feature.

Copy link
Contributor

@yotamberk yotamberk left a comment

Choose a reason for hiding this comment

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

This is a great enhancement!

@yotamberk yotamberk merged commit f99d204 into almende:develop Oct 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants