-
-
Notifications
You must be signed in to change notification settings - Fork 994
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
Add a dedicated render method #53
Comments
I see why it would be useful - as I understand it you'd be looking to have a way to render the tree only from an array. The problem, as I believe I've mentioned other times in similar issues, is that this library doesn't have a default "style" for the blocks (if you ignore the demo, which is meant to be a proof of concept). What I mean by this, is for example, maybe somebody wants to implement this library in such a way that blocks contain GIFs or images in them, or even custom elements such as dropdowns, text inputs, etc. Without storing the HTML, those elements would be lost in the process. Obviously if the blocks all followed a pattern, and you were able to generate a block using only its content / ID, then you wouldn't need the HTML. But I fail to see how it would be possible to allow for all sorts of customization without the HTML - you would if anything still need to store the content of each individual block, if you didn't store the whole canvas, but I'm not sure if that's the sort of solution you're looking for? I would be keen to implement a better method both for outputting and importing / rendering on demand, so if you have a better idea on how to go about it, I would appreciate it!
I believe I added this at the request of #14, as a way to allow for custom logic. Aside from that it's also helpful to recognize what element in the DOM represents a certain block in the array. I suppose if you removed the need for HTML as I explained before, then a render function wouldn't require that since it would be dissociated from the DOM.
Right, I assume that is because of adding new methods which "breaks" the order of the methods passed to flowy(). I will look into it, I have to admit I am not very happy with the current initialization method - while passing the canvas is obviously necessary, the way the other optional functions are passed, successively, makes it harder to initialize it in a very custom way (e.g. only with onRelease and onRearrange methods). So I think what you're proposing makes sense. |
@alyssaxuu Thank you for your quick reply.
Lets say that a block contain a gif, or text input etc. Wouldn't that depends on the data you pass when rendering a specific block (and thus the developers should also store this data for later re-rendering) That said, we can still employ a hybrid version: pass the html but make it optional. If the html is present, we can use it as a quick way to setup the tree. That way, things work just like before for people who prefer to store the whole tree html.
PS: I'm working with flowy on the preset that both draggable blocks and the tree blocks are stored and queried from a relational database (which I think is true for many use cases). That is the reason why I emphasize the need of standardizing data structure and the possibility of re-rendering everything from available data. Lets take https://tray.io/ as a very good example for the use case of flowy. The Connectors are draggable blocks in this case.
|
I just don't see how the blocks would be re-drawn only using the data from the array. Keep in mind this is a method that I have to create - as in, being able to redraw the whole tree with the blocks and arrows from a single array. Because of that, I need something that tells me how the blocks will be styled. So for example if I have an array and a block contains certain data regarding the logic, how am I supposed to generate that block via a render method? Should I add a new method to individually render the blocks, so e.g. you receive the block data (for example, a specific ID + logic that is linked to a certain block type in your database and furthermore a template) and you return the HTML based on the templates you've created for your application? This is the current blocker. |
I was working on some ideas regarding this as well. For me there would need to be a way to store and retrieve flows to and from a database, also it would be nice to separate structure and design. This afternoon I had a spare hour and created the following:
Just tested now with a simple flow (only three blocks), but it seems to work. Draws the flow and arrows. It would offer the option to perhaps export as you do currently or in json and also import in html or json, making sure that the library is still simple to use (without needing a database). I would like to work on some more ideas:
Can share my changes if that is of interest. |
@arnoldligtvoet please do, always good to see how it is done. |
This is how I see it: we always have 2 types of data:
When we import the tree, both data can be passed to the import, or alternatively the "tree blocks" data should contain the corresponding "draggable data". Lets take your demo code for example: https://github.com/alyssaxuu/flowy/blob/master/demo/main.js On your demo, you have 3 main sections: the left draggable menu, the center flow tree, and the right configuration menu. Lets ignore the left and the right as they are not important for now. Everything inside the snapping function relies on "value" which is essential the data of the "draggable blocks". The other functions are really related to the rendering of the tree as I see it. If you do have an example when the above data is not enough for us to redraw the tree please let me know. Perhaps I missed something? |
If exporting without HTML it should be added as an optional flag so it will be backwards compatible |
@fcnyp certainly, it should be easy to check for the present of HTML option. |
@alyssaxuu I think the burden of the rendering is on the developer who uses this library. The most difficult part for me as a developer was to draw the tree with all the svg connectors etc to be honest. Going back to your demo, I think you have lots of if/else to render the block anyway: The only different now, is to move all that to a render function. Lets assume that we have this render function:
|
In fact, we can be smart and make it even shorter like this:
|
I see. So as I understand it, the developer would use a "render" method (not what you're describing, something like flowy.render(blocks)) to pass an array of blocks first, and then they would use a callback (which I assume is what you're describing) that would be send back the information of each block individually with all of its data, so the developer can then return the HTML. Seems reasonable enough. I assume since this would allow for the blocks to be different dimensions than they originally were, the entire tree would have to be recalculated as well, so that would allow flexibility in regards to changing the design of a product while still being compatible with old data. I think it is pretty straightforward now - I will definitely work on implementing something like this. |
@alyssaxuu That sounds great. If I may, I would like to suggest the following approach:
This is a BREAKING CHANGE, but I think it's necessary because:
|
@alyssaxuu if you need any additional hand to work on the new changes please do let us know. We can create a new branch to work on the new features to keep the master branch stable. I look forward to the changes |
@alyssaxuu I found this library which can give us many ideas: https://github.com/kieler/elkjs The tree structure is well thought-out:
I would convert it to:
Also, the rendering is done via a worker (neat, not necessary for now but is super neat for future performance improvement) |
Maybe would be useful to define block constructor function as input parameter. interface IBlock {
id: number; // or even better - string, to store UUID.
// other block data
}
flowy(canvas, { blockConstructor: (block: IBlock) => 'block html' }); |
Would be nice, but not 100% mandatory. You can get around the issue with javascript, albeit not the best looking code but still work. |
may be you can try. |
I understand that this request has been posted and closed here: #40
However, I would like to bring it up and present some reasons why I think it will be very useful:
And while we are at it, I would also argue that we should NOT pass the attr together with the blocks (at least not to the render function, because render function only expects the block information and data which can be passed manually).
I do understand why we have attr: right now we have the draggable blocks initialized as dom elements which is good but at the same time limited.
We should not mix dom objects with block objects. A block object should have:
{ "id": 1, "parent": 0, "data": {"name": "blockid", "value": "1" } }
For the html dom based draggable blocks, they can still be coded like this:
<div class="create-flowy" data-id="1" data-parent="0" data-data='{"name": "blockid", "value": "1"}'>Grab me</div>
or perhaps:
<div class="create-flowy" data-id="1" data-parent="0" data-name="blockid" data-value="value">Grab me</div>
One last thing is regarding the current api, perhaps if we switch to this it will be easier to allow easier change in the future:
flowy(canvas, ongrab, onrelease, onsnap, onrearrange, spacing_x, spacing_y);
to
flowy({canvas: element, onGrab: function, onRelease: function, onSnap: function, onRearrange: function, render: function, spacingX: number, spacingY: number});
or (only canvas is a must, all other things are optional)
flowy(canvas, {onGrab: function, onRelease: function, onSnap: function, onRearrange: function, render: function, spacingX: number, spacingY: number});
The text was updated successfully, but these errors were encountered: