Librarify #18
Merged
Librarify #18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We shouldn't contaminate underscore with custom extensions.
Many things happen in this commit and they are all intermingled in ways they are hard to separate. - Start using Bower for dependencies, reason being to make nomnoml itself a bower package. - Upgrade Dagre from 0.4.5 to 0.7.1, reason being that the older version wasn't a bower package and didn't support RequireJS, the new version has both of these features. - Switch from using Underscore to using Lo-Dash, because Dagre depends on Lo-Dash it was just nicer if nomnoml switched to that as well. Known issues: - With the upgrade of Dagre the layout doesn't come out exactly the same. The upgrade also messes with the rendering of relation arrows.
Turns out that with the upgrade of Dagre, the path now already comes with the end points calculated so now nomnoml doesn't have to do that manually anymore.
Since Dagre is now providing the necessary points we can let the layout function stop adding the compartment position to the relation path since that was a work around for later figuring out the intersection point with the nodes involved.
Since the latest version (0.7.1) of Dagre caused so many undesireable effects, going back to the older version and embedding it within nomnoml instead seems like a better option. The size of `nomnoml.js` went from 40k to 80k as a result (note: Dagre istelf embeds graphlib).
|
@korroz very nice! would it be possible to leverage what you've done to create a CLI tool that outputs a SVG (or any image format really)? |
|
@dlobue thanks! Making nomnoml capable of outputting on a command line was exactly my ambition actually. However there is one rather major technical challenge, namely text measuring. Without a rendering context (such as a browser) getting the length of text is impossible. So even if the output would be SVG it seems like you still need something like a headless browser (like PhantomJS) to produce a layout. But yeah, it's something I will look at in the future. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal of this branch was to make the core functionality of nomnoml standalone in the form of a single javascript library file. When I started the core functionality was tightly coupled with the nomnoml web application so a lot of work went into refactoring dependencies. In the end this branch has these main features:
nomnoml.js.Lesser features of the project:
nomnoml.js. (npm)Another sort of outstanding issue is to register the repo in Bower so people can use the short form
nomnomlwhen installing the package instead of the full url to the github repo.The above are the major points. Code wise there have been some major restructuring that may be interesting to note, some of which:
_skanaar.vector.jshelper functions were pushed into the nomnoml namespace as the objectnomnoml.vectorMath.nomnoml.app.js, moving many functions and even splittingfitCanvasSize()into two.I also need to mention that while this is a good step, separating core from app there are still things I'm not particularly pleased with, so there are areas which could use further improvement.
And lastly, this should close issue #17.