Hi Thomas,

i haven't installed your skin to see the error, because i think it's a 
relatively easy thing you can track down by yourself. You give us 5 lines of 
code where you think the error comes from (how have you tracked it down to 
these 5 lines?). What, if you click on the error message in the browser console 
to find out, _which_ line and which function/statement/variable throws the 
exception?

If you're using Google Chrome, i suggest you to read this webpage [1], 
especially the "Exceptions" part, to know, how to debug your JavaScript code by 
yourself :)

Next point: Your code is like a mess, sorry :) You don't follow our developer 
guidelines and coding conventions at all, here just some examples:
- Naming of global configuration variables
-> You use globals like "image1", "link2" and so on. I see two problems:
1. no wg Prefix, read [2], so no possibility to use the Config feature, e.g.
2. No meaningful namens: What does image1, image2 and so on do? What they 
control? It's like i would name you "person1", variables have feelings, so give 
them meaningful names :P Don't forget to add your skin prefix, read [3]

- Massive use of global key word
Look at this piece of code:
https://github.com/paladox/Metrolook/blob/master/MetrolookTemplate.php#L40-L68

That really looks bad :) If you have a list of globals, write it as a list with 
one global key word, e.g.:
global $image1, $image2, $link1, $link2;

It's a bit confusing to use globals and the new Config feature, which you 
register here [4] (i suggest that it's because you copied Vector), i would 
suggest to be consistent and use globals _or_ the Config feature (where it is 
possible), see [4.1]. And:
https://github.com/paladox/Metrolook/blob/master/MetrolookTemplate.php#L73
That's not the purpose of the Config feature to access foreign configuration 
values. It's possible at the moment, but, maybe, it will not anymore in next 
MediaWiki versions.

- Licensing:
I'm not a lawyer, but i haven't found a notice, that the skin is based on 
Vector (nor the authors of Vector), which may not allowed with the GPL license 
of Vector. But again: I'm not a lawyer :)

- ResourceLoader:
You mixed up your skin template with css styles, JS code, html and PHP. That's 
one way to build a skin, but it's not a really good one. I suggest, that you 
look at the examples from Vector (and other skins and extensions) and that you 
read the Documentation on mediawiki.org [5] to learn how to use the RL to 
deliver your styles and scripts. The RL gives you some advantages (all 
documented on mediawiki.org).

And some other resources:
- MediaWiki coding conventions: 
https://www.mediawiki.org/wiki/Manual:Coding_conventions (very important to 
improve your code readability)
- "How to start as a mediawiki hacker": 
https://www.mediawiki.org/wiki/How_to_become_a_MediaWiki_hacker

Please feel free to click the links on all wikipages ;)

[1] https://developer.chrome.com/devtools/docs/javascript-debugging
[2] https://www.mediawiki.org/wiki/Manual:Wg_variable
[3] https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP#Variables
[4] https://github.com/paladox/Metrolook/blob/master/Metrolook.php#L46
[4.1] https://www.mediawiki.org/wiki/Manual:Configuration_for_developers
[5] https://www.mediawiki.org/wiki/ResourceLoader#Documentation

Freundliche Grüße / Kind regards
Florian Schmidt

-----Original-Nachricht-----
Betreff: Re: [Wikitech-l] .js errors in Metrolook skins
Datum: Thu, 12 Feb 2015 23:58:40 +0100
Von: Thomas Mulhall <[email protected]>
An: Wikimedia developers <[email protected]>

Bump. 

     On Tuesday, 10 February 2015, 14:02, Thomas Mulhall 
<[email protected]> wrote:
   

  Hi I am getting js script errors in Metrolook skin. The error is in 
$(document).click(function(e) {
 if (!$(e.target).closest('#'+openDiv).length) {
  toggleDiv(openDiv);
 }
});
and error says Object expected.
Source code is at 
https://git.wikimedia.org/summary/mediawiki%2Fskins%2FMetrolook and 
https://github.com/paladox/Metrolook 
If I have to split js script out of the MetrolookTemplate.php could I have some 
help to do that please thanks.  

   
_______________________________________________
Wikitech-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l



_______________________________________________
Wikitech-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Reply via email to