wikimedia/mediawiki-extensions-VisualEditor
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/VisualEditor synced 2024-09-25 03:08:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

(bug 42401) Cursor movement fixes

Browse source 
ve.ce.Surface
* Added ve.ce.Surface.adjustCursor, which replaces repetitive and buggy code that was handling left and right arrow key presses
* New method only affects the selection target, so it won't collapse the selection on you - this was what caused bug 42401
* Made hasSlugAtOffset() actually return a boolean

ve.dm.Document
* Fixed turn-around issue in ve.dm.Document.getRelativeOffset - if the offset is already valid and we can't move in the direction we want, we should just leave it be, not turn around
* Since this method was being used by ve.ce.Surface to correct the cursor position on arrow key presses, it was causing the strange cursor jumping when you pressed an arrow key while at the edge of a document

ve.dm.SurfaceFragment
* Fixed typo where getAnnotationRangeFromSelection was preserving selection direction, but checking the wrong properties

ve.dm.Document.test
* Added tests that verify turn-around issue is fixed

Change-Id: Iba55cfc3d531e7d1333b78c94912ff22179aace8
This commit is contained in:
Trevor Parscal 2012-11-27 17:29:09 -08:00 committed by Catrope
parent a525cd86b5
commit 5e477cbe98
4 changed files with 80 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

113
modules/ve/ce/ve.ce.Surface.js
View file

