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

Rendering Order #17

Closed
josephmturner opened this issue Aug 11, 2022 · 5 comments
Closed

Rendering Order #17

josephmturner opened this issue Aug 11, 2022 · 5 comments

Comments

@josephmturner
Copy link
Contributor

Hello! Thank you for dagre-reactjs - it has made a big difference in our project .

In our use case, nodes should render last so that they display on top of edges and markers. The change is as simple as moving

{nodes.map((node, index) => {
return renderNodeFunc(
node,
this.reportNodeSize.bind(this, index),
this.valueCache,
this.props.layoutStage
);
})}
down to the end of the group:
<g>
{nodes.map((node, index) => {
return renderNodeFunc(
node,
this.reportNodeSize.bind(this, index),
this.valueCache,
this.props.layoutStage
);
})}
{edges.map((edgeMeta, index) => {
return renderEdgeFunc(index, edgeMeta);
})}
{edges.map((edgeMeta, index) => {
return renderEdgeLabelFunc(
index,
edgeMeta,
this.reportEdgeLabelSize.bind(this, index)
);
})}
</g>

Would you be open to a PR which adds an optional attribute to the graphOptions prop which controls the rendering order? Something like renderingOrder, an array of strings like ['nodes', 'edges', edgeLabels'] (current behavior), ['edges', 'edgeLabels', 'nodes'] (what I need), or some other iteration?

@josephmturner
Copy link
Contributor Author

It might be more appropriate to pass renderingOrder directly to DagreReact, not inside graphOptions. Let me know what you think!

@bobthekingofegypt
Copy link
Owner

Sure, happy to take a PR for this.

Just browsing the repo I realise I left the branches in a bit of a mess by never releasing version 1.0 final. I should really do that at some point, but the latest version of the code is on the alpha branch (1.0.0-alpha.2 on npm). If you are going to make changes I'd make them against that starting point.

Is your project working against 1.0.0-alpha.2 or the older 0.2.0 release?

@josephmturner
Copy link
Contributor Author

Thank you! We are using the npm version 1.0.0-alpha.2. I'll be sure to make changes against that version.

@bobthekingofegypt
Copy link
Owner

Thanks for that.

I will try push out another alpha release to make sure I can get a release working and then later I'll push out the current version as version 1.0 final.

@josephmturner
Copy link
Contributor Author

Splendid! Thank you for your prompt response and merge!!

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

No branches or pull requests

2 participants