hinclude icon indicating copy to clipboard operation
hinclude copied to clipboard

review addDOMLoadEvent

Open mnot opened this issue 13 years ago • 4 comments

Make sure that addDOMLoadEvent is still relevant / correct, and plays nicely with jquery, etc.

mnot avatar Dec 24 '11 11:12 mnot

I see this one is still open. I have a different comment on it: I wonder if we should auto run hinclude, in some cases we want to do something first before triggering it. With the current code it is not possible.

The easiest way is to completely remove this line: hinclude.addDOMLoadEvent(function () { hinclude.run(); });

The second way, is that within the run method we can check for some configuration such as var load = this.get_meta("include_load", "custom");

What do you think?

yellow1912 avatar Oct 22 '17 10:10 yellow1912

I don't think hInclude should take over the DOM loading infrastructure; there are plenty of libraries out there for that.

mnot avatar Oct 23 '17 03:10 mnot

Actually, what I mean to say is that hinclude should not automatically trigger on domload event the way it is working right now. There are use cases when we want to manually trigger hinclude instead.

yellow1912 avatar Oct 23 '17 04:10 yellow1912

Seems like that function is being used as constructor.

Maybe refactor it to be like this

function hinclude(){
     // code from addDOMLoadEvent
}
hinclude.prototype = { ... }

leroy0211 avatar Dec 11 '18 21:12 leroy0211