Trevor Parscal has uploaded a new change for review.
https://gerrit.wikimedia.org/r/76839
Change subject: The greatest commit in the history of the world*
......................................................................
The greatest commit in the history of the world*
* Within a 3 block radius of Mission and New Montgomery streets, starting from
July 30th at about 2pm 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
* (bugs 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(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/39/76839/1
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: newchange
Gerrit-Change-Id: I339d9253ad472c2f42c3179edc84a83d27561270
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Trevor Parscal <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
