fix(core): fix external template loading for grid-filter, grid-footer…

[linsolas]

…-cell and grid-header-cell

Fixes external template loading for grid-filter, grid-footer-cell and grid-header-cell.

This issue occurs when trying to use filterHeaderTemplate, footerCellTemplate or headerCellTemplate on a column definition with an external template (e.g. myPersonalTemplate.html as a value).
In fact, Angular UI grid was not waiting for the template to be loaded before executing $elmt.append() function and the template was not displayed (empty template).
This fix waits for the external template to be loaded first.

[dlgski]

is there an issue # that this is resolving, or a plunkr?

[mportuga]

Can you please also add unit tests?

[pleandre]

Hi ! I just looked at your issue backlog.
This commit is fixing the issue about footerCellTemplate loading:
#4744,

It fixes issues about headerCellTemplate loading:
#5717, #5115, #5196, #5712, #5260, #5196, #4851, #4514, #5601

And it also fixes the same issues for filterHeaderTemplate loading (nothing in your backlog)

[mportuga]

@pleandre I am glad to hear it. We will be glad to merge this, once @linsolas adds some unit tests.

[pleandre]

I'll try to add some unit tests to the code I committed.

[mportuga]

@pleandre Any progress?

[mportuga]

Closing due to lack of response. Feel free to reopen this when you have time to add some tests.

[needforspeed]

Is this fix going to be in the future release?

[apodznoev]

Hey guys, this piece of code is still really important!

[pleandre]

@mportuga
This code fixes template loading, for some reason it was done in one way with cellTemplate:
https://github.com/societe-generale/angular-ui-ui-grid/blob/4392e16580734d7bc021636199ca9b91cc44ba28/src/js/core/services/gridClassFactory.js

col.cellTemplatePromise = templateGetPromises[0]; // Code from master

For the cell template it was getting a promise but for others it was not: headerCellTemplate, footerCellTemplate, filterHeaderTemplate. Thus it was not waiting for the template to be loaded and appending an empty template for the header, footer and filter if it was not loaded.

I just looked at the same way it was done for cellTemplate and applied it to headerCellTemplate, footerCellTemplate and filterHeaderTemplate

It was not tested and their was no reason it was done in a different way for cellTemplate.
I looked but I don't know how to test this issue, could you merge it, it fixes a bug and it's done exactly the same way as cellTemplate.

I gave up adding test, I looked a bit but it seems complicated to test. For headerCellTemplate, footerCellTemplate, filterHeaderTemplate there is no mechanism that waits for the template to be loaded. I don't see how to test that if I load an external template which is too slow to load, then it will wait for it to be loaded. I tried but it is not an easy thing to test and I cannot spend days on this.

Could you merge it without test ? There is no reason to have a different way of loading for cellTemplate and the other templates. There was no test commited to test or to explain this difference with cellTemplate.

[pleandre]

@dlgski @mportuga could I get an answer on this or get a hint to unit test it, but there is no reason to do it in one way for celltemplates and another way for others. I just applied the same logic as celltemplates.

Could you re-open this pull request ? we have to maintain our own version of the library because of this bug on a forked branch.

[AdamDiament]

I also really need this, thanks!

[thenem]

i am having the same issue. could really use this fixed. when will it be merged?

[pleandre]

Hi, this pull request is still important and fixes template loading issues. There was two different way of loading templates: one was using a promise and waiting for completion before appending the template, the other was not using promise / waiting for completion. In this pull request I changed to use the same way everywhere and it fixed my problem. Could you give some advice for the unit test you expect, it seems a bit complicated to implement and there is no existing tests for the current way. Otherwise would it be possible to merge it ?