New Added support for custom edge line style

[parmar-abhinav]

This PR introduces a new feature allowing users to have greater control over the appearance of edges in the graph by implementing custom edge line styles. User can set a Solid, Dashed, Dotted or even custom edges by defining own edge patterns.

[tonilastre]

This is repeated in all three types, but it is all the same code. Additionally, rendering should be super fast which means all unnecessary calls should be removed. For example, for each edge calling getEdgeLineStyle creates a new object just to figure out the correct style - when rendering has a min of 25 FPS, this will start to impact the process.

I would suggest removing getEdgeLineStyle - no need for it because you have the latest information in the this.style of the edge model.
I would suggest having new function on the edge model, something like getLineDashPattern that returns number[] | null. In case of solid or undefined lineStyle, it returns null. In case of any other, it returns a default one (for dashed or dotted) or a user-defined one (in case of custom).

Then this code (and in all three edge types) would be simple as:

const lineDashPattern = edge.getLineDashPattern();
context.setLineDashPatern(lineDashPattern ?? []);

[parmar-abhinav]

Thanks for your feedback. I've made the following changes to the code as you suggested:

• removed the getEdgeLineStyle() function, as you pointed out that it is unnecessary and creates a new object each time it is called.

• added a new function called getLineDashPattern() to the edge model. This function returns a number[] or null, depending on whether the edge has a custom line style or not.

• updated the code to use the getLineDashPattern() function instead of getEdgeLineStyle().

I believe these changes will make the code more efficient and improve the rendering performance.

[tonilastre]

Maybe you will need to move this closer to the models/edge - check the other comment on where the code around patterns should be.

[tonilastre]

Suggested change

	export const isArrayOfNumbers = (value: any): value is boolean => {
	export const isArrayOfNumbers = (value: any): value is number[] => {

In typeguards, this param should return the correct type. Typeguards return a boolean by default + this is for the Typescript to be able to know what the type is after typeguard returns true.

[tonilastre]

Suggested change

	const isNotNumber = value.some((element) => !isNumber(element));
	return !isNotNumber;
	return value.some((element) => !isNumber(element));

This is pretty much clear enough where you don't have any benefit from an additional variable. If this was a more complex function, then the named variable would be a good choice for better readability.

[tonilastre]

No need to have edge because you already have a context of EdgeStyle. I would suggest naming this lineStyle.

[parmar-abhinav]

Renamed it to lineStyle, pls check.

[tonilastre]

This should be fixed from a typescript side to be more precise. For example, currently, you can have { type: "solid", dashPattern: [1, 2, 3] } which is not correct. An array can only be used with custom type. E.g.

export type IEdgeLineStyle =
  | { type: EdgeLineStyleType.SOLID }
  | { type: EdgeLineStyleType.DASHED }
  | { type: EdgeLineStyleType.DOTTED }
  | { type: EdgeLineStyleType.CUSTOM, pattern: number[] };

or even

export type IEdgeLineStyle =
  | EdgeLineStyleType.SOLID
  | EdgeLineStyleType.DASHED
  | EdgeLineStyleType.DOTTED
  | { type: EdgeLineStyleType.CUSTOM, pattern: number[] };

Maybe the second one might even work better if users will use SOLID, DASHED or DOTTED more. The first one is easier to use from library point of view, the second one from user point of view.

[tonilastre]

Nitpicking but I am also thinking about renaming dashPattern to just pattern because you already have a context which is a line style. What do you think? Also, what do you think about those two proposals above, which one makes more sense to you?

[parmar-abhinav]

I agree with your suggestion to fix the TypeScript type for IEdgeLineStyle.

Yes The second approach might even work better if users will use SOLID, DASHED or DOTTED more. However, I am thinking to use the first approach because it gives users a more consistent and predictable experience. With the first approach, users know that they can always specify the line style using the same syntax, regardless of whether they are using a predefined style or a custom style.

[tonilastre]

Yep, agree with you. The first approach is slightly a better option because of the usage predictability.

[tonilastre]

Nothing to change here, but just a general comment that once node and edge style have a setter for styles setStyle (what 1.0.0 will require), this check will be done there because the check is done once then.