r92177 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92176‎ | r92177 | r92178 >
Date:18:38, 14 July 2011
Author:tparscal
Status:deferred
Tags:
Comment:
Refactored annotateContent methods to use "method" as a separate argument rather than bundling it into the annotation object. Added lots of documentation.
Modified paths:
  • /trunk/parsers/wikidom/demos/es/index.html (modified) (history)
  • /trunk/parsers/wikidom/lib/es/es.Block.js (modified) (history)
  • /trunk/parsers/wikidom/lib/es/es.Content.js (modified) (history)
  • /trunk/parsers/wikidom/lib/es/es.ParagraphBlock.js (modified) (history)
  • /trunk/parsers/wikidom/lib/es/es.Surface.js (modified) (history)

Diff [purge]

Index: trunk/parsers/wikidom/lib/es/es.ParagraphBlock.js
——@@ -75,12 +75,13 @@
7676 *
7777 * If a range arguments are not provided, all content will be annotated.
7878 *
 79+ * @param method {String} Way to apply annotation ("toggle", "add" or "remove")
7980 * @param annotation {Object} Annotation to apply
8081 * @param start {Integer} Offset to begin annotating from
8182 * @param end {Integer} Offset to stop annotating to
8283 */
83 -Block.prototype.annotateContent = function( annotation, start, end ) {
84 - this.content.annotate( annotation, start, end );
 84+Block.prototype.annotateContent = function( method, annotation, start, end ) {
 85+ this.content.annotate( method, annotation, start, end );
8586 };
8687
8788 extend( ParagraphBlock, Block );
Index: trunk/parsers/wikidom/lib/es/es.Surface.js
——@@ -521,10 +521,11 @@
522522 *
523523 * If a selection argument is not provided, the current selection will be annotated.
524524 *
 525+ * @param method {String} Way to apply annotation ("toggle", "add" or "remove")
525526 * @param annotation {Object} Annotation to apply
526527 * @param selection {Selection} Range to apply annotation to
527528 */
528 -Surface.prototype.annotateContent= function( annotation, selection ) {
 529+Surface.prototype.annotateContent= function( method, annotation, selection ) {
529530 if ( selection === undefined ) {
530531 selection = this.selection;
531532 }
——@@ -535,7 +536,7 @@
536537 to = selection.end;
537538 if ( from.block === to.block ) {
538539 // Single block annotation
539 - from.block.annotateContent( annotation, from.offset, to.offset );
 540+ from.block.annotateContent( method, annotation, from.offset, to.offset );
540541 from.block.renderContent( from.offset, function() {
541542 surface.drawSelection();
542543 } );
——@@ -545,19 +546,19 @@
546547 var block = this.doc.blocks[i];
547548 if ( block === from.block ) {
548549 // From offset to length
549 - block.annotateContent( annotation, from.offset, block.getLength() );
 550+ block.annotateContent( method, annotation, from.offset, block.getLength() );
550551 block.renderContent( from.offset, function() {
551552 surface.drawSelection();
552553 } );
553554 } else if ( block === to.block ) {
554555 // From 0 to offset
555 - block.annotateContent( annotation, 0, to.offset );
 556+ block.annotateContent( method, annotation, 0, to.offset );
556557 block.renderContent( 0, function() {
557558 surface.drawSelection();
558559 } );
559560 } else {
560561 // Full coverage
561 - block.annotateContent( annotation, 0, block.getLength() );
 562+ block.annotateContent( method, annotation, 0, block.getLength() );
562563 block.renderContent( 0, function() {
563564 surface.drawSelection();
564565 } );
Index: trunk/parsers/wikidom/lib/es/es.Block.js
——@@ -98,10 +98,11 @@
9999 *
100100 * If a range arguments are not provided, all content will be annotated.
101101 *
 102+ * @param method {String} Way to apply annotation ("toggle", "add" or "remove")
102103 * @param annotation {Object} Annotation to apply
103104 * @param start {Integer} Offset to begin annotating from
104105 * @param end {Integer} Offset to stop annotating to
105106 */
106 -Block.prototype.annotateContent = function( annotation, start, end ) {
 107+Block.prototype.annotateContent = function( method, annotation, start, end ) {
107108 throw 'Block.annotateContent not implemented in this subclass.';
108109 };
Index: trunk/parsers/wikidom/lib/es/es.Content.js
——@@ -13,6 +13,60 @@
1414 this.data = content || [];
1515 };
1616
 17+/* Static Members */
 18+
 19+/**
 20+ * List of annotation rendering implementations.
 21+ *
 22+ * Each supported annotation renderer must have an open and close property, each either a string or
 23+ * a function which accepts a data argument.
 24+ */
 25+Content.annotationRenderers = {
 26+ 'template': {
 27+ 'open': function( data ) {
 28+ return '<span class="editSurface-format-object">' + data.html;
 29+ },
 30+ 'close': '</span>',
 31+ },
 32+ 'bold': {
 33+ 'open': '<span class="editSurface-format-bold">',
 34+ 'close': '</span>',
 35+ },
 36+ 'italic': {
 37+ 'open': '<span class="editSurface-format-italic">',
 38+ 'close': '</span>',
 39+ },
 40+ 'size': {
 41+ 'open': function( data ) {
 42+ return '<span class="editSurface-format-' + data.type + '">';
 43+ },
 44+ 'close': '</span>',
 45+ },
 46+ 'script': {
 47+ 'open': function( data ) {
 48+ return '<span class="editSurface-format-' + data.type + '">';
 49+ },
 50+ 'close': '</span>',
 51+ },
 52+ 'link': {
 53+ 'open': function( data ) {
 54+ return '<span class="editSurface-format-link" data-href="' + data.href + '">';
 55+ },
 56+ 'close': '</span>'
 57+ }
 58+};
 59+
 60+Content.htmlCharacters = {
 61+ '&': '&amp;',
 62+ '<': '&lt;',
 63+ '>': '&gt;',
 64+ '\'': '&#039;',
 65+ '"': '&quot;',
 66+ ' ': '&nbsp;',
 67+ '\n': '<span class="editSurface-whitespace">&#182;</span>',
 68+ '\t': '<span class="editSurface-whitespace">&#8702;</span>'
 69+};
 70+
1771 /* Static Methods */
1872
1973 /**
——@@ -126,8 +180,73 @@
127181 };
128182
129183 /**
 184+ * Gets a rendered opening or closing of an annotation.
 185+ *
 186+ * Tag nesting is handled using a stack, which keeps track of what is currently open. A common stack
 187+ * argument should be used while rendering content.
 188+ *
 189+ * @param bias {String} Which side of the annotation to render, either "open" or "close"
 190+ * @param annotation {Object} Annotation to render
 191+ * @param stack {Array} List of currently open annotations
 192+ * @return {String} Rendered annotation
 193+ */
 194+Content.renderAnnotation = function( bias, annotation, stack ) {
 195+ var renderers = Content.annotationRenderers,
 196+ type = annotation.type,
 197+ out = '';
 198+ if ( type in renderers ) {
 199+ if ( bias === 'open' ) {
 200+ // Add annotation to the top of the stack
 201+ stack.push( annotation );
 202+ // Open annotation
 203+ out += typeof renderers[type]['open'] === 'function'
 204+ ? renderers[type]['open']( annotation.data )
 205+ : renderers[type]['open'];
 206+ } else {
 207+ if ( stack[stack.length - 1] === annotation ) {
 208+ // Remove annotation from top of the stack
 209+ stack.pop();
 210+ // Close annotation
 211+ out += typeof renderers[type]['close'] === 'function'
 212+ ? renderers[type]['close']( annotation.data )
 213+ : renderers[type]['close'];
 214+ } else {
 215+ // Find the annotation in the stack
 216+ var depth = stack.indexOf( annotation );
 217+ if ( depth === -1 ) {
 218+ throw 'Invalid stack error. An element is missing from the stack.';
 219+ }
 220+ // Close each already opened annotation
 221+ for ( var i = stack.length - 1; i >= depth + 1; i-- ) {
 222+ out += typeof renderers[stack[i].type]['close'] === 'function'
 223+ ? renderers[stack[i].type]['close']( stack[i].data )
 224+ : renderers[stack[i].type]['close'];
 225+ }
 226+ // Close the buried annotation
 227+ out += typeof renderers[type]['close'] === 'function'
 228+ ? renderers[type]['close']( annotation.data )
 229+ : renderers[type]['close'];
 230+ // Re-open each previously opened annotation
 231+ for ( var i = depth + 1; i < stack.length; i++ ) {
 232+ out += typeof renderers[stack[i].type]['open'] === 'function'
 233+ ? renderers[stack[i].type]['open']( stack[i].data )
 234+ : renderers[stack[i].type]['open'];
 235+ }
 236+ // Remove the annotation from the middle of the stack
 237+ stack.splice( depth, 1 );
 238+ }
 239+ }
 240+ }
 241+ return out;
 242+};
 243+
 244+/* Methods */
 245+
 246+/**
130247 * Gets plain text version of the content within a specific range.
131248 *
 249+ * Range arguments (start and end) are clamped if out of range.
 250+ *
132251 * @param start {Integer} Optional beginning of range, if omitted range will begin at 0
133252 * @param end {Integer} Optional end of range, if omitted range will end a this.data.length
134253 * @return {String} Plain text within given range
——@@ -151,10 +270,27 @@
152271 return text;
153272 };
154273
 274+/**
 275+ * Gets a new Content object containing content data within a specific range.
 276+ *
 277+ * Range arguments (start and end) are clamped if out of range.
 278+ *
 279+ * @param start {Integer} Optional beginning of range, if omitted range will begin at 0
 280+ * @param end {Integer} Optional end of range, if omitted range will end a this.data.length
 281+ * @return {Content} New content object
 282+ */
155283 Content.prototype.slice = function( start, end ) {
156284 return new Content( this.data.slice( start, end ) );
157285 };
158286
 287+/**
 288+ * Inserts content data at a specific position.
 289+ *
 290+ * Inserted content will inherit annotations from neighboring content.
 291+ *
 292+ * @param start {Integer} Position to insert content at
 293+ * @param insert {Array} Content data to insert
 294+ */
159295 Content.prototype.insert = function( start, insert ) {
160296 // TODO: Prefer to not take annotations from a neighbor that's a space character
161297 var neighbor = this.data[Math.max( start - 1, 0 )];
——@@ -170,99 +306,37 @@
171307 Array.prototype.splice.apply( this.data, [start, 0].concat( insert ) )
172308 };
173309
 310+/**
 311+ * Removes content data within a specific range.
 312+ *
 313+ * @param start {Integer} Beginning of range
 314+ * @param end {Integer} End of range
 315+ */
174316 Content.prototype.remove = function( start, end ) {
175317 this.data.splice( start, end - start );
176318 };
177319
 320+/**
 321+ * Gets the length of the content data.
 322+ *
 323+ * @return {Integer} Length of content data
 324+ */
178325 Content.prototype.getLength = function() {
179326 return this.data.length;
180327 };
181328
182 -Content.annotationRenderers = {
183 - 'template': {
184 - 'open': function( data ) {
185 - return '<span class="editSurface-format-object">' + data.html;
186 - },
187 - 'close': '</span>',
188 - },
189 - 'bold': {
190 - 'open': '<span class="editSurface-format-bold">',
191 - 'close': '</span>',
192 - },
193 - 'italic': {
194 - 'open': '<span class="editSurface-format-italic">',
195 - 'close': '</span>',
196 - },
197 - 'size': {
198 - 'open': function( data ) {
199 - return '<span class="editSurface-format-' + data.type + '">';
200 - },
201 - 'close': '</span>',
202 - },
203 - 'script': {
204 - 'open': function( data ) {
205 - return '<span class="editSurface-format-' + data.type + '">';
206 - },
207 - 'close': '</span>',
208 - },
209 - 'link': {
210 - 'open': function( data ) {
211 - return '<span class="editSurface-format-link" data-href="' + data.href + '">';
212 - },
213 - 'close': '</span>'
214 - }
215 -};
216 -
217 -Content.renderAnnotation = function( bias, annotation, stack ) {
218 - var renderers = Content.annotationRenderers,
219 - type = annotation.type,
220 - out = '';
221 - if ( type in renderers ) {
222 - if ( bias === 'open' ) {
223 - // Add annotation to the top of the stack
224 - stack.push( annotation );
225 - // Open annotation
226 - out += typeof renderers[type]['open'] === 'function'
227 - ? renderers[type]['open']( annotation.data )
228 - : renderers[type]['open'];
229 - } else {
230 - if ( stack[stack.length - 1] === annotation ) {
231 - // Remove annotation from top of the stack
232 - stack.pop();
233 - // Close annotation
234 - out += typeof renderers[type]['close'] === 'function'
235 - ? renderers[type]['close']( annotation.data )
236 - : renderers[type]['close'];
237 - } else {
238 - // Find the annotation in the stack
239 - var depth = stack.indexOf( annotation );
240 - if ( depth === -1 ) {
241 - throw 'Invalid stack error. An element is missing from the stack.';
242 - }
243 - // Close each already opened annotation
244 - for ( var i = stack.length - 1; i >= depth + 1; i-- ) {
245 - out += typeof renderers[stack[i].type]['close'] === 'function'
246 - ? renderers[stack[i].type]['close']( stack[i].data )
247 - : renderers[stack[i].type]['close'];
248 - }
249 - // Close the buried annotation
250 - out += typeof renderers[type]['close'] === 'function'
251 - ? renderers[type]['close']( annotation.data )
252 - : renderers[type]['close'];
253 - // Re-open each previously opened annotation
254 - for ( var i = depth + 1; i < stack.length; i++ ) {
255 - out += typeof renderers[stack[i].type]['open'] === 'function'
256 - ? renderers[stack[i].type]['open']( stack[i].data )
257 - : renderers[stack[i].type]['open'];
258 - }
259 - // Remove the annotation from the middle of the stack
260 - stack.splice( depth, 1 );
261 - }
262 - }
263 - }
264 - return out;
265 -};
266 -
 329+/**
 330+ * Gets a list of indexes within the content data which use a given annotation.
 331+ *
 332+ * Strict coverage may be used to compare not only annotation types, but also their data. Since new
 333+ * line characters are never annotated, they are always considered covered.
 334+ *
 335+ * @param start {Integer} Beginning of range
 336+ * @param end {Integer} End of range
 337+ * @param annotation {Object} Annotation to compare with
 338+ * @param strict {Boolean} Optionally compare annotation data as well as type
 339+ * @return {Array} List of indexes of covered characters within content data
 340+ */
267341 Content.prototype.coverageOfAnnotation = function( start, end, annotation, strict ) {
268342 var coverage = [];
269343 for ( var i = start; i < end; i++ ) {
——@@ -282,16 +356,27 @@
283357 return coverage;
284358 };
285359
 360+/**
 361+ * Gets the first index within an annotated character that matches a given annotation.
 362+ *
 363+ * Strict coverage may be used to compare not only annotation types, but also their data.
 364+ *
 365+ * @param offset {Integer} Index of character within content data to find annotation within
 366+ * @param annotation {Object} Annotation to compare with
 367+ * @param strict {Boolean} Optionally compare annotation data as well as type
 368+ */
286369 Content.prototype.indexOfAnnotation = function( offset, annotation, strict ) {
287370 var annotatedCharacter = this.data[offset];
288 - for ( var i = 1; i < this.data[offset].length; i++ ) {
289 - if ( annotatedCharacter[i].type === annotation.type ) {
290 - if ( strict ) {
291 - if ( Content.compareObjects( annotatedCharacter[i].data, annotation.data ) ) {
 371+ if ( typeof annotatedCharacter !== 'string' ) {
 372+ for ( var i = 1; i < this.data[offset].length; i++ ) {
 373+ if ( annotatedCharacter[i].type === annotation.type ) {
 374+ if ( strict ) {
 375+ if ( Content.compareObjects( annotatedCharacter[i].data, annotation.data ) ) {
 376+ return i;
 377+ }
 378+ } else {
292379 return i;
293380 }
294 - } else {
295 - return i;
296381 }
297382 }
298383 }
——@@ -299,18 +384,22 @@
300385 };
301386
302387 /**
303 - * Applies an annotation to a given range.
 388+ * Applies an annotation to content data within a given range.
304389 *
305 - * If a range arguments are not provided, all content will be annotated.
 390+ * If a range arguments are not provided, all content will be annotated. New line characters are
 391+ * never annotated. The add method will replace and the remove method will delete any existing
 392+ * annotations with the same type as the annotation argument, regardless of their data properties.
 393+ * The toggle method will use add if any of the content within the range is not already covered by
 394+ * the annotation, or remove if all of it is.
306395 *
 396+ * @param method {String} Way to apply annotation ("toggle", "add" or "remove")
307397 * @param annotation {Object} Annotation to apply
308398 * @param start {Integer} Offset to begin annotating from
309399 * @param end {Integer} Offset to stop annotating to
310400 */
311 -Content.prototype.annotate = function( annotation, start, end ) {
 401+Content.prototype.annotate = function( method, annotation, start, end ) {
312402 start = Math.max( start, 0 );
313403 end = Math.min( end, this.data.length );
314 - method = annotation.method;
315404 if ( method === 'toggle' ) {
316405 var coverage = this.coverageOfAnnotation( start, end, annotation, false );
317406 if ( coverage.length === end - start ) {
——@@ -361,17 +450,13 @@
362451 }
363452 };
364453
365 -Content.htmlCharacters = {
366 - '&': '&amp;',
367 - '<': '&lt;',
368 - '>': '&gt;',
369 - '\'': '&#039;',
370 - '"': '&quot;',
371 - ' ': '&nbsp;',
372 - '\n': '<span class="editSurface-whitespace">&#182;</span>',
373 - '\t': '<span class="editSurface-whitespace">&#8702;</span>'
374 -};
375 -
 454+/**
 455+ * Gets an HTML rendering of a range of content data.
 456+ *
 457+ * @param start {Integer} Beginning of range
 458+ * @param end {Integer} End of range
 459+ * @param {String} Rendered HTML of content data
 460+ */
376461 Content.prototype.render = function( start, end ) {
377462 if ( start || end ) {
378463 return this.slice( start, end ).render();
——@@ -412,6 +497,5 @@
413498 out += right[0] in Content.htmlCharacters ? Content.htmlCharacters[right[0]] : right[0];
414499 left = right;
415500 }
416 -
417501 return out;
418502 }
Index: trunk/parsers/wikidom/demos/es/index.html
——@@ -107,65 +107,44 @@
108108 var surface = new Surface( $('#es-editor'), doc );
109109
110110 $( '#es-toolbar-bold' ).click( function() {
111 - surface.annotateContent( {
112 - 'method': 'toggle',
113 - 'type': 'bold'
 111+ surface.annotateContent( 'toggle', { 'type': 'bold'
114112 } );
115113 } );
116114 $( '#es-toolbar-italic' ).click( function() {
117 - surface.annotateContent( {
118 - 'method': 'toggle',
119 - 'type': 'italic'
120 - } );
 115+ surface.annotateContent( 'toggle', { 'type': 'italic' } );
121116 } );
122117 $( '#es-toolbar-small' ).click( function() {
123 - surface.annotateContent( {
124 - 'method': 'toggle',
 118+ surface.annotateContent( 'toggle', {
125119 'type': 'size',
126 - 'data': {
127 - 'type': 'small'
128 - }
 120+ 'data': { 'type': 'small' }
129121 } );
130122 } );
131123 $( '#es-toolbar-big' ).click( function() {
132 - surface.annotateContent( {
133 - 'method': 'toggle',
 124+ surface.annotateContent( 'toggle', {
134125 'type': 'size',
135 - 'data': {
136 - 'type': 'big'
137 - }
 126+ 'data': { 'type': 'big' }
138127 } );
139128 } );
140129 $( '#es-toolbar-sub' ).click( function() {
141 - surface.annotateContent( {
142 - 'method': 'toggle',
 130+ surface.annotateContent( 'toggle', {
143131 'type': 'script',
144 - 'data': {
145 - 'type': 'sub'
146 - }
 132+ 'data': { 'type': 'sub' }
147133 } );
148134 } );
149135 $( '#es-toolbar-super' ).click( function() {
150 - surface.annotateContent( {
151 - 'method': 'toggle',
 136+ surface.annotateContent( 'toggle', {
152137 'type': 'script',
153 - 'data': {
154 - 'type': 'super'
155 - }
 138+ 'data': { 'type': 'super' }
156139 } );
157140 } );
158141 $( '#es-toolbar-link' ).click( function() {
159 - surface.annotateContent( {
160 - 'method': 'toggle',
 142+ surface.annotateContent( 'toggle', {
161143 'type': 'link',
162144 'data': { 'href': '#' }
163145 } );
164146 } );
165147 $( '#es-toolbar-clear' ).click( function() {
166 - surface.annotateContent( {
167 - 'method': 'remove',
168 - 'type': 'all'
169 - } );
 148+ surface.annotateContent( 'remove', { 'type': 'all' } );
170149 } );
171150 } );
172151 </script>

Status & tagging log