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

Remove fixed width/height of 500px from decision graph svg #276

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from

Conversation

chbrown
Copy link

@chbrown chbrown commented Apr 5, 2017

The tutorial's decision graph page, https://clojure-liberator.github.io/liberator/tutorial/decision-graph.html, links to the raw SVG image at https://clojure-liberator.github.io/liberator/assets/img/decision-graph.svg, but due to the <svg> element's fixed dimensions, is very small, and no easier to navigate than the embedded version.

Since the container (a <span> with display: block!) has fixed dimensions, this does not overflow in the constrained space of the tutorial, as you can see at https://chbrown.github.io/liberator/tutorial/decision-graph (this patch-1 branch is already merged into the gh-pages branch in my fork)

Compare https://chbrown.github.io/liberator/assets/img/decision-graph.svg, which, in my opinion, is easier to explore.

This makes it default to full size when viewed alone, but does not change the appearance of the embedding in the tutorial.
@ordnungswidrig
Copy link
Member

Hey Christopher, thank you for the contribution. Did you check if the updated graph also works correctly in the debug trace view?

@chbrown
Copy link
Author

chbrown commented Apr 6, 2017

I hadn't come across the debug trace view; the first time I tried (wrap-trace :header :ui) it didn't work (Exception in thread "main" java.io.FileNotFoundException: Could not locate compojure/core__init.class or compojure/core.clj on classpath., compiling:(liberator/dev.clj:1:1)) because it assumed I was using compojure but I'm not using compojure.

So I added compojure to my dev dependencies and hey, wrap-trace is really handy!

And it appears that the debug trace uses an entirely different svg, specifically, https://github.com/clojure-liberator/liberator/blob/master/src/liberator/trace.svg

@ordnungswidrig
Copy link
Member

Yes, sorry it does. Although they should be in sync preferrably.

@cch1
Copy link
Contributor

cch1 commented Oct 24, 2017

It would be great to have this resolved. @chbrown , any chance you can push this to completion?

@chbrown
Copy link
Author

chbrown commented Oct 24, 2017

Thanks for your reminder, @cch1; the change I made works just fine (i.e., the updated graph does also work correctly in the debug trace view). Presumably, the blocker — de-duplicating the different svg sources — is more of a build artifact, which depends on how @ordnungswidrig keeps the docs/gh-pages and master in sync.

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

Successfully merging this pull request may close these issues.

3 participants