jenkins-bot has submitted this change and it was merged.
Change subject: Refactor out trigger listening to allow target and document
triggers
......................................................................
Refactor out trigger listening to allow target and document triggers
Introduce ve.TriggerListener which handles a set of commands.
Some commands can be triggered anywhere in the document, e.g. commandHelp.
Some commands can be triggered anywhere in a target (i.e. the surface
and any dialog spaces) e.g. findAndReplace. All other commands
can only be triggered from within a surface.
The target listener allows us to remove a hack from FindAndReplaceDialog.
For target/document listeners to work we need to add extra keydown listeners
to isolated windows.
Change-Id: I10857cc66526ca77560cb1e8e78ff1c1a08f3736
---
M .docs/categories.json
M .docs/eg-iframe.html
M build/modules.json
M demos/ve/desktop.html
M demos/ve/mobile.html
M src/init/sa/ve.init.sa.Target.js
M src/init/ve.init.Target.js
M src/ui/dialogs/ve.ui.FindAndReplaceDialog.js
M src/ui/dialogs/ve.ui.ToolbarDialog.js
M src/ui/inspectors/ve.ui.FragmentInspector.js
M src/ui/ve.ui.Surface.js
M src/ui/ve.ui.TargetToolbar.js
M src/ui/ve.ui.Toolbar.js
M src/ui/ve.ui.TriggerRegistry.js
A src/ve.TriggerListener.js
M tests/index.html
16 files changed, 222 insertions(+), 79 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/.docs/categories.json b/.docs/categories.json
index 2d5707e..81d5261 100644
--- a/.docs/categories.json
+++ b/.docs/categories.json
@@ -4,7 +4,7 @@
"groups": [
{
"name": "Utilities",
- "classes": ["ve", "ve.Range",
"ve.EventSequencer", "ve.Filibuster"]
+ "classes": ["ve", "ve.Range",
"ve.EventSequencer", "ve.Filibuster", "ve.TriggerListener"]
},
{
"name": "Nodes",
diff --git a/.docs/eg-iframe.html b/.docs/eg-iframe.html
index 27b4637..498e608 100644
--- a/.docs/eg-iframe.html
+++ b/.docs/eg-iframe.html
@@ -108,6 +108,7 @@
<!-- visualEditor.base.build -->
<script src="../src/ve.js"></script>
<script src="../src/ve.utils.js"></script>
+ <script src="../src/ve.TriggerListener.js"></script>
<script src="../src/ve.track.js"></script>
<script src="../src/init/ve.init.js"></script>
<script src="../src/init/ve.init.Platform.js"></script>
diff --git a/build/modules.json b/build/modules.json
index 6d7fc86..fceeaf0 100644
--- a/build/modules.json
+++ b/build/modules.json
@@ -131,6 +131,7 @@
"scripts": [
"src/ve.js",
"src/ve.utils.js",
+ "src/ve.TriggerListener.js",
{ "file": "src/ve.debug.js", "debug": true },
"src/ve.track.js",
"src/init/ve.init.js",
diff --git a/demos/ve/desktop.html b/demos/ve/desktop.html
index 528167e..88dd4d7 100644
--- a/demos/ve/desktop.html
+++ b/demos/ve/desktop.html
@@ -118,6 +118,7 @@
<!-- visualEditor.base.build -->
<script src="../../src/ve.js"></script>
<script src="../../src/ve.utils.js"></script>
+ <script src="../../src/ve.TriggerListener.js"></script>
<script src="../../src/ve.debug.js"></script>
<script src="../../src/ve.track.js"></script>
<script src="../../src/init/ve.init.js"></script>
diff --git a/demos/ve/mobile.html b/demos/ve/mobile.html
index d59d1e9..f7fd342 100644
--- a/demos/ve/mobile.html
+++ b/demos/ve/mobile.html
@@ -119,6 +119,7 @@
<!-- visualEditor.base.build -->
<script src="../../src/ve.js"></script>
<script src="../../src/ve.utils.js"></script>
+ <script src="../../src/ve.TriggerListener.js"></script>
<script src="../../src/ve.debug.js"></script>
<script src="../../src/ve.track.js"></script>
<script src="../../src/init/ve.init.js"></script>
diff --git a/src/init/sa/ve.init.sa.Target.js b/src/init/sa/ve.init.sa.Target.js
index 3f91d08..c663df7 100644
--- a/src/init/sa/ve.init.sa.Target.js
+++ b/src/init/sa/ve.init.sa.Target.js
@@ -64,8 +64,6 @@
if ( this.setupDone ) {
return;
}
-
- // Properties
this.setupDone = true;
surface = this.addSurface( dmDoc );
this.$element.append( surface.$element );
@@ -100,7 +98,11 @@
*/
ve.init.sa.Target.prototype.createSurface = function ( dmDoc, config ) {
config = ve.extendObject( {
- excludeCommands: this.constructor.static.excludeCommands,
+ excludeCommands: OO.simpleArrayUnion(
+ this.constructor.static.excludeCommands,
+ this.constructor.static.documentCommands,
+ this.constructor.static.targetCommands
+ ),
importRules: this.constructor.static.importRules
}, config );
return new this.surfaceClass( dmDoc, config );
diff --git a/src/init/ve.init.Target.js b/src/init/ve.init.Target.js
index 44f59bb..a1ce349 100644
--- a/src/init/ve.init.Target.js
+++ b/src/init/ve.init.Target.js
@@ -25,9 +25,12 @@
// Properties
this.$element = $container;
+ this.elementDocument = this.$element[0].ownerDocument;
this.surfaces = [];
this.surface = null;
this.toolbar = null;
+ this.documentTriggerListener = new ve.TriggerListener(
this.constructor.static.documentCommands );
+ this.targetTriggerListener = new ve.TriggerListener(
this.constructor.static.targetCommands );
// Initialization
this.$element.addClass( 've-init-target' );
@@ -35,6 +38,11 @@
if ( ve.init.platform.constructor.static.isInternetExplorer() ) {
this.$element.addClass( 've-init-target-ie' );
}
+
+ // Events
+ this.onDocumentKeyDownHandler = this.onDocumentKeyDown.bind( this );
+ $( this.elementDocument ).on( 'keydown', this.onDocumentKeyDownHandler
);
+ this.$element.on( 'keydown', this.onTargetKeyDown.bind( this ) );
// Register
ve.init.target = this;
@@ -53,6 +61,7 @@
this.$element.remove();
this.$element = null;
}
+ $( this.elementDocument ).off( 'keydown', this.onDocumentKeyDownHandler
);
ve.init.target = null;
};
@@ -142,6 +151,25 @@
}
];
+/**
+ * List of commands which can be triggered anywhere from within the document
+ *
+ * @type {string[]} List of command names
+ */
+ve.init.Target.static.documentCommands = ['commandHelp'];
+
+/**
+ * List of commands which can be triggered from within the target element
+ *
+ * @type {string[]} List of command names
+ */
+ve.init.Target.static.targetCommands = ['findAndReplace'];
+
+/**
+ * List of commands to exclude from the target entirely
+ *
+ * @type {string[]} List of command names
+ */
ve.init.Target.static.excludeCommands = [];
/**
@@ -168,6 +196,36 @@
/* Methods */
/**
+ * Handle key down events on the document
+ *
+ * @param {jQuery.Event} e Key down event
+ */
+ve.init.Target.prototype.onDocumentKeyDown = function ( e ) {
+ var command, trigger = new ve.ui.Trigger( e );
+ if ( trigger.isComplete() ) {
+ command = this.documentTriggerListener.getCommandByTrigger(
trigger.toString() );
+ if ( command && command.execute( this.getSurface() ) ) {
+ e.preventDefault();
+ }
+ }
+};
+
+/**
+ * Handle key down events on the target
+ *
+ * @param {jQuery.Event} e Key down event
+ */
+ve.init.Target.prototype.onTargetKeyDown = function ( e ) {
+ var command, trigger = new ve.ui.Trigger( e );
+ if ( trigger.isComplete() ) {
+ command = this.targetTriggerListener.getCommandByTrigger(
trigger.toString() );
+ if ( command && command.execute( this.getSurface() ) ) {
+ e.preventDefault();
+ }
+ }
+};
+
+/**
* Create a surface.
*
* @method
@@ -177,7 +235,11 @@
*/
ve.init.Target.prototype.createSurface = function ( dmDoc, config ) {
config = ve.extendObject( {
- excludeCommands: this.constructor.static.excludeCommands,
+ excludeCommands: OO.simpleArrayUnion(
+ this.constructor.static.excludeCommands,
+ this.constructor.static.documentCommands,
+ this.constructor.static.targetCommands
+ ),
importRules: this.constructor.static.importRules
}, config );
return new ve.ui.DesktopSurface( dmDoc, config );
diff --git a/src/ui/dialogs/ve.ui.FindAndReplaceDialog.js
b/src/ui/dialogs/ve.ui.FindAndReplaceDialog.js
index 1375b52..d711ca2 100644
--- a/src/ui/dialogs/ve.ui.FindAndReplaceDialog.js
+++ b/src/ui/dialogs/ve.ui.FindAndReplaceDialog.js
@@ -127,7 +127,6 @@
this.previousButton.connect( this, { click: 'onPreviousButtonClick' } );
this.replaceButton.connect( this, { click: 'onReplaceButtonClick' } );
this.replaceAllButton.connect( this, { click: 'onReplaceAllButtonClick'
} );
- this.$content.on( 'keydown', this.onContentKeyDown.bind( this ) );
// Initialization
this.findText.$input.attr( 'tabIndex', 1 );
@@ -194,20 +193,6 @@
this.fragment = [];
this.surface = null;
}, this );
-};
-
-/**
- * Handle keydown events inside the dialog
- */
-ve.ui.FindAndReplaceDialog.prototype.onContentKeyDown = function ( e ) {
- var command, trigger = new ve.ui.Trigger( e );
- if ( trigger.isComplete() ) {
- command = this.surface.getCommandByTrigger( trigger.toString()
);
- if ( command && command.getName() === 'findAndReplace' ) {
- e.preventDefault();
- this.close();
- }
- }
};
/**
diff --git a/src/ui/dialogs/ve.ui.ToolbarDialog.js
b/src/ui/dialogs/ve.ui.ToolbarDialog.js
index 8a221e6..160baca 100644
--- a/src/ui/dialogs/ve.ui.ToolbarDialog.js
+++ b/src/ui/dialogs/ve.ui.ToolbarDialog.js
@@ -29,4 +29,19 @@
OO.inheritClass( ve.ui.ToolbarDialog, OO.ui.Dialog );
+/* Methods */
+
+/**
+ * @inheritdoc
+ */
+ve.ui.ToolbarDialog.prototype.initialize = function () {
+ // Parent method
+ ve.ui.ToolbarDialog.super.prototype.initialize.call( this );
+
+ // Events
+ // Hack: required for keystrokes from isolated windows to make it back
to the target
+ this.$content.on( 'keydown', ve.init.target.onDocumentKeyDown.bind(
ve.init.target ) );
+ this.$content.on( 'keydown', ve.init.target.onTargetKeyDown.bind(
ve.init.target ) );
+};
+
ve.ui.ToolbarDialog.static.size = 'full';
diff --git a/src/ui/inspectors/ve.ui.FragmentInspector.js
b/src/ui/inspectors/ve.ui.FragmentInspector.js
index 5110783..4f31473 100644
--- a/src/ui/inspectors/ve.ui.FragmentInspector.js
+++ b/src/ui/inspectors/ve.ui.FragmentInspector.js
@@ -73,6 +73,9 @@
// Events
this.form.connect( this, { submit: 'onFormSubmit' } );
+ // Hack: required for keystrokes from isolated windows to make it back
to the target
+ this.$content.on( 'keydown', ve.init.target.onDocumentKeyDown.bind(
ve.init.target ) );
+ this.$content.on( 'keydown', ve.init.target.onTargetKeyDown.bind(
ve.init.target ) );
// Initialization
this.$element.addClass( 've-ui-fragmentInspector' );
diff --git a/src/ui/ve.ui.Surface.js b/src/ui/ve.ui.Surface.js
index ca3280e..1dc30f5 100644
--- a/src/ui/ve.ui.Surface.js
+++ b/src/ui/ve.ui.Surface.js
@@ -35,6 +35,9 @@
this.$blockers = this.$( '<div>' );
this.$controls = this.$( '<div>' );
this.$menus = this.$( '<div>' );
+ this.triggerListener = new ve.TriggerListener( OO.simpleArrayDifference(
+ Object.keys( ve.ui.commandRegistry.registry ),
config.excludeCommands || []
+ ) );
if ( dataOrDoc instanceof ve.dm.Document ) {
// ve.dm.Document
documentModel = dataOrDoc;
@@ -48,9 +51,6 @@
this.model = new ve.dm.Surface( documentModel );
this.view = new ve.ce.Surface( this.model, this, { $: this.$ } );
this.dialogs = this.createDialogWindowManager();
- this.commands = [];
- this.commandsByTrigger = {};
- this.triggers = {};
this.importRules = config.importRules || {};
this.enabled = true;
this.context = this.createContext();
@@ -68,7 +68,6 @@
} );
// Initialization
- this.setupCommands( config.excludeCommands );
this.$menus.append( this.context.$element );
this.$element
.addClass( 've-ui-surface' )
@@ -263,28 +262,6 @@
};
/**
- * Get command associated with trigger string.
- *
- * @method
- * @param {string} trigger Trigger string
- * @returns {ve.ui.Command|undefined} Command
- */
-ve.ui.Surface.prototype.getCommandByTrigger = function ( trigger ) {
- return this.commandsByTrigger[trigger];
-};
-
-/**
- * Get triggers for a specified name.
- *
- * @method
- * @param {string} name Trigger name
- * @returns {ve.ui.Trigger[]} Triggers
- */
-ve.ui.Surface.prototype.getTriggers = function ( name ) {
- return this.triggers[name];
-};
-
-/**
* Disable editing.
*
* @method
@@ -323,7 +300,7 @@
}
if ( triggerOrAction instanceof ve.ui.Trigger ) {
- command = this.getCommandByTrigger( triggerOrAction.toString()
);
+ command = this.triggerListener.getCommandByTrigger(
triggerOrAction.toString() );
if ( command ) {
// Have command call execute with action arguments
return command.execute( this );
@@ -338,35 +315,6 @@
}
}
return false;
-};
-
-/**
- * Setup command and trigger list based on rules.
- *
- * @method
- * @param {string[]} [excludeCommands] List of commands to exclude
- * @throws {Error} If trigger is not complete
- */
-ve.ui.Surface.prototype.setupCommands = function ( excludeCommands ) {
- var i, name, triggers, trigger, key;
- for ( name in ve.ui.commandRegistry.registry ) {
- if ( !excludeCommands || ve.indexOf( name, excludeCommands )
=== -1 ) {
- this.commands.push( name );
- triggers = ve.ui.triggerRegistry.lookup( name );
- if ( triggers ) {
- for ( i = triggers.length - 1; i >= 0; i-- ) {
- trigger = triggers[i];
- key = trigger.toString();
- // Validate trigger
- if ( key.length === 0 ) {
- throw new Error( 'Incomplete
trigger: ' + trigger );
- }
- this.commandsByTrigger[key] =
ve.ui.commandRegistry.registry[name];
- }
- this.triggers[name] = triggers;
- }
- }
- }
};
/**
diff --git a/src/ui/ve.ui.TargetToolbar.js b/src/ui/ve.ui.TargetToolbar.js
index 00e9816..acb2b27 100644
--- a/src/ui/ve.ui.TargetToolbar.js
+++ b/src/ui/ve.ui.TargetToolbar.js
@@ -17,7 +17,7 @@
*/
ve.ui.TargetToolbar = function VeUiTargetToolbar( target, surface, options ) {
// Parent constructor
- ve.ui.Toolbar.call( this, surface, options );
+ ve.ui.TargetToolbar.super.call( this, surface, options );
// Properties
this.target = target;
@@ -37,3 +37,23 @@
ve.ui.TargetToolbar.prototype.getTarget = function () {
return this.target;
};
+
+/**
+ * @inheritdoc
+ */
+ve.ui.TargetToolbar.prototype.getTriggers = function ( name ) {
+ var triggers = ve.ui.TargetToolbar.super.prototype.getTriggers.apply(
this, arguments );
+ return triggers ||
+ this.getTarget().targetTriggerListener.getTriggers( name ) ||
+ this.getTarget().documentTriggerListener.getTriggers( name );
+};
+
+/**
+ * @inheritdoc
+ */
+ve.ui.TargetToolbar.prototype.getCommands = function () {
+ return ve.ui.TargetToolbar.super.prototype.getCommands.apply( this,
arguments ).concat(
+ this.getTarget().targetTriggerListener.getCommands(),
+ this.getTarget().documentTriggerListener.getCommands()
+ );
+};
diff --git a/src/ui/ve.ui.Toolbar.js b/src/ui/ve.ui.Toolbar.js
index 9fd5f7a..43bb99f 100644
--- a/src/ui/ve.ui.Toolbar.js
+++ b/src/ui/ve.ui.Toolbar.js
@@ -84,7 +84,7 @@
}
// FIXME should use .static.getCommandName(), but we have tools that
aren't ve.ui.Tool subclasses :(
commandName = tool.static.commandName;
- return !commandName || ve.indexOf( commandName,
this.getSurface().commands ) !== -1;
+ return !commandName || ve.indexOf( commandName, this.getCommands() )
!== -1;
};
/**
@@ -192,10 +192,29 @@
};
/**
+ * Get triggers for a specified name.
+ *
+ * @param {string} name Trigger name
+ * @returns {ve.ui.Trigger[]|undefined} Triggers
+ */
+ve.ui.Toolbar.prototype.getTriggers = function ( name ) {
+ return this.getSurface().triggerListener.getTriggers( name );
+};
+
+/**
+ * Get a list of commands available to this toolbar's surface
+ *
+ * @return {string[]} Command names
+ */
+ve.ui.Toolbar.prototype.getCommands = function () {
+ return this.getSurface().triggerListener.getCommands();
+};
+
+/**
* @inheritdoc
*/
ve.ui.Toolbar.prototype.getToolAccelerator = function ( name ) {
- var i, l, triggers = this.surface.getTriggers( name ), shortcuts = [];
+ var i, l, triggers = this.getTriggers( name ), shortcuts = [];
if ( triggers ) {
for ( i = 0, l = triggers.length; i < l; i++ ) {
diff --git a/src/ui/ve.ui.TriggerRegistry.js b/src/ui/ve.ui.TriggerRegistry.js
index bc620b0..05577f3 100644
--- a/src/ui/ve.ui.TriggerRegistry.js
+++ b/src/ui/ve.ui.TriggerRegistry.js
@@ -33,6 +33,7 @@
* @param {ve.ui.Trigger[]|Object} triggers Trigger object(s) or map of
trigger object(s) keyed by
* platform name e.g. 'mac' or 'pc'
* @throws {Error} Trigger must be an instance of ve.ui.Trigger
+ * @throws {Error} Incomplete trigger
*/
ve.ui.TriggerRegistry.prototype.register = function ( name, triggers ) {
var i, l, triggerList,
@@ -54,6 +55,9 @@
if ( !( triggerList[i] instanceof ve.ui.Trigger ) ) {
throw new Error( 'Trigger must be an instance of
ve.ui.Trigger' );
}
+ if ( !triggerList[i].isComplete() ) {
+ throw new Error( 'Incomplete trigger' );
+ }
}
OO.Registry.prototype.register.call( this, name, triggerList );
diff --git a/src/ve.TriggerListener.js b/src/ve.TriggerListener.js
new file mode 100644
index 0000000..417fe1b
--- /dev/null
+++ b/src/ve.TriggerListener.js
@@ -0,0 +1,80 @@
+/*!
+ * VisualEditor UserInterface TriggerListener class.
+ *
+ * @copyright 2011-2014 VisualEditor Team and others; see
http://ve.mit-license.org
+ */
+
+/**
+ * Trigger listener
+ *
+ * @class
+ *
+ * @constructor
+ * @param {string[]} commands Commands to listen to triggers for
+ */
+ve.TriggerListener = function VeUiTriggerListener( commands ) {
+ // Properties
+ this.commands = [];
+ this.commandsByTrigger = {};
+ this.triggers = {};
+
+ this.setupCommands( commands );
+};
+
+/* Inheritance */
+
+OO.initClass( ve.TriggerListener );
+
+/* Methods */
+
+/**
+ * Setup commands
+ *
+ * @param {string[]} commands Commands to listen to triggers for
+ */
+ve.TriggerListener.prototype.setupCommands = function ( commands ) {
+ var i, j, command, triggers;
+ this.commands = commands;
+ if ( commands.length ) {
+ for ( i = this.commands.length - 1; i >= 0; i-- ) {
+ command = this.commands[i];
+ triggers = ve.ui.triggerRegistry.lookup( command );
+ if ( triggers ) {
+ for ( j = triggers.length - 1; j >= 0; j-- ) {
+
this.commandsByTrigger[triggers[j].toString()] = ve.ui.commandRegistry.lookup(
command );
+ }
+ this.triggers[command] = triggers;
+ }
+ }
+ }
+};
+
+/**
+ * Get list of commands.
+ *
+ * @returns {string[]} Commands
+ */
+ve.TriggerListener.prototype.getCommands = function () {
+ return this.commands;
+};
+
+/**
+ * Get command associated with trigger string.
+ *
+ * @method
+ * @param {string} trigger Trigger string
+ * @returns {ve.ui.Command|undefined} Command
+ */
+ve.TriggerListener.prototype.getCommandByTrigger = function ( trigger ) {
+ return this.commandsByTrigger[trigger];
+};
+
+/**
+ * Get triggers for a specified name.
+ *
+ * @param {string} name Trigger name
+ * @returns {ve.ui.Trigger[]|undefined} Triggers
+ */
+ve.TriggerListener.prototype.getTriggers = function ( name ) {
+ return this.triggers[name];
+};
diff --git a/tests/index.html b/tests/index.html
index 3eb07fa..3193c60 100644
--- a/tests/index.html
+++ b/tests/index.html
@@ -69,6 +69,7 @@
<!-- visualEditor.base.build -->
<script src="../src/ve.js"></script>
<script src="../src/ve.utils.js"></script>
+ <script src="../src/ve.TriggerListener.js"></script>
<script src="../src/ve.track.js"></script>
<script src="../src/init/ve.init.js"></script>
<script src="../src/init/ve.init.Platform.js"></script>
--
To view, visit https://gerrit.wikimedia.org/r/176955
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I10857cc66526ca77560cb1e8e78ff1c1a08f3736
Gerrit-PatchSet: 10
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>
Gerrit-Reviewer: Catrope <[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
