r112019 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r112018‎ | r112019 | r112020 >
Date:16:37, 21 February 2012
Author:jdlrobson
Status:ok (Comments)
Tags:
Comment:
add basic utilities library

going forward it would be nice to provide access
to whatever library is used whether it is a basic
set of functions for modules that cannot use jquery,
jquery itself or something more minimal like zepto
or xuijs

the addClass and removeClass functions are needed for
my next commit which will rewrite fullscreen search in
the beta to use css
Modified paths:
  • /trunk/extensions/MobileFrontend/javascripts/application.js (modified) (history)
  • /trunk/extensions/MobileFrontend/javascripts/beta_application.js (modified) (history)
  • /trunk/extensions/MobileFrontend/tests/js/test_application.js (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/tests/js/test_application.js
——@@ -1,6 +1,26 @@
22 var MFE = MobileFrontend;
33 var MFET = window.MobileFrontendTests;
44
 5+module("MobileFrontend application.js: utils");
 6+
 7+test("addClass", function() {
 8+ var el = $("<div />")[0];
 9+ MFE.utils(el).addClass("foo");
 10+ MFE.utils(el).addClass("bar");
 11+ strictEqual($(el).hasClass("foo"), true);
 12+ strictEqual($(el).hasClass("bar"), true);
 13+});
 14+
 15+test("removeClass", function() {
 16+ var el = $("<div />")[0];
 17+ MFE.utils(el).addClass("foo");
 18+ MFE.utils(el).addClass("bar");
 19+ MFE.utils(el).removeClass("foo");
 20+ MFE.utils(el).removeClass("bar");
 21+ strictEqual($(el).hasClass("foo"), false);
 22+ strictEqual($(el).hasClass("bar"), false);
 23+});
 24+
525 module("MobileFrontend application.js: cookies");
626
727 test("read and write cookies", function() {
Index: trunk/extensions/MobileFrontend/javascripts/beta_application.js
——@@ -177,13 +177,40 @@
178178 return null;
179179 }
180180
 181+ function utilities( el ) {
 182+ function addClass( name ) {
 183+ var className = el.className;
 184+ var classNames = className.split( ' ' );
 185+ classNames.push(name); // TODO: only push if unique
 186+ el.className = classNames.join( ' ' );
 187+ }
 188+
 189+ function removeClass( name ) {
 190+ var className = el.className,
 191+ classNames = className.split( ' ' ),
 192+ newClasses = [], i;
 193+ for( i = 0; i < classNames.length; i++ ) {
 194+ if( classNames[i] !== name ) {
 195+ newClasses.push( classNames[i] );
 196+ }
 197+ }
 198+ el.className = newClasses.join( ' ' );
 199+ }
 200+
 201+ return {
 202+ addClass: addClass,
 203+ removeClass: removeClass
 204+ };
 205+ }
 206+
181207 return {
182208 readCookie: readCookie,
183209 writeCookie: writeCookie,
184210 removeCookie: removeCookie,
185211 wm_reveal_for_hash: wm_reveal_for_hash,
186212 wm_toggle_section: wm_toggle_section,
187 - init: init
 213+ init: init,
 214+ utils: utilities
188215 };
189216
190217 }();
Index: trunk/extensions/MobileFrontend/javascripts/application.js
——@@ -166,13 +166,40 @@
167167 return null;
168168 }
169169
 170+ function utilities( el ) {
 171+ function addClass( name ) {
 172+ var className = el.className;
 173+ var classNames = className.split( ' ' );
 174+ classNames.push(name); // TODO: only push if unique
 175+ el.className = classNames.join( ' ' );
 176+ }
 177+
 178+ function removeClass( name ) {
 179+ var className = el.className,
 180+ classNames = className.split( ' ' ),
 181+ newClasses = [], i;
 182+ for( i = 0; i < classNames.length; i++ ) {
 183+ if( classNames[i] !== name ) {
 184+ newClasses.push( classNames[i] );
 185+ }
 186+ }
 187+ el.className = newClasses.join( ' ' );
 188+ }
 189+
 190+ return {
 191+ addClass: addClass,
 192+ removeClass: removeClass
 193+ };
 194+ }
 195+
170196 return {
171197 readCookie: readCookie,
172198 writeCookie: writeCookie,
173199 removeCookie: removeCookie,
174200 wm_reveal_for_hash: wm_reveal_for_hash,
175201 wm_toggle_section: wm_toggle_section,
176 - init: init
 202+ init: init,
 203+ utils: utilities
177204 };
178205
179206 }();

Comments

#Comment by Nikerabbit (talk | contribs)   17:13, 21 February 2012

Reimplementing parts of jQuery? Isn't there any suitable small library that can be used?

#Comment by Jdlrobson (talk | contribs)   17:37, 21 February 2012

Yes. There are several libraries including zepto [1] and xui [2]. I understand that several mobile devices cannot run jquery due to the payload of the library using up all their memory and crashing the browser and that these modules/plugins need to be able to run in native javascript or using a very slim library (I'm not sure if xui or zepto are slim enough as I am still new and familiarising myself with the problem space! ). Can anyone point me at links to these discussions?

I know that the android app for wikipedia [1] currently uses jquery so I imagine a situation where the MobileFrontend extension could work with either jquery or some other library (in this case the utils library is a handwritten one which I agree is not a long term solution but would be interchangeable with jquery or zepto) and certain enhancements to the mobile frontend would only work in situations where a particular library was available.

[1] https://github.com/madrobby/zepto [2] https://github.com/xui/xui [3] github.com/wikimedia/WikipediaMobile

#Comment by Robmoen (talk | contribs)   21:46, 21 February 2012

As I understand it, the main idea here is to limit the payload size. At this time it makes sense to add to application.js rather than include an entire library. Marking ok.

Status & tagging log