search

iVis-at-Bilkent / cose-base

Core module for compound spring embedder based layout styles
MIT License
9 stars 2 forks source link

Added CoSEConstants.TILING_COMPARE_BY #9

Closed metincansiper closed 3 years ago

metincansiper commented 3 years ago

Works with https://github.com/cytoscape/cytoscape.js-cose-bilkent/pull/116

hasanbalci commented 3 years ago

Hi @metincansiper, I liked this option since it will give more flexibility to users to sort the nodes to be tiled. I will review the changes in detail, but can you please make them in unstable branches?

metincansiper commented 3 years ago

Hi @hasanbalci, thanks! I realized that there would be one detail that I would better revise and update if needed. I will make it and make the PR's to unstable branch after that.

metincansiper commented 3 years ago

@hasanbalci I updated the base branches of this PR and the one related to cytoscape.js-cose-bilkent repository (https://github.com/cytoscape/cytoscape.js-cose-bilkent/pull/116) as unstable.

However, I realized something in this repository. It happens both the branch in my fork after making it based on unstable and the original unstable branch of this repository. I will report the output from the unstable branch of this repository:

When I use the node version 13.6.0 at the build step (npm run build) I see the following error: (node:34424) UnhandledPromiseRejectionWarning: Error: No valid exports main found for '/Users/siperm/Documents/Workspace/cose-base/node_modules/colorette'.

Then I updated my node version as 15.13.0 and the build was successful but I see that the bundled cose-base.js file is produced as minified. However, the existing cose-base.js file is not minified in the unstable branch.

Then I tried the node version 8.2.1 and at the build step I see the error of SyntaxError: Unexpected token ....

Is there a specific version of node that must be used?

hasanbalci commented 3 years ago

@metincansiper It seems that we updated webpack version, but didn't configure webpack.config.js accordingly. I use node version 13.10.1 and it didn't give error but minified the cose-base.js file. Thanks for reporting this. I will make the required changes after I reviewed and merged your PR.

metincansiper commented 3 years ago

@hasanbalci I added my responses/questions below your comments.

hasanbalci commented 3 years ago

@metincansiper I can't see your responses.

metincansiper commented 3 years ago

@hasanbalci sorry I forgot to submit it. I think it must be visible now.

metincansiper commented 3 years ago

@hasanbalci I made the updates that we have discussed. BTW as we discussed before the auto generated cose-base.js file ended up being minimized in this commit.