Open stalniy opened 4 years ago
implemented in my fork https://github.com/stalniy/cytoscape-node-html-label
This is very useful. Any chance it will be merged?
@stalniy Could you send a PR for this one too?
Frankly speaking, after implementing this and trying to work with this, I found out that it's a bit inconvinient.
In case of DOM node, we need 2 functions: 1 to init and create DOM element and another which is called when DOM element needs to be updated.
Otherwise you will need to implement some kind of memoization based on data.id
.
So, the question: how do we approach this issue?
tpl
an object with 2 methods: init
, update
tpl
a single function and force users to implement memoization by yourself (initial suggestion) I think that for starters, the library should support elements because if you use components (React / Vue / etc.) then this is a must while adding an update method could be done by the user.
Here's how I update (Vue):
const nodeTemplate = (n, parent, cyn) => {
if (n.el) {
// if the card already exists, just update it instead of creating a new one
const card = n.el.__vue__; // eslint-disable-line
card.toggleOpen(n.open);
card.updateClassName(cyn.classes());
return n.el;
}
const Card = Vue.extend(NodeCard);
const instance = new Card({
parent,
propsData: {
id: n.id,
title: n.label,
open: n.open,
items: n.children,
className: cyn.classes(),
},
});
instance.$mount();
return instance.$el;
};
.
.
cy.attachHtmlToNodes([{
query: 'node',
valignBox: 'bottom',
cssClass: 'fixVAlign',
tpl: (n) => {
// register the node before returning it for fast referencing
n.el = nodeTemplate(n, parent, cy.$(`#${n.id}`));
return n.el;
},
}]);
@herzaso your implementation changes data, what is not acceptable in some cases. It may be a model object and if we use TypeScript you will need to cast type or use any
what is not good either.
So, I'd like to propose 2 ways:
create
- called only once, the 1st time template is used, returns DOM element or stringupdate
- (optional) called each time data
is changed, if update
is not defined it fallbacks to create
destroy
- (optional) called when the node is removed1st case is a special case of the 2nd. In that case, our object will have only 1 function create
.
const log = event => console.log('here');
cy.attachHtmlToNodes([{
query: 'node',
tpl: {
create(data) {
const tpl = document.createElement('div');
div.addEventListener('click', log, false);
return div;
},
update(data, node) {
node.textContent = data.title;
},
destroy(data, node) {
node.removeEventListener('click', log)
}
}
}]);
@josejulio if you are OK with the proposed implementation, I'll send a PR
@josejulio if you are OK with the proposed implementation, I'll send a PR
It makes sense what you have there, please go ahead!
Woah, you guys are mind readers. Any updates, @stalniy? @herzaso, thanks for the workaround! :)
Sorry, no progress on this yet. I'm working on my open source project. Will get back to this in few weeks I guess
Wow, I would certainly find such a feature useful! I just wanted to ask, has there been any progress so far? @stalniy
No, I end up using string template. Also I don't work on that project that's why it's not a high priority for me anymore
Hi there!
First of all, thanks for the amazing work! It works extremely good for graphs with 1000+ nodes, so you saved us a lot of time :)
I'd like to request a feature which will allow to return HTMLElement from the
tpl
option. Something like this:The good thing about HTMLElement is that I can add event listener easily or use HTMLTemplate to create nodes (what should be faster than innerHTML) - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template
As far as I see the only line which is affected is here. So, here we can add an
if
which checks if it's astring
then useinnerHTML
otherwise useappendChild
.Let me know what you think about this!
P.S.: If you are OK, I or somebody from my team will send a PR to fix that.