Re: [Wikitech-l] .js errors in Metrolook skins

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-… 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 <thomasmulhall410(a)yahoo.com> An: Wikimedia developers <wikitech-l(a)lists.wikimedia.org> Bump. On Tuesday, 10 February 2015, 14:02, Thomas Mulhall <thomasmulhall410(a)yahoo.com> 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 Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l