On Mon, 19 Mar 2012 02:07:54 -0700, Dmitriy Sintsov <[email protected]>
wrote:
* Daniel Friesen <[email protected]> [Mon, 19 Mar 2012 01:35:21
-0700]:
Autoloading classes is not possible. Even if every browser supported
getters and we could use them to dynamically load classes, this would
require synchronous http calls. Which are absolutely HORRIBLE because
they
block the entire JS thread and in addition typically even block the
browser's UI.
How does ResourceLoader figures that function loadEditPrototypes() was
defined in 'ext.jqgmap.edit' when I call this function from
'ext.jqgmap'? And if there was no extra http call, then why the list of
scripts in Chrome debugger does not include 'JQGmap.edit.js' among the
scripts before mw.loader.using() and loadEditPrototypes() call was
performed?
RL doesn't know anything about loadEditPrototypes, I don't know what would
make you think that it does.
I can't really debug and say what's wrong without your code actually
running somewhere.
You shouldn't be dropping the closure, you don't want local things in
the
global scope. You also shouldn't be using the global $ and mw
directly.
Prototypes are not local things. They are carried out with function they
belong to. I define few prototypes in 'ext.jqgmap.view' then I want to
add more prototypes in 'ext.jqgmap.edit'. But not always, only when the
edit mode is active. However you are right about minor local variables.
Everything should be using a pattern like:
( function( $, mw ) {
} )( jQuery, mediaWiki );
Ok, I'll try the closure call with two arguments in every module, didn't
think about that. I'll report if that will fix the abrupt termination of
module "chain" execution.
> var jqgmap = [];
> for ( var mapIndex in jqgmap ) {
This is VERY bad JavaScript coding practice. Please use $.each().
I know that I can use $.each() here. I don't do that for few reasons:
for..in is easier to debug (step by step key) and also because for..in
was problematic only for objects {}, not for arrays [] AFAIK. However I
might be wrong. Maybe I'll switch to $.each() however the code was
working with for..in perfectly well before I refactored it into multiple
modules. There was only one module. However it was growing so I had to
split it into view and edit submodules.
Completely wrong.
for..in is object iteration. It is not to be used for array iteration.
When you use for..in to iterate over an array you're actually iterating
over object keys. Unset things may be left out, if there are any
properties set you'll iterate over those when you shouldn't, if someone
sets something on Array.prototype. (eg: Someone implements .forEach in an
older browser) you'll iterate over that as well, and it's perfectly valid
for a browser to iterate over the keys in an order that's different from
the actual order of items in the array.
> $('<div class="jqgmap_code" id="jqgmap_code' + this.Idx + '">'+
> '<input type="checkbox"></input>' + mw.msg(
'jqgmap-show-code'
)
> +
> '<input type="checkbox"></input>' + mw.msg(
'jqgmap-show-code'
)
> +
> '<div class="jqgmap_preview"></div>' +
> '<div class="jqgmap_preview"></div>' +
> '</div>')
I should really start poking people about this one. `"<div>" . wfMsg(
'foo' ) . "</div>"` is bad in PHP code, and it's just as bad inside
JS.
You should be creating your html with NO + concatenation, and then
using
a
.find() and .text() to insert things where they belong. That, or use
multiple $() calls with .append() to create the full structure.
Likewise you shouldn;t be inserting this.Idx that way into the
attribute.
It should be added with something like .attr( "jqgmap_code" + this.Idx
);
I might consider refactoring of jQuery DOM nodes creation, however the
loading of 'ext.jqgmap.edit' module does not work now so I cannot
continue until I'll figure out why. This code is not ideal however it's
not the reason of the fault. This code was working perfectly well before
splitting into multiple modules and introducing
mw.loader.using('ext.jqgmap.edit',function(){...});
Dmitriy
--
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://daniel.friesen.name]
_______________________________________________
Wikitech-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l
