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

Improve Types/Docs #113

Open
amcdnl opened this issue Dec 28, 2020 · 5 comments
Open

Improve Types/Docs #113

amcdnl opened this issue Dec 28, 2020 · 5 comments

Comments

@amcdnl
Copy link
Contributor

amcdnl commented Dec 28, 2020

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
Copy link
Member

uruuru commented Dec 29, 2020

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
Copy link
Contributor Author

amcdnl commented Dec 29, 2020

@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
Copy link
Member

le-cds commented Jan 4, 2021

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
Copy link
Contributor

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
Copy link
Member

uruuru commented Feb 10, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants