felixhayashi / TW5-TiddlyMap

Map drawing and topic visualization for your wiki
http://tiddlymap.org
BSD 2-Clause "Simplified" License
837 stars 126 forks source link

Bug - Static HTML export of tiddler with embedded graph #176

Closed erlanger closed 8 years ago

erlanger commented 8 years ago

An export to Static HTML of a tiddler with an embedded view fails with the following error:

Internal JavaScript Error

Well, this is embarrassing. It is recommended that 
you restart TiddlyWiki by refreshing your browser

Uncaught TypeError: Cannot set property parentNode of #<Node> which has only a getter
erlanger commented 8 years ago

The best way for static HTML export would probably be to export the graph as an SVG graph and embed it in the html. SVG is better than png because it scales better and it uses less space than a png.

felixhayashi commented 8 years ago

An export to Static HTML of a tiddler with an embedded view fails with the following error:

Never did that before :) I'll have a look at that. Thanks for telling.

The best way for static HTML export would probably be to export the graph as an SVG graph and embed it in the html. SVG is better than png because it scales better and it uses less space than a png.

The problem is that visjs draws on a canvas and doesn't create an svg. An html canvas is plain 2d and cannot be decomposed into svg so while I agree that an svg would be nice, it is not possible :(

tobibeer commented 8 years ago

Would be amazing to see TiddlyMap output as an image with static rendering. It would be awesome² if that tiddler contained an image map-that allowed to click the nodes... but this is possibly way out there, in terms of figuring out the bounding boxes.

felixhayashi commented 8 years ago

@tobibeer

Would be amazing to see TiddlyMap output as an image with static rendering.

While it is possible, that would be too much work for me to implement that now :D

But you can always create a static snapshot of the map… But I know this is not what you had in mind ;)

2015-10-06 15 44 08

tobibeer commented 8 years ago

That is a-ok, the only problem being that any static rendering process would need a reliable place as to where to find that image so as to include it.

felixhayashi commented 8 years ago

Sorry to bother you @Jermolene but I got a small problem with the fakedom passed by TW when exporting tiddlers :(

When in TiddlyWiki and exporting a bunch of tiddlers (some containing the TiddlyMap widget) as static HTML then parent.insertBefore throws an error as described in the OP by @erlanger.

2015-10-06 17 02 27

This happens if I receive a TW_Element fake dom element and try to insert/append a node that was created via document.createElement instead of using this.document to create it since the element created is not of Type TW_Element then.

MapWidget.prototype.render = function(parent, nextSibling) {

  this.parentDomNode = parent;
  this.domNode = document.createElement("div");
  parent.insertBefore(this.domNode, nextSibling); // <--- this line trows the exception!
  …

}

I now corrected the widget code above and use this.document instead of document.

However, the Visjs lib I am loading is always using document.createElement and thus will always produce errors when calling x.appendChild(document.createElement("div")) (where x is of type TW_Element).

So my question: Am I correct that, when operating in fakedom mode, my only option chance to avoid a crash is to render a placeholder instead of loading the visjs lib?

Thanks in advance for your expertise.

-Felix

erlanger commented 8 years ago

@felixhayashi Thanks for looking at this so promptly! The TW5 community is amazing. If you can't render an SVG, then it probably would be possible to use the PNG export function

@tobibeer @felixhayashi Can you use the the tw.utils.makedatauri (sp?) function to make a static data uri to embed the PNG in the static HTML page?

tobibeer commented 8 years ago

@erlanger

Can you use the the tw.utils.makedatauri (sp?) function to make a static data uri to embed the PNG in the static HTML page?

that sure sounds like a great idea for any tmap specific static-render-handling to take care of ...not sure if @felixhayashi is able to hook into process that eventually yields the save as png dialog. This line here suggests that he just might as he's doing exactly that already...

https://github.com/felixhayashi/TW5-TiddlyMap/blob/6358ab26fcbc5ae0aa2cda9cee1ddb35a4d632c8/jsdoc/widget.map.js.html#L1324

felixhayashi commented 8 years ago

Hi @tobibeer and @erlanger

@felixhayashi Thanks for looking at this so promptly!

No problem :)

If you can't render an SVG, then it probably would be possible to use the PNG export function

At the moment, I am not sure whether I can render a png during export because visjs doesn't seem to work in a fakedom environment that TW creates during export.

But I'll see if I can try to make a png map-screenshot in advance (at some point when the graph was loaded the last time) and load this png during export.

Can you use the the tw.utils.makedatauri (sp?) function to make a static data uri to embed the PNG in the static HTML page?

The problem is not how to insert/embed the png but how to create the actual canvas so I can take a snapshot image ;)

..not sure if @felixhayashi is able to hook into process that eventually yields the save as png dialog.

