kieler / elkjs

ELK's layout algorithms for JavaScript
Other
1.79k stars 97 forks source link

Improve Types/Docs #113

Open amcdnl opened 3 years ago

amcdnl commented 3 years ago

Request

The documentation is quite difficult to use - even after building an entire project with it I'm still unsure of many of the options I have available. I find myself quite a bit searching Github code repos for combinations along with digging through source code. The docs for the Java project are not very helpful either since you have to guess namespaces/etc how they translate.

I think improving the types would also be very beneficial as they are not even complete and cause conflicts when using props not on them.

I would be happy to help make this happen but I'm not quite equipped to help guide the way. Maybe if someone on the team could kick this off and the community can help follow.

Suggestion

Here is a list of options I currently use but I'm not even sure what some of them do besides adding/removing them to see different layouts.

const options = {
  'elk.nodeLabels.placement': 'INSIDE V_CENTER H_RIGHT',
  'elk.algorithm': 'org.eclipse.elk.layered',
  'elk.direction': 'DOWN',
  nodeLayering: 'INTERACTIVE',
  'org.eclipse.elk.edgeRouting': 'ORTHOGONAL',
  'elk.layered.unnecessaryBendpoints': 'true',
  'elk.layered.spacing.edgeNodeBetweenLayers': '50',
  'org.eclipse.elk.layered.nodePlacement.bk.fixedAlignment': 'BALANCED',
  'org.eclipse.elk.layered.cycleBreaking.strategy': 'DEPTH_FIRST',
  'org.eclipse.elk.insideSelfLoops.activate': 'true',
  separateConnectedComponents: 'false',
  'spacing.componentComponent': '70',
  spacing: '75',
  'spacing.nodeNodeBetweenLayers': '70'
};

I think it would also be very helpful if they were real objects rather than dot notation props. I could imagine the above to be something like:

export interface ElkLayoutOptions {
   spacing: number | { nodeNodeBetweenLayers: number; };
   node: {
       layering: NodeLayeringType;
       label: { 
           placement: PlacementType;
       };
   };
};

You could very easily use a flattening npm package to turn those objects into dot notation props automatically.

uruuru commented 3 years ago

I'm not going to comment further on the state of the documentation :). I agree that it's hard to find the right options and that explanations could be improved. Our try to automate as much as possible with the given capacities we had, resulted in the layout option reference.

Regarding typescript types: I believe your suggestion would be a valuable improvement (as an alternative to the string-based option names). For it to be maintainable, I think the type defs must be generated automatically. Also, the json graph parser must be made aware of the fact that options can be nested objects. Hence, I see two tasks:

First, allow nested options. I believe, I created a ticket for this a while back (can't find it right now) or at least had this in the back of my head. This functionality must be added to the JsonImporter#L404.

Second, the typescript definitions must be generated. We already generate the Java Code for the layout options based on *.melk files. See Layered.melk for an example. The actual code generation happens in the core.meta plugin in the MetaDataJvmModelInferrer.xtend and the MelkDocumentationGenerator.xtend I believe it's rarther hard to understand how an actual file is generated in the first place (independent of the actual code generation). Maybe somebody familiar with it could provide a skeleton one can start with. I cannot say if anybody finds time at this point though.

amcdnl commented 3 years ago

@uruuru - understand completely.

I've done a LOT of graphs and this is BY FAR the best graph engine compared to libs like dagre, however, due to the issues with the docs its very difficult to use which would really hurt adoption.

Sadly I have zero experience with Java so if the requirement is dynamically generated based on that I won't be able to help. Wish there was something I could do to help outside of that though.

le-cds commented 3 years ago

If you guys figure out what the TypeScript files will have to look like, I'm happy to help figure out how to auto-generate them. 🙂

Vadorequest commented 3 years ago

I'm also lost in the documentation.

@amcdnl In your example, you define

  'elk.layered.spacing.edgeNodeBetweenLayers': '50',
  'org.eclipse.elk.layered.nodePlacement.bk.fixedAlignment': 'BALANCED',

Why does it use org.eclipse. only on the second line? I'd assume the first line would also need it.

Right now, I'm kinda throwing options randomly and see if they have any effect, I wish there was a proper TS type for the defaultLayoutOptions, it'd definitely make things much easier.

uruuru commented 3 years ago

@Vadorequest the front part of a layout option identifier can be omitted as long as the remaining part is unique.