''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]''
These control the images that are under the arrow next to the sitename and
brings a drop down of sites you want to show with the image and url
On Friday, 13 February 2015, 7:37, "[email protected]"
<[email protected]> wrote:
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