In general, creation of an png is not a problem but unfortunately it is not possible during the export for the reasons described above (https://github.com/felixhayashi/TW5-TiddlyMap/issues/176#issuecomment-145917305).

-Felix

Jermolene commented 8 years ago

Hi @felixhayashi

Firstly, the crash you're seeing is actually inside the fakedom implementation. The problem is triggered because you're calling TW_Element.insertBefore(this.domNode, nextSibling) with nextSibling not being a child of the specified parent node.

The problem with external libraries hardwiring themselves to the global "document" is a bit of a nuisance. In several cases I've had to wrap libraries with a header and footer to take care of things:

https://github.com/Jermolene/TiddlyWiki5/blob/master/plugins/tiddlywiki/katex/files/tiddlywiki.files#L136-L137 https://github.com/Jermolene/TiddlyWiki5/blob/master/plugins/tiddlywiki/railroad/files/tiddlywiki.files#L17-L18

It's not quite the same, but there's something similar happening here:

https://github.com/Jermolene/TiddlyWiki5/blob/master/plugins/tiddlywiki/jasmine/files/tiddlywiki.files#L25

felixhayashi commented 8 years ago

@Jermolene thanks for responding here!

The problem with external libraries hardwiring themselves to the global "document" is a bit of a nuisance. In several cases I've had to wrap libraries with a header and footer to take care of things:

That is a great idea, I didn't think of overriding the global document object during library import. Thanks!

Firstly, the crash you're seeing is actually inside the fakedom implementation. The problem is triggered because you're calling TW_Element.insertBefore(this.domNode, nextSibling) with nextSibling not being a child of the specified parent node.

Ehm, but nextSibling is passed to me by the parent widget so it should be either null or also a fakedom element or not? Also, in the code above, if I use this.document.createElement() to create this.domNode everything works fine (despite vis.js crashing later on). Or maybe I misunderstood you?

-Felix

Jermolene commented 8 years ago

@felixhayashi the problem in the original code sample above was the mixing of the fake dom and the real dom. You fixed that issue with this.document.createElement(), leaving the problem with vis.js still using the wrong DOM.

felixhayashi commented 8 years ago

Thanks again @Jermolene! Now it is clear.

felixhayashi commented 8 years ago

@erlanger

I now realized that I cannot calculate the snapshot images during export so in any case I need to do it at some point before. However, the only chance of taking a good snapshot of the canvas is when the a is loaded for the first time, i.e. when you open a view or a widget is displayed. I could save a screenshot then and later transclude it, but this comes with two drawbacks.

  1. Your wiki will store one snapshot image for every view you have which will use some of your wiki space (5 views = 5 images). The size of the image would depend on the canvas size that was active when taking the screenshot.
  2. Snapshot images may not be up-to-date since the snapshot would need to be taken at the beginning when opening a view. So all view manipulations after opening it will not be reflected in the snapshot. Also, when a view hasn't been opened for a while but styles have somehow changed, these may not be reflected unless you opened the view before export so a new snapshot is taken…

Under these circumstances, would you still think this snapshot feature makes sense? In any case, I would make this feature optional.

Alternatively, I would just display a non-image placeholder.

-Felix

erlanger commented 8 years ago

I now realized that I cannot calculate the snapshot images during export so in any case I need to do it at some point before.

@felixhayashi - I see, this is because after export is started the fakedom doesn't have enough info to render a canvas right?; If this is the case there may be a better work around.

  1. Insert a new button under the top more action (down arrow) button (maybe next to the "export all"). It could be called something like 'Export HTML with maps'
  2. This button is totally under your control, then you can:
    1. Open all the tiddlers that have maps, render the canvas and save to a png
    2. Store the png datauri in a field of the tiddlers with maps
    3. Call the original 'Static HTML export' function, and use the embedded datauri to render the static HTML
    4. Delete the png field in the tiddlers with maps and close the tiddlers after the Static HTML export returns

What do you think? PS. @Jermolene is there a way to execute a function before the Static HTML code is called? PSS. The 'Export HTML with maps' button is can be a configurable option in the control panel

Jermolene commented 8 years ago

Hi @felixhayashi

PS. @Jermolene is there a way to execute a function before the Static HTML code is called?

Not at the moment, no.

Wouldn't the best approach be for your widget to generate a PNG when invoked to generate static output? In the browser you could still render the graph to canvas, and then take a snapshot of it. Under Node.js you could use this:

https://github.com/Automattic/node-canvas

You'd need to use a variable so that the widget can tell whether it is being used to generate static output. We can update the core static templates to provide a standard variable indicating static output.

tobibeer commented 8 years ago

We can update the core static templates to provide a standard variable indicating static output.

Perhaps add it to the list here: https://github.com/Jermolene/TiddlyWiki5/issues/1977 ...of things called maybe "environment status tiddlers".

felixhayashi commented 8 years ago

@erlanger

I now realized that I cannot calculate the snapshot images during export so in any case I need to do it at some point before.

@felixhayashi - I see, this is because after export is started the fakedom doesn't have enough info to render a canvas right?;

No, this is actually fine, I got all the information I need. The original problem is/was that visjs doesn't work during export. Jeremy provided a workaround for that that I haven't tested yet.

However I realized the bigger problem is that visjs needs time to calculate graphs (sometimes 5 secondes for very big graphs) and does this in an asynchronous manner. So, while your described steps make sense, I cannot simply say "render tiddlers with their widgets and then extract png images and also visjs". In any case, a solution would need to involve adding various callbacks which is also a bit difficult here. Also what you suggested, adding a "prepare" function that TW invokes before export would not change that. But I appreciate your input.

@Jermolene

Wouldn't the best approach be for your widget to generate a PNG when invoked to generate static output? In the browser you could still render the graph to canvas, and then take a snapshot of it.

I think from TW's side everything is fine. Thanks for all your suggestions and your time. It is just too complicated to generate map snapshot images programmatically on demand during export due to the delayed, asynchronous rendering of vis :(

@Jermolene (@tobibeer)

It is also not necessary to add a variable indicating whether we are running from a command line just for TiddlyMap's sake because I do not plan to support static TiddlyMap output ouside the browser. But I appreciate the offer and I am not saying an environment variable doesn't make sense and if you want to introduce it anyway, please go ahead.

@erlanger

I'll now push a TiddlyMap update that fixes the error message and then continue to think about how the snapshot image thingy could be done.

erlanger commented 8 years ago

Thanks! I appreciate you continuing to think about how to take the snapshot image!

tobibeer commented 8 years ago

I must admit, while I wouldn't want errors thrown at me, I did not have any expectations for automagically created static output images.

So anything that allows someone wanting to...

  1. generate the static images => is working, manually, in the browser
  2. reference them upon static rendering => is the missing link

...would possibly be happy about having anything to show, even if not the most recent version. Some "noscript", "noframe", well "novis" kinda fallback content via transclusion... of, perhaps, an image or more, perhaps maintained under some system namespace, e.g.

$:/some/where/tmap/static/map-123

...which users could create as fallback references and which tmap picks up during static rendering. Nothing highly automated on the content creation side but at least not a blank screen either.

There could also be a catch-all fallback like...

$:/some/where/tmap/static-default

...shown for any tmap, unless it has the above individual static output defined.

Well, if anything, the only automation one could have happen would take place within a browser environment. If anything, this should be a plugin-extension to tmap, so nothing that ships out of the box, e.g.

The actual images maybe put under...

$:/some/where/tmap/static-png/map-123

In other words, even I or @erlanger could create that static-export-wizard thingy... but pulling that stuff in during static rendering requires modification to tmap itself.

So, @felixhayashi, the thing that you could provide is the mechanism that picks up those static fallback transclusions, while you could leave it to everyone else to get that content there. Not exactly sure based on what to reference individual maps as there can be any number of maps in a tiddler. Perhaps it requires some hard-wired ids (mabe the "view" title?).

You could just have one config tiddler called static namespace where you hold the root namespace of where you expect to pick-up the fallback tiddlers, e.g.:

$:/some/where/tmap/static

tobibeer commented 8 years ago

However, if you never used the inner content of the widget yet, you could scratch all of the above and take that content as the fallback!

<$tiddlymap view="All Tasks" height="250px">
static fallback content here!
</$tiddlymap>

...and entirely leave it to the user to fill that as needed. Seems the slimmest of all options for you and us to handle gracefully.

felixhayashi commented 8 years ago

@tobibeer Much thanks for this detailed description of what you would expect, helps me a lot!

So, @felixhayashi, the thing that you could provide is the mechanism that picks up those static fallback transclusions, while you could leave it to everyone else to get that content there.

I already implemented this as a hidden feature ;) If you place a snapshot tiddler below a view's path ($:/plugins/felixhayashi/tiddlymap/graph/views/VIEWNAME/snapshot), this tiddler will be transcluded in the TMap's canvas area in static mode. So this tiddler may be an image or a tiddler of another type…

But I'll make this whole process more user friendly with #178. Probably release the next version soon or at least release a preview version.

<$tiddlymap view="All Tasks" height="250px">static output here!</$tiddlymap>

Also a nice idea, indeed! I never thought about using the widget's body as fallback for this. But I'll reserve this for the moment. The widget body is a valuable place and maybe there is even something better to do with it in the future ;)

@erlanger

Thanks! I appreciate you continuing to think about how to take the snapshot image!

No problem, it makes TM better so it is worth thinking about it ;)

felixhayashi commented 8 years ago

I'll close this now. For further discussions: #178