r101641 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r101640‎ | r101641 | r101642 >
Date:17:52, 2 November 2011
Author:tparscal
Status:deferred
Tags:
Comment:
Added leftBias argument to getRenderedPositionFromOffset so clicking on the right side of a line that ends in a space and it wrapped will place the cursor at the end of the line, rather than at the beginning of the next line.
Modified paths:
  • /trunk/parsers/wikidom/lib/hype/bases/es.DocumentNode.js (modified) (history)
  • /trunk/parsers/wikidom/lib/hype/bases/es.DocumentViewBranchNode.js (modified) (history)
  • /trunk/parsers/wikidom/lib/hype/bases/es.DocumentViewLeafNode.js (modified) (history)
  • /trunk/parsers/wikidom/lib/hype/views/es.ContentView.js (modified) (history)
  • /trunk/parsers/wikidom/lib/hype/views/es.SurfaceView.js (modified) (history)

Diff [purge]

Index: trunk/parsers/wikidom/lib/hype/bases/es.DocumentNode.js
——@@ -117,7 +117,7 @@
118118 nodeLength = this.children[i].getElementLength();
119119 if ( offset >= nodeOffset && offset < nodeOffset + nodeLength ) {
120120 if ( !shallow && this.children[i].getChildren().length ) {
121 - return this.getNodeFromOffset.call( this.children[i], offset - nodeOffset );
 121+ return this.getNodeFromOffset.call( this.children[i], offset - nodeOffset - 1 );
122122 } else {
123123 return this.children[i];
124124 }
Index: trunk/parsers/wikidom/lib/hype/bases/es.DocumentViewLeafNode.js
——@@ -64,8 +64,8 @@
6565 * @param {Integer} offset Offset to get position for
6666 * @returns {es.Position} Position of offset
6767 */
68 -es.DocumentViewLeafNode.prototype.getRenderedPositionFromOffset = function( offset ) {
69 - var position = this.contentView.getRenderedPositionFromOffset( offset ),
 68+es.DocumentViewLeafNode.prototype.getRenderedPositionFromOffset = function( offset, leftBias ) {
 69+ var position = this.contentView.getRenderedPositionFromOffset( offset, leftBias ),
7070 offset = this.$content.offset();
7171 position.top += offset.top;
7272 position.left += offset.left;
——@@ -83,8 +83,8 @@
8484 return this.model.getElementLength();
8585 };
8686
87 -es.DocumentViewLeafNode.prototype.getRenderedLineRange = function( offset ) {
88 - return this.contentView.getRenderedLineRange( offset );
 87+es.DocumentViewLeafNode.prototype.getRenderedLineRangeFromOffset = function( offset ) {
 88+ return this.contentView.getRenderedLineRangeFromOffset( offset );
8989 };
9090
9191 /* Inheritance */
Index: trunk/parsers/wikidom/lib/hype/bases/es.DocumentViewBranchNode.js
——@@ -74,7 +74,7 @@
7575 for ( var i = 1; i < this.children.length; i++ ) {
7676 if ( this.horizontal && this.children[i].$.offset().left > position.left ) {
7777 break;
78 - } else if ( this.children[i].$.offset().top > position.top ) {
 78+ } else if ( !this.horizontal && this.children[i].$.offset().top > position.top ) {
7979 break;
8080 }
8181 node = this.children[i];
——@@ -90,11 +90,12 @@
9191 * @param {Integer} offset Offset to get position for
9292 * @returns {es.Position} Position of offset
9393 */
94 -es.DocumentViewBranchNode.prototype.getRenderedPositionFromOffset = function( offset ) {
 94+es.DocumentViewBranchNode.prototype.getRenderedPositionFromOffset = function( offset, leftBias ) {
9595 var node = this.getNodeFromOffset( offset, true );
9696 if ( node !== null ) {
9797 return node.getRenderedPositionFromOffset(
98 - offset - this.getOffsetFromNode( node, true ) - 1
 98+ offset - this.getOffsetFromNode( node, true ) - 1,
 99+ leftBias
99100 );
100101 }
101102 return null;
——@@ -110,12 +111,12 @@
111112 return this.model.getElementLength();
112113 };
113114
114 -es.DocumentViewBranchNode.prototype.getRenderedLineRange = function( offset ) {
 115+es.DocumentViewBranchNode.prototype.getRenderedLineRangeFromOffset = function( offset ) {
115116 var node = this.getNodeFromOffset( offset, true );
116117 if ( node !== null ) {
117118 var nodeOffset = this.getOffsetFromNode( node, true );
118119 return es.Range.newFromTranslatedRange(
119 - node.getRenderedLineRange( offset - nodeOffset - 1 ),
 120+ node.getRenderedLineRangeFromOffset( offset - nodeOffset - 1 ),
120121 nodeOffset + 1
121122 );
122123 }
Index: trunk/parsers/wikidom/lib/hype/views/es.SurfaceView.js
——@@ -81,8 +81,10 @@
8282 es.SurfaceView.boundaryTest = /([ \-\t\r\n\f])/;
8383
8484 es.SurfaceView.prototype.onMouseDown = function( e ) {
85 - var offset = this.documentView.getOffsetFromEvent( e );
86 - this.showCursor( offset );
 85+ var position = es.Position.newFromEventPagePosition( e ),
 86+ offset = this.documentView.getOffsetFromEvent( e ),
 87+ nodeView = this.documentView.getNodeFromOffset( offset, false );
 88+ this.showCursor( offset, position.left > nodeView.$.offset().left + nodeView.$.width() / 2 );
8789 if ( !this.$input.is( ':focus' ) ) {
8890 this.$input.focus().select();
8991 }
——@@ -153,10 +155,10 @@
154156 } else if ( instruction === 'home' ) {
155157 this.showCursor( this.documentView.getRenderedLineRange( this.cursor.offset ).start );
156158 } else if ( instruction === 'end' ) {
157 - var end = this.documentView.getRenderedLineRange( this.cursor.offset ).end
 159+ var end = this.documentView.getRenderedLineRange( this.cursor.offset ).end;
158160 var data = this.documentView.getModel().data;
159161 if ( es.DocumentModel.isContentData( data, end ) ) {
160 - while( es.SurfaceView.boundaryTest.exec( data[ end - 1 ] ) ) {
 162+ while ( es.SurfaceView.boundaryTest.exec( data[ end - 1 ] ) ) {
161163 end--;
162164 }
163165 }
——@@ -170,10 +172,12 @@
171173 * @method
172174 * @param offset {Integer} Position to show the cursor at
173175 */
174 -es.SurfaceView.prototype.showCursor = function( offset ) {
 176+es.SurfaceView.prototype.showCursor = function( offset, leftBias ) {
175177 if ( typeof offset !== 'undefined' ) {
176178 this.cursor.offset = offset;
177 - var position = this.documentView.getRenderedPositionFromOffset( this.cursor.offset );
 179+ var position = this.documentView.getRenderedPositionFromOffset(
 180+ this.cursor.offset, leftBias
 181+ );
178182 this.cursor.$.css( {
179183 'left': position.left,
180184 'top': position.top,
Index: trunk/parsers/wikidom/lib/hype/views/es.ContentView.js
——@@ -208,8 +208,8 @@
209209 es.ContentView.prototype.drawSelection = function( range ) {
210210 range.normalize();
211211
212 - var fromLineIndex = this.getRenderedLineIndex( range.start ),
213 - toLineIndex = this.getRenderedLineIndex( range.end ),
 212+ var fromLineIndex = this.getRenderedLineIndexFromOffset( range.start ),
 213+ toLineIndex = this.getRenderedLineIndexFromOffset( range.end ),
214214 fromPosition = this.getRenderedPositionFromOffset( range.start ),
215215 toPosition = this.getRenderedPositionFromOffset( range.end );
216216
——@@ -271,7 +271,7 @@
272272 * @param {Integer} offset Offset to get line for
273273 * @returns {Integer} Index of rendered lin offset is within
274274 */
275 -es.ContentView.prototype.getRenderedLineIndex = function( offset ) {
 275+es.ContentView.prototype.getRenderedLineIndexFromOffset = function( offset ) {
276276 for ( var i = 0; i < this.lines.length; i++ ) {
277277 if ( this.lines[i].range.containsOffset( offset ) ) {
278278 return i;
——@@ -280,6 +280,39 @@
281281 return this.lines.length - 1;
282282 };
283283
 284+/*
 285+ * Gets the index of the rendered line closest to a given position.
 286+ *
 287+ * If the position is above the first line, the offset will always be 0, and if the position is
 288+ * below the last line the offset will always be the content length. All other vertical
 289+ * positions will fall inside of one of the lines.
 290+ *
 291+ * @method
 292+ * @returns {Integer} Index of rendered line closest to position
 293+ */
 294+es.ContentView.prototype.getRenderedLineIndexFromPosition = function( position ) {
 295+ var lineCount = this.lines.length;
 296+ // Positions above the first line always jump to the first offset
 297+ if ( !lineCount || position.top < 0 ) {
 298+ return 0;
 299+ }
 300+ // Find which line the position is inside of
 301+ var i = 0,
 302+ top = 0;
 303+ while ( i < lineCount ) {
 304+ top += this.lines[i].height;
 305+ if ( position.top < top ) {
 306+ break;
 307+ }
 308+ i++;
 309+ }
 310+ // Positions below the last line always jump to the last offset
 311+ if ( i === lineCount ) {
 312+ return i - 1;
 313+ }
 314+ return i;
 315+};
 316+
284317 /**
285318 * Gets the range of the rendered line a given offset is within.
286319 *
——@@ -289,7 +322,7 @@
290323 * @param {Integer} offset Offset to get line for
291324 * @returns {es.Range} Range of line offset is within
292325 */
293 -es.ContentView.prototype.getRenderedLineRange = function( offset ) {
 326+es.ContentView.prototype.getRenderedLineRangeFromOffset = function( offset ) {
294327 for ( var i = 0; i < this.lines.length; i++ ) {
295328 if ( this.lines[i].range.containsOffset( offset ) ) {
296329 return this.lines[i].range;
——@@ -318,36 +351,10 @@
319352 // Localize position
320353 position.subtract( es.Position.newFromElementPagePosition( this.$ ) );
321354
322 - //
 355+ // Get the line object nearest the position
 356+ var line = this.lines[this.getRenderedLineIndexFromPosition( position )];
 357+
323358 /*
324 - * Line finding
325 - *
326 - * If the position is above the first line, the offset will always be 0, and if the position is
327 - * below the last line the offset will always be the content length. All other vertical
328 - * positions will fall inside of one of the lines.
329 - */
330 - var lineCount = this.lines.length;
331 - // Positions above the first line always jump to the first offset
332 - if ( !lineCount || position.top < 0 ) {
333 - return 0;
334 - }
335 - // Find which line the position is inside of
336 - var i = 0,
337 - top = 0;
338 - while ( i < lineCount ) {
339 - top += this.lines[i].height;
340 - if ( position.top < top ) {
341 - break;
342 - }
343 - i++;
344 - }
345 - // Positions below the last line always jump to the last offset
346 - if ( i == lineCount ) {
347 - return this.model.getContentLength();
348 - }
349 - // Alias current line object
350 - var line = this.lines[i];
351 - /*
352359 * Offset finding
353360 *
354361 * Now that we know which line we are on, we can just use the "fitCharacters" method to get the
——@@ -374,8 +381,8 @@
375382 return Math.min(
376383 // If the position is right of the center of the character it's on top of, increment offset
377384 fit.end + ( position.left >= center ? 1 : 0 ),
378 - // If the line ends in a non-boundary character, decrement offset
379 - line.range.end + ( this.boundaryTest.exec( line.text.substr( -1 ) ) ? -1 : 0 )
 385+ // Don't allow the value to be higher than the end
 386+ line.range.end
380387 );
381388 };
382389
——@@ -390,8 +397,8 @@
391398 * @returns {Object} Object containing left, top and bottom properties, each positions in pixels as
392399 * well as a line index
393400 */
394 -es.ContentView.prototype.getRenderedPositionFromOffset = function( offset ) {
395 - /*
 401+es.ContentView.prototype.getRenderedPositionFromOffset = function( offset, leftBias ) {
 402+ /*
396403 * Range validation
397404 *
398405 * Rather than clamping the range, which can hide errors, exceptions will be thrown if offset is
——@@ -416,7 +423,7 @@
417424 position = new es.Position();
418425 while ( lineIndex < lineCount ) {
419426 line = this.lines[lineIndex];
420 - if ( line.range.containsOffset( offset ) ) {
 427+ if ( line.range.containsOffset( offset ) || ( leftBias && line.range.end === offset ) ) {
421428 position.bottom = position.top + line.height;
422429 break;
423430 }

Status & tagging log