r102770 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r102769‎ | r102770 | r102771 >
Date:11:53, 11 November 2011
Author:santhosh
Status:resolved (Comments)
Tags:
Comment:
Create ext.webfonts.init and ext.webfonts.core modules, set 'position' => 'top' for both of these modules so
that they start loading and initializing from the head and only execute after document is ready.

ext.webFonts.fontlist.js extends mw.webfonts.config object with the list of fonts and languages

As per https://www.mediawiki.org/wiki/User:Krinkle/Extension_review/WebFonts
Modified paths:
  • /trunk/extensions/WebFonts/WebFonts.hooks.php (modified) (history)
  • /trunk/extensions/WebFonts/WebFonts.php (modified) (history)
  • /trunk/extensions/WebFonts/resources/ext.webfonts.fontlist.js (modified) (history)
  • /trunk/extensions/WebFonts/resources/ext.webfonts.init.js (added) (history)
  • /trunk/extensions/WebFonts/resources/ext.webfonts.js (modified) (history)

Diff [purge]

Index: trunk/extensions/WebFonts/resources/ext.webfonts.js
@@ -10,7 +10,7 @@
1111 mw.webfonts = {
1212
1313 oldconfig: false,
14 - config: mw.webfonts.config,
 14+ config: { fonts: {}, languages: {} },
1515 version: '0.1.2',
1616 fonts : [],
1717 set: function( font ) {
@@ -335,8 +335,4 @@
336336 }
337337 };
338338
339 - $( document ).ready( function() {
340 - mw.webfonts.setup();
341 - } );
342 -
343339 } ) ( jQuery );
Index: trunk/extensions/WebFonts/resources/ext.webfonts.fontlist.js
@@ -4,9 +4,8 @@
55 */
66
77 ( function ( $ ) {
8 - mw.webfonts = {};
98
10 - mw.webfonts.config = {
 9+ config = {
1110 fonts: {
1211 RufScript: {
1312 eot: "Latn/Rufscript.eot",
@@ -316,4 +315,5 @@
317316 cdo: [ "Charis SIL" ]
318317 }
319318 };
 319+ $.extend( mw.webfonts.config, config);
320320 } ) ( jQuery );
Index: trunk/extensions/WebFonts/resources/ext.webfonts.init.js
@@ -0,0 +1,8 @@
 2+/**
 3+ * WebFonts startup script
 4+ */
 5+( function( $ ) {
 6+ $( document ).ready( function() {
 7+ mw.webfonts.setup();
 8+ } );
 9+} )( jQuery );
Property changes on: trunk/extensions/WebFonts/resources/ext.webfonts.init.js
___________________________________________________________________
Added: svn:eol-style
110 + native
Index: trunk/extensions/WebFonts/WebFonts.php
@@ -40,17 +40,22 @@
4141
4242 $wgWebFontsEnabledByDefault = true;
4343
44 -$wgResourceModules['webfonts'] = array(
45 - 'scripts' => 'resources/ext.webfonts.js',
46 - 'styles' => 'resources/ext.webfonts.css',
 44+$wgResourceModules['ext.webfonts.init'] = array(
 45+ 'scripts' => 'resources/ext.webfonts.init.js',
4746 'localBasePath' => $dir,
4847 'remoteExtPath' => 'WebFonts',
49 - 'messages' => array( 'webfonts-load', 'webfonts-reset' ),
50 - 'dependencies' => array( 'jquery.cookie', 'ext.webfonts.fontlist'),
 48+ 'dependencies' => 'ext.webfonts.core',
 49+ 'position' => 'top',
5150 );
5251
53 -$wgResourceModules['ext.webfonts.fontlist'] = array(
54 - 'scripts' => 'resources/ext.webfonts.fontlist.js',
 52+$wgResourceModules['ext.webfonts.core'] = array(
 53+ 'scripts' => array( 'resources/ext.webfonts.js', 'resources/ext.webfonts.fontlist.js' ),
 54+ 'styles' => 'resources/ext.webfonts.css',
5555 'localBasePath' => $dir,
5656 'remoteExtPath' => 'WebFonts',
 57+ 'messages' => array( 'webfonts-load', 'webfonts-reset' ),
 58+ 'dependencies' => 'jquery.cookie' ,
 59+ 'position' => 'top',
5760 );
 61+
 62+
Index: trunk/extensions/WebFonts/WebFonts.hooks.php
@@ -15,7 +15,7 @@
1616 public static function addModules( $out, $skin ) {
1717
1818 if ( $out->getUser()->getOption( 'webfontsEnable' ) ) {
19 - $out->addModules( 'webfonts' );
 19+ $out->addModules( 'ext.webfonts.init' );
2020 }
2121
2222 return true;

Follow-up revisions

RevisionCommit summaryAuthorDate
r104393Change the scope of config to local....santhosh05:25, 28 November 2011

Comments

#Comment by Krinkle (talk | contribs)   23:34, 27 November 2011

Looks good! Just one small thing.

+	config = {

config is put in the global scope, make sure to make it local only by placing var in front of it. Marking fixme because of it, everything else is ok.

Status & tagging log