Support return of HTMLElement from `tpl` option

[stalniy]

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:

cy.nodeHtmlLabel([
    {
        query: 'node', 
        tpl(data) {
          const tpl = document.createElement('div');
          div.textContent = data.id;
          div.addEventListener('click', event => console.log('here'), false)
          return div
        }
    }
]);

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 a string then use innerHTML otherwise use appendChild.

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.

[stalniy]

implemented in my fork https://github.com/stalniy/cytoscape-node-html-label

[ofir130]

This is very useful. Any chance it will be merged?

[josejulio]

@stalniy Could you send a PR for this one too?

[stalniy]

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?

1. Make tpl an object with 2 methods: init, update
2. Make tpl a single function and force users to implement memoization by yourself (initial suggestion)

[herzaso]

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;
  },
}]);

[stalniy]

@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:

1. a regular function, use it when you need to simple DOM and return a string (backward compatibility)
2. an object with 3 functions, 2 of which are optional:

• create - called only once, the 1st time template is used, returns DOM element or string
• update - (optional) called each time data is changed, if update is not defined it fallbacks to create
• destroy- (optional) called when the node is removed

1st 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)
    }
  }
}]);

[stalniy]

@josejulio if you are OK with the proposed implementation, I'll send a PR

[josejulio]

@josejulio if you are OK with the proposed implementation, I'll send a PR

It makes sense what you have there, please go ahead!

[joshhoegen]

Woah, you guys are mind readers. Any updates, @stalniy? @herzaso, thanks for the workaround! :)

[stalniy]

Sorry, no progress on this yet. I'm working on my open source project. Will get back to this in few weeks I guess

[Mikeee-e]

Wow, I would certainly find such a feature useful! I just wanted to ask, has there been any progress so far? @stalniy

[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