the-graph icon indicating copy to clipboard operation
the-graph copied to clipboard

width and height leaking out to json

Open forresto opened this issue 10 years ago • 6 comments

width and height shouldn't be saved out to node metadata: should only be used internally. Ref #208

forresto avatar Apr 16 '15 13:04 forresto

Hi, coming back to this issue...can you explain why width/height shouldn't be saved out to node metadata? We're using the-graph as the foundation of another editor and we want to be able to change, and save, the different width/heights, and so do need access to them, which is easily done using the metadata - it'd be a pita to have to dig them out in some other way.

davidmaxwaterman avatar May 11 '15 07:05 davidmaxwaterman

For now it shouldn't be saved out because it isn't read in.

We won't strip metadata out, so your use case shouldn't be a problem.

forresto avatar May 11 '15 14:05 forresto

I don't quite get what you mean by 'read in'. Our editor can save and read graphs to/from fbp files, and it seems to reproduce the previous state perfectly well.

Are you saying that the width/height are somehow ignored or overwritten? (It's possible I've not noticed this since)

Yes, you don't strip metadata out, but isn't this very issue all about 'stripping out' this metadata? I suppose I'm arguing against this being done.

davidmaxwaterman avatar May 11 '15 14:05 davidmaxwaterman

the-graph ignores dimension metadata that comes in. The way that it leaks out its a bug... somewhere I'm saving to the noflo metadata object, which should be read-only.

forresto avatar May 11 '15 15:05 forresto

OK, but could I suggest that it shouldn't ignore it, and does actually export it as metadata? This would allow us to more easily implement project save/open functionality, which is what we're trying to do.

Actually, I just test it, and the-graph does not ignore the width and height when importing...I set a breakpoint in the callback to window.noflo.graph.loadJSON(), and manually changed the values before our code assigns it to the-graph-editor.graph, and the values had the desired effect - ie it was short and fat :)

So, I suppose I still question this issue, and think it should be left 'as is'. I suppose I could be missing something...perhaps it doesn't make much sense for the data to be part of an FBP file? We have both JSON export (as a 'project', into which we add other project 'state' stuff), and FBP export (which is so it can be imported by other tools).

Thoughts?

davidmaxwaterman avatar May 11 '15 15:05 davidmaxwaterman

I think the-graph should instead respect these parameters when coming from the JSON file. That way one can import (and roundtrip) files with them looking the same.

jonnor avatar Sep 20 '16 12:09 jonnor