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

Circle Packing #117

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Circle Packing #117

wants to merge 29 commits into from

Conversation

houxinli
Copy link
Collaborator

@houxinli houxinli commented Sep 10, 2019

Circle Packing

list of modifications:

this is a demo using the word frequency data set:
Alt Text
This is the first pull request of hierarchical templates, and it was derived from a very old code base, many files have been changed. Thank you for any comments, criticism and discussion!

@tracyhenry
Copy link
Owner

Thanks Xinli.

A quick question: does this include treemap? Also, can you include in the PR body a GIF of the template so that everyone knows what it looks like? I find this website to be useful to upload gifs generate external links: https://giphy.com/

In any case, we need to find some time to sit down and have all these three templates merged (pie, treemap, circle packing, anything else?)

@tracyhenry
Copy link
Owner

feature request: add option to opt out breadcrumb

Copy link
Owner

@tracyhenry tracyhenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR Xinli. See my inline comments.
Sorry that there are a lot, but majority of them are minor/my questions.

front-end/js/globalVar.js Show resolved Hide resolved
front-end/js/globalVar.js Outdated Show resolved Hide resolved
front-end/js/globalVar.js Outdated Show resolved Hide resolved
front-end/js/embed/README.md Outdated Show resolved Hide resolved
front-end/js/dynamicLayers.js Outdated Show resolved Hide resolved
compiler/src/template-api/CirclePacking.js Show resolved Hide resolved
compiler/src/template-api/CirclePacking.js Outdated Show resolved Hide resolved
back-end/src/main/java/server/CanvasRequestHandler.java Outdated Show resolved Hide resolved
back-end/src/main/java/third_party/Exclude.java Outdated Show resolved Hide resolved
@tracyhenry
Copy link
Owner

Can you also check that your Java line endings are correct. It seems like some of them are using CRLF

Copy link
Owner

@tracyhenry tracyhenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two additional comments

@@ -0,0 +1,79 @@
package project;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: test if this can be moved to an indexer file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nashorn failed even though PackNode is in the same file. Seems that Nashorn only supports public classes

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one way is not passing ArrayList<PackNode> into nashorn...

Serialize packnodes to string before hand, then deserialize in packSib...

back-end/src/main/java/index/PsqlCirclePackingIndexer.java Outdated Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

svg.getBBox() ignores transforms when static layer semantic zoom
2 participants