jenkins-bot has submitted this change and it was merged.
Change subject: Link inspector bug fixes
......................................................................
Link inspector bug fixes
Formerly known as "The greatest commit in the history of the world*".
* Within a 3 block radius of Drayton Park and Auburt Park, starting
from July 30th at about 9pm or so.
Bugs:
* (bug 51404) Allow escaping out of the link inspector when in creation
mode (no text is selected, text will be inserted based on link target)
and the text input is empty
* (bug 51065 and bug 51415) Keep model and view in sync when changing the
link inspector's text input value and showing options in a menu
* (bug 51523) Either restore selection at the time of close to what it was
before opening the inspector (when using back) or to what it was before
closing (might be changed by transactions processed during the close
method) - this makes it simpler and more natural when clicking away from
the link inspector, even when there are changes that must be saved by
the link inspector on close
Bonus:
* Use only the light blue highlight color for menu widget items - the
checkmark already displays the selected item, the dark blue is just
masking the current highlight position and confusing the peoples
* Remove links when the user deletes everything from the link inspector's
text input and then closes the link inspector
* Replace select menu's evil "silent" selectItem/highlightItem argument
with a new method called initializeSelection which sets both selection
and highlighting to an item without emitting events - this is needed
when synchronizing the view with the model so the model isn't
immediately told to change to a value it already has
* Make the MWTitle lookup menu not flash like crazy as you type (this was
caused by a copy-paste oversight overriding
initializeLookupMenuSelection unnecessarily)
Bug: 51404
Bug: 51065
Bug: 51415
Bug: 51523
Change-Id: I339d9253ad472c2f42c3179edc84a83d27561270
---
M modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
M modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
M modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
M modules/ve/ui/styles/ve.ui.Widget.css
M modules/ve/ui/tools/dropdowns/ve.ui.FormatDropdownTool.js
M modules/ve/ui/widgets/ve.ui.LookupInputWidget.js
M modules/ve/ui/widgets/ve.ui.MenuWidget.js
M modules/ve/ui/widgets/ve.ui.SelectWidget.js
8 files changed, 61 insertions(+), 63 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
b/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
index cb72a3b..87d6bd0 100644
--- a/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
+++ b/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
@@ -144,16 +144,19 @@
};
/**
- * Set selection in the lookup menu with current information.
- *
- * @method
- * @chainable
+ * @inheritDoc
*/
ve.ui.MWLinkTargetInputWidget.prototype.initializeLookupMenuSelection =
function () {
- // Attempt to maintain selection on current annotation
- this.lookupMenu.selectItem( this.lookupMenu.getItemFromData(
this.annotation ), true );
+ var item;
+
// Parent method
ve.ui.LookupInputWidget.prototype.initializeLookupMenuSelection.call(
this );
+
+ // Update annotation to match selected item
+ item = this.lookupMenu.getSelectedItem();
+ if ( item ) {
+ this.setAnnotation( item.getData() );
+ }
};
/**
@@ -165,7 +168,7 @@
* @param {string} value New value
*/
ve.ui.MWLinkTargetInputWidget.prototype.setValue = function ( value ) {
- // Keep annotation in sync with value, call parent method.
+ // Keep annotation in sync with value by skipping parent and calling
grandparent method
ve.ui.TextInputWidget.prototype.setValue.call( this, value );
};
diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
b/modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
index 1ec7595..3179d3c 100644
--- a/modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
+++ b/modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
@@ -130,16 +130,3 @@
return items;
};
-
-/**
- * Set selection in the lookup menu with current information.
- *
- * @method
- * @chainable
- */
-ve.ui.MWTitleInputWidget.prototype.initializeLookupMenuSelection = function ()
{
- // Attempt to maintain selection on current annotation
- this.lookupMenu.selectItem( this.lookupMenu.getItemFromData(
this.annotation ), true );
- // Parent method
- ve.ui.LookupInputWidget.prototype.initializeLookupMenuSelection.call(
this );
-};
diff --git a/modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
b/modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
index d8424d4..c5f4c9c 100644
--- a/modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
+++ b/modules/ve/ui/inspectors/ve.ui.AnnotationInspector.js
@@ -115,17 +115,17 @@
// Parent method
ve.ui.Inspector.prototype.onClose.call( this, action );
- var i, len, annotations, selection,
+ var i, len, annotations,
insert = false,
undo = false,
clear = false,
set = false,
target = this.targetInput.getValue(),
annotation = this.targetInput.getAnnotation(),
- remove = action === 'remove' && !!annotation,
+ remove = target === '' || ( action === 'remove' && !!annotation
),
surfaceModel = this.surface.getModel(),
fragment = surfaceModel.getFragment( this.initialSelection,
false ),
- currentSelection = surfaceModel.getSelection();
+ selection = surfaceModel.getSelection();
if ( remove ) {
clear = true;
@@ -159,19 +159,15 @@
fragment.annotateContent( 'clear', annotations[i] );
}
}
- if ( set ) {
+ if ( set && annotation ) {
// Apply new annotation
fragment.annotateContent( 'set', annotation );
}
if ( action === 'back' ) {
+ // Restore selection to what it was before we expanded it
selection = this.previousSelection;
}
- // Update selection unless it's been changed since opening inspector
- if ( currentSelection.equals( this.initialSelection ) ) {
- this.surface.execute(
- 'content', 'select', selection || new ve.Range(
fragment.getRange().end )
- );
- }
+ this.surface.execute( 'content', 'select', selection );
// Reset state
this.isNewAnnotation = false;
};
diff --git a/modules/ve/ui/styles/ve.ui.Widget.css
b/modules/ve/ui/styles/ve.ui.Widget.css
index d422bdc..12f3d60 100644
--- a/modules/ve/ui/styles/ve.ui.Widget.css
+++ b/modules/ve/ui/styles/ve.ui.Widget.css
@@ -397,6 +397,14 @@
display: block;
}
+.ve-ui-menuItemWidget.ve-ui-optionWidget-selected {
+ background-color: transparent;
+}
+
+.ve-ui-menuItemWidget.ve-ui-optionWidget-highlighted {
+ background-color: #e1f3ff;
+}
+
/* ve.ui.MenuSectionItemWidget */
.ve-ui-menuSectionItemWidget {
diff --git a/modules/ve/ui/tools/dropdowns/ve.ui.FormatDropdownTool.js
b/modules/ve/ui/tools/dropdowns/ve.ui.FormatDropdownTool.js
index ea57ae4..104ecf8 100644
--- a/modules/ve/ui/tools/dropdowns/ve.ui.FormatDropdownTool.js
+++ b/modules/ve/ui/tools/dropdowns/ve.ui.FormatDropdownTool.js
@@ -164,10 +164,10 @@
}
}
if ( match ) {
- this.menu.selectItem( match, true );
+ this.menu.intializeSelection( match );
this.setLabel( match.$label.text() );
} else {
- this.menu.selectItem( null, true );
+ this.menu.intializeSelection( null );
this.setLabel();
}
};
diff --git a/modules/ve/ui/widgets/ve.ui.LookupInputWidget.js
b/modules/ve/ui/widgets/ve.ui.LookupInputWidget.js
index b7c1447..fda4a4e 100644
--- a/modules/ve/ui/widgets/ve.ui.LookupInputWidget.js
+++ b/modules/ve/ui/widgets/ve.ui.LookupInputWidget.js
@@ -143,7 +143,7 @@
*/
ve.ui.LookupInputWidget.prototype.initializeLookupMenuSelection = function () {
if ( !this.lookupMenu.getSelectedItem() ) {
- this.lookupMenu.selectItem(
this.lookupMenu.getFirstSelectableItem(), true );
+ this.lookupMenu.intializeSelection(
this.lookupMenu.getFirstSelectableItem() );
}
this.lookupMenu.highlightItem( this.lookupMenu.getSelectedItem() );
};
diff --git a/modules/ve/ui/widgets/ve.ui.MenuWidget.js
b/modules/ve/ui/widgets/ve.ui.MenuWidget.js
index 163e035..ab41d4f 100644
--- a/modules/ve/ui/widgets/ve.ui.MenuWidget.js
+++ b/modules/ve/ui/widgets/ve.ui.MenuWidget.js
@@ -128,11 +128,13 @@
*
* @method
* @param {ve.ui.OptionWidget} [item] Item to select, omit to deselect all
- * @param {boolean} [silent=false] Update UI only, do not emit `select` event
* @chainable
*/
-ve.ui.MenuWidget.prototype.selectItem = function ( item, silent ) {
- if ( !this.disabled && !silent ) {
+ve.ui.MenuWidget.prototype.selectItem = function ( item ) {
+ // Parent method
+ ve.ui.SelectWidget.prototype.selectItem.call( this, item );
+
+ if ( !this.disabled ) {
if ( item ) {
this.disabled = true;
item.flash( ve.bind( function () {
@@ -143,8 +145,6 @@
this.hide();
}
}
-
- ve.ui.SelectWidget.prototype.selectItem.call( this, item, silent );
return this;
};
@@ -162,6 +162,7 @@
ve.ui.MenuWidget.prototype.addItems = function ( items, index ) {
var i, len, item;
+ // Parent method
ve.ui.SelectWidget.prototype.addItems.call( this, items, index );
for ( i = 0, len = items.length; i < len; i++ ) {
diff --git a/modules/ve/ui/widgets/ve.ui.SelectWidget.js
b/modules/ve/ui/widgets/ve.ui.SelectWidget.js
index 89dcefa..254a74c 100644
--- a/modules/ve/ui/widgets/ve.ui.SelectWidget.js
+++ b/modules/ve/ui/widgets/ve.ui.SelectWidget.js
@@ -93,7 +93,7 @@
this.pressed = true;
item = this.getTargetItem( e );
if ( item && item.isSelectable() ) {
- this.selectItem( item, true );
+ this.intializeSelection( item );
this.selecting = item;
$( this.$$.context ).one( 'mouseup', ve.bind(
this.onMouseUp, this ) );
}
@@ -130,7 +130,7 @@
if ( !this.disabled && this.pressed ) {
item = this.getTargetItem( e );
if ( item && item !== this.selecting && item.isSelectable() ) {
- this.selectItem( item, true );
+ this.intializeSelection( item );
this.selecting = item;
}
}
@@ -244,25 +244,16 @@
*
* @method
* @param {ve.ui.OptionWidget} [item] Item to highlight, omit to deselect all
- * @param {boolean} [silent=false] Update UI only, do not emit `highlight`
event
* @emits highlight
* @chainable
*/
-ve.ui.SelectWidget.prototype.highlightItem = function ( item, silent ) {
+ve.ui.SelectWidget.prototype.highlightItem = function ( item ) {
var i, len;
- item = this.getItemFromData( item && item.getData() );
- if ( item ) {
- item.setHighlighted( true );
- }
for ( i = 0, len = this.items.length; i < len; i++ ) {
- if ( this.items[i] !== item ) {
- this.items[i].setHighlighted( false );
- }
+ this.items[i].setHighlighted( this.items[i] === item );
}
- if ( !silent ) {
- this.emit( 'highlight', item );
- }
+ this.emit( 'highlight', item );
return this;
};
@@ -272,24 +263,36 @@
*
* @method
* @param {ve.ui.OptionWidget} [item] Item to select, omit to deselect all
- * @param {boolean} [silent=false] Update UI only, do not emit `select` event
* @emits select
* @chainable
*/
-ve.ui.SelectWidget.prototype.selectItem = function ( item, silent ) {
+ve.ui.SelectWidget.prototype.selectItem = function ( item ) {
var i, len;
- item = this.getItemFromData( item && item.getData() );
- if ( item ) {
- item.setSelected( true );
- }
for ( i = 0, len = this.items.length; i < len; i++ ) {
- if ( this.items[i] !== item ) {
- this.items[i].setSelected( false );
- }
+ this.items[i].setSelected( this.items[i] === item );
}
- if ( !silent ) {
- this.emit( 'select', item );
+ this.emit( 'select', item );
+
+ return this;
+};
+
+/**
+ * Setup selection and highlighting.
+ *
+ * This should be used to synchronize the UI with the model without emitting
events that would in
+ * turn update the model.
+ *
+ * @param {ve.ui.OptionWidget} [item] Item to select
+ * @chainable
+ */
+ve.ui.SelectWidget.prototype.intializeSelection = function( item ) {
+ var i, len, selected;
+
+ for ( i = 0, len = this.items.length; i < len; i++ ) {
+ selected = this.items[i] === item;
+ this.items[i].setSelected( selected );
+ this.items[i].setHighlighted( selected );
}
return this;
--
To view, visit https://gerrit.wikimedia.org/r/76839
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I339d9253ad472c2f42c3179edc84a83d27561270
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Trevor Parscal <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