@ -413,67 +413,16 @@ ve.ce.Surface.prototype.onKeyDown = function ( e ) {
this.emit( 'selectionStart' );
}
var offset,
relativeContentOffset,
relativeStructuralOffset,
relativeStructuralOffsetNode,
hasSlug,
newOffset;
switch ( e.keyCode ) {
// Left arrow
case 37:
offset = this.model.getSelection().start;
relativeContentOffset = this.documentView.model.getRelativeContentOffset( offset, -1 );
relativeStructuralOffset = this.documentView.model.getRelativeStructuralOffset( offset - 1, -1, true );
relativeStructuralOffsetNode = this.documentView.documentNode.getNodeFromOffset( relativeStructuralOffset );
hasSlug = this.documentView.getSlugAtOffset( relativeStructuralOffset ) || false;
if ( hasSlug ) {
if ( relativeContentOffset > offset ) {
// If relativeContentOffset returns a greater number, there's nowhere to go toward the left. Go right.
newOffset = relativeStructuralOffset;
} else {
// Move cursor to whichever is nearest to the original offset.
newOffset = Math.max( relativeContentOffset, relativeStructuralOffset );
}
} else if ( relativeContentOffset !== offset - 1 ) {
// The closest content offet is further away than just one offset. Don't trust the browser. Move programatically.
newOffset = relativeContentOffset;
}
if ( newOffset ) {
this.model.change(
null,
new ve.Range( newOffset )
);
if ( this.adjustCursor( -1 ) ) {
e.preventDefault();
}
break;
// Right arrow
case 39:
offset = this.model.getSelection().start;
relativeContentOffset = this.documentView.model.getRelativeContentOffset( offset, 1 );
relativeStructuralOffset = this.documentView.model.getRelativeStructuralOffset( offset + 1, 1, true );
relativeStructuralOffsetNode = this.documentView.documentNode.getNodeFromOffset( relativeStructuralOffset );
hasSlug = this.documentView.getSlugAtOffset( relativeStructuralOffset ) || false;
if ( hasSlug ) {
if ( relativeContentOffset < offset ) {
// If relativeContentOffset returns a lesser number, there's nowhere to go toward the right. Go left.
newOffset = relativeStructuralOffset;
} else {
// Move cursor to whichever is nearest to the original offset.
newOffset = Math.min( relativeContentOffset, relativeStructuralOffset );
}
} else if ( relativeContentOffset !== offset + 1 ) {
newOffset = relativeContentOffset;
}
if ( newOffset ) {
this.model.change(
null,
new ve.Range( newOffset )
);
if ( this.adjustCursor( 1 ) ) {
e.preventDefault();
}
break;
@ -775,6 +724,62 @@ ve.ce.Surface.prototype.handleEnter = function ( e ) {
this.surfaceObserver.start();
};
/**
* Adjusts the cursor position in a given distance.
*
* This method only affects the selection target, preserving selections that are not collapsed and
* the direction of the selection.
*
* @method
* @param {Number} adjustment Distance to adjust the cursor, can be positive or negative
* @returns {Boolean} Cursor was moved
*/
ve.ce.Surface.prototype.adjustCursor = function ( adjustment ) {
// Bypass for zero-adjustment
if ( !adjustment ) {
return false;
}
var adjustedTargetOffset,
bias = adjustment > 0 ? 1 : -1,
selection = this.model.getSelection(),
targetOffset = selection.to,
documentModel = this.model.getDocument(),
relativeContentOffset = documentModel.getRelativeContentOffset( targetOffset, adjustment ),
relativeStructuralOffset = documentModel.getRelativeStructuralOffset(
targetOffset + bias, adjustment, true
);
// Check if we've moved into a slug
if ( this.hasSlugAtOffset( relativeStructuralOffset ) ) {
// Check if the relative content offset is in the opposite direction we are trying to go
if ( ( relativeContentOffset - targetOffset < 0 ? -1 : 1 ) !== bias ) {
// There's nothing past the slug we are already in, stay in it
adjustedTargetOffset = relativeStructuralOffset;
} else {
// There's a slug neaby, go into it if it's closer
adjustedTargetOffset = adjustment < 0 ?
Math.max( relativeContentOffset, relativeStructuralOffset ) :
Math.min( relativeContentOffset, relativeStructuralOffset );
}
}
// Check if we've moved a different distance than we asked for
else if ( relativeContentOffset !== targetOffset + adjustment ) {
// We can't trust the browser, move programatically
adjustedTargetOffset = relativeContentOffset;
}
// If the target changed, update the model
if ( adjustedTargetOffset ) {
this.model.change(
null,
new ve.Range(
selection.isCollapsed() ?
adjustedTargetOffset : selection.from, adjustedTargetOffset
)
);
return true;
}
return false;
};
/**
* Responds to backspace and delete key events.
*
@ -1062,7 +1067,7 @@ ve.ce.Surface.prototype.getNearestCorrectOffset = function ( offset, direction )
* @returns {Boolean} A slug exists at the given offset
*/
ve.ce.Surface.prototype.hasSlugAtOffset = function ( offset ) {
return this.documentView.getSlugAtOffset( offset ) || false;
return !!this.documentView.getSlugAtOffset( offset );
};
/**

7
modules/ve/dm/ve.dm.Document.js
View file

@ -805,6 +805,8 @@ ve.dm.Document.prototype.rebuildNodes = function ( parent, index, numNodes, offs
* Gets an offset a given distance from another using a callback to check if offsets are valid.
*
* - If {offset} is not already valid, one step will be used to move it to an valid one.
* - If {offset} is already valid and can not be moved in the direction of {distance} and still be
* valid, it will be left where it is
* - If {distance} is zero the result will either be {offset} if it's already valid or the
* nearest valid offset to the right if possible and to the left otherwise.
* - If {offset} is after the last valid offset and {distance} is >= 1, or if {offset} if
@ -862,6 +864,11 @@ ve.dm.Document.prototype.getRelativeOffset = function ( offset, distance, callba
// Only turn around if we're about to reach the edge
( ( direction < 0 && i === 0 ) || ( direction > 0 && i === this.data.length ) )
) {
// Before we turn around, let's see if we are at a valid position
if ( callback.apply( window, [this.data, start].concat( args ) ) ) {
// Stay where we are
return start;
}
// Start over going in the opposite direction
direction *= -1;
i = start;

3
modules/ve/dm/ve.dm.SurfaceFragment.js
View file

@ -204,7 +204,8 @@ ve.dm.SurfaceFragment.prototype.expandRange = function ( scope, type ) {
range = this.document.getAnnotatedRangeFromSelection( this.range, type );
// Adjust selection if it does not contain the annotated range
if ( this.range.start > range.start || this.range.end < range.end ) {
if ( this.range.from > this.range.start ) {
// Maintain range direction
if ( this.range.from > this.range.to ) {
range.flip();
}
} else {

12
modules/ve/test/dm/ve.dm.Document.test.js
View file

@ -1013,6 +1013,18 @@ QUnit.test( 'getRelativeContentOffset', function ( assert ) {
'distance': 1,
'expected': 60
},
{
'msg': 'stop at left edge if already valid',
'offset': 1,
'distance': -1,
'expected': 1
},
{
'msg': 'stop at right edge if already valid',
'offset': 60,
'distance': 1,
'expected': 60
},
{
'msg': 'first content offset is farthest left',
'offset': 2,