DLynch has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/250881

Change subject: [BREAKING CHANGE] Create CommandHelpRegistry for 
CommandHelpDialog
......................................................................

[BREAKING CHANGE] Create CommandHelpRegistry for CommandHelpDialog

It's easier to deal with command registration if there's a central
registry that isn't attached to the dialog class. This registry has
slightly more registration logic than normal, as it accounts for
registering new triggers / sequences on an existing command instead
of just overwriting it.

Change-Id: Id68383d7f5b44031ee41597b3c81ebbba54696ce
---
M build/modules.json
M demos/ve/desktop.html
M demos/ve/mobile.html
M src/ui/dialogs/ve.ui.CommandHelpDialog.js
A src/ui/ve.ui.CommandHelpRegistry.js
M src/ui/ve.ui.js
M tests/index.html
7 files changed, 157 insertions(+), 99 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor 
refs/changes/81/250881/1

diff --git a/build/modules.json b/build/modules.json
index 7779105..156ef59 100644
--- a/build/modules.json
+++ b/build/modules.json
@@ -406,6 +406,7 @@
                        "src/ui/ve.ui.ToolFactory.js",
                        "src/ui/ve.ui.Command.js",
                        "src/ui/ve.ui.CommandRegistry.js",
+                       "src/ui/ve.ui.CommandHelpRegistry.js",
                        "src/ui/ve.ui.Trigger.js",
                        "src/ui/ve.ui.TriggerRegistry.js",
                        "src/ui/ve.ui.Sequence.js",
diff --git a/demos/ve/desktop.html b/demos/ve/desktop.html
index 39f43a7..442d1fd 100644
--- a/demos/ve/desktop.html
+++ b/demos/ve/desktop.html
@@ -367,6 +367,7 @@
                <script src="../../src/ui/ve.ui.ToolFactory.js"></script>
                <script src="../../src/ui/ve.ui.Command.js"></script>
                <script src="../../src/ui/ve.ui.CommandRegistry.js"></script>
+               <script 
src="../../src/ui/ve.ui.CommandHelpRegistry.js"></script>
                <script src="../../src/ui/ve.ui.Trigger.js"></script>
                <script src="../../src/ui/ve.ui.TriggerRegistry.js"></script>
                <script src="../../src/ui/ve.ui.Sequence.js"></script>
diff --git a/demos/ve/mobile.html b/demos/ve/mobile.html
index 8f40031..44da8bb 100644
--- a/demos/ve/mobile.html
+++ b/demos/ve/mobile.html
@@ -369,6 +369,7 @@
                <script src="../../src/ui/ve.ui.ToolFactory.js"></script>
                <script src="../../src/ui/ve.ui.Command.js"></script>
                <script src="../../src/ui/ve.ui.CommandRegistry.js"></script>
+               <script 
src="../../src/ui/ve.ui.CommandHelpRegistry.js"></script>
                <script src="../../src/ui/ve.ui.Trigger.js"></script>
                <script src="../../src/ui/ve.ui.TriggerRegistry.js"></script>
                <script src="../../src/ui/ve.ui.Sequence.js"></script>
diff --git a/src/ui/dialogs/ve.ui.CommandHelpDialog.js 
b/src/ui/dialogs/ve.ui.CommandHelpDialog.js
index e932ebc..16789ec 100644
--- a/src/ui/dialogs/ve.ui.CommandHelpDialog.js
+++ b/src/ui/dialogs/ve.ui.CommandHelpDialog.js
@@ -41,82 +41,26 @@
 ve.ui.CommandHelpDialog.static.commandGroups = {
        textStyle: {
                title: 'visualeditor-shortcuts-text-style',
-               commands: {
-                       bold: { trigger: 'bold', msg: 
'visualeditor-annotationbutton-bold-tooltip' },
-                       italic: { trigger: 'italic', msg: 
'visualeditor-annotationbutton-italic-tooltip' },
-                       link: { trigger: 'link', msg: 
'visualeditor-annotationbutton-link-tooltip' },
-                       superscript: { trigger: 'superscript', msg: 
'visualeditor-annotationbutton-superscript-tooltip' },
-                       subscript: { trigger: 'subscript', msg: 
'visualeditor-annotationbutton-subscript-tooltip' },
-                       underline: { trigger: 'underline', msg: 
'visualeditor-annotationbutton-underline-tooltip' },
-                       code: { trigger: 'code', msg: 
'visualeditor-annotationbutton-code-tooltip' },
-                       strikethrough: { trigger: 'strikethrough', msg: 
'visualeditor-annotationbutton-strikethrough-tooltip' },
-                       clear: { trigger: 'clear', msg: 
'visualeditor-clearbutton-tooltip' }
-               },
                promote: [ 'bold', 'italic', 'link' ],
                demote: [ 'clear' ]
        },
        clipboard: {
                title: 'visualeditor-shortcuts-clipboard',
-               commands: {
-                       cut: {
-                               shortcuts: [ {
-                                       mac: 'cmd+x',
-                                       pc: 'ctrl+x'
-                               } ],
-                               msg: 'visualeditor-clipboard-cut'
-                       },
-                       copy: {
-                               shortcuts: [ {
-                                       mac: 'cmd+c',
-                                       pc: 'ctrl+c'
-                               } ],
-                               msg: 'visualeditor-clipboard-copy'
-                       },
-                       paste: {
-                               shortcuts: [ {
-                                       mac: 'cmd+v',
-                                       pc: 'ctrl+v'
-                               } ],
-                               msg: 'visualeditor-clipboard-paste'
-                       },
-                       pasteSpecial: { trigger: 'pasteSpecial', msg: 
'visualeditor-clipboard-paste-special' }
-               },
                promote: [],
                demote: []
        },
        formatting: {
                title: 'visualeditor-shortcuts-formatting',
-               commands: {
-                       paragraph: { trigger: 'paragraph', msg: 
'visualeditor-formatdropdown-format-paragraph' },
-                       heading: { shortcuts: [ 'ctrl+1-6' ], msg: 
'visualeditor-formatdropdown-format-heading-label' },
-                       pre: { trigger: 'preformatted', msg: 
'visualeditor-formatdropdown-format-preformatted' },
-                       blockquote: { trigger: 'blockquote', msg: 
'visualeditor-formatdropdown-format-blockquote' },
-                       indentIn: { trigger: 'indent', msg: 
'visualeditor-indentationbutton-indent-tooltip' },
-                       indentOut: { trigger: 'outdent', msg: 
'visualeditor-indentationbutton-outdent-tooltip' },
-                       listBullet: { sequence: [ 'bulletStar' ], msg: 
'visualeditor-listbutton-bullet-tooltip' },
-                       listNumber: { sequence: [ 'numberDot' ], msg: 
'visualeditor-listbutton-number-tooltip' }
-               },
                promote: [ 'paragraph', 'pre', 'blockquote' ],
                demote: []
        },
        history: {
                title: 'visualeditor-shortcuts-history',
-               commands: {
-                       undo: { trigger: 'undo', msg: 
'visualeditor-historybutton-undo-tooltip' },
-                       redo: { trigger: 'redo', msg: 
'visualeditor-historybutton-redo-tooltip' }
-               },
                promote: [ 'undo', 'redo' ],
                demote: []
        },
        other: {
                title: 'visualeditor-shortcuts-other',
-               commands: {
-                       findAndReplace: { trigger: 'findAndReplace', msg: 
'visualeditor-find-and-replace-title' },
-                       findNext: { trigger: 'findNext', msg: 
'visualeditor-find-and-replace-next-button' },
-                       findPrevious: { trigger: 'findPrevious', msg: 
'visualeditor-find-and-replace-previous-button' },
-                       selectAll: { trigger: 'selectAll', msg: 
'visualeditor-content-select-all' },
-                       commandHelp: { trigger: 'commandHelp', msg: 
'visualeditor-dialog-command-help-title' }
-               },
                promote: [ 'findAndReplace', 'findNext', 'findPrevious' ],
                demote: [ 'commandHelp' ]
        }
@@ -153,7 +97,7 @@
        this.$container = $( '<div>' ).addClass( 
've-ui-commandHelpDialog-container' );
 
        for ( i in commandGroups ) {
-               commands = 
ve.ui.CommandHelpDialog.static.sortedCommandsFromGroup( commandGroups[ i ] );
+               commands = 
ve.ui.CommandHelpDialog.static.sortedCommandsFromGroup( i, commandGroups[ i 
].promote, commandGroups[ i ].demote );
                $list = $( '<dl>' ).addClass( 've-ui-commandHelpDialog-list' );
                for ( j = 0, jLen = commands.length; j < jLen; j++ ) {
                        if ( commands[ j ].trigger ) {
@@ -229,66 +173,39 @@
 };
 
 /**
- * Register a command for display in the dialog
- *
- * @static
- * @param {string} groupName The group in which to display this
- * @param {string} commandName The key for the command; never displayed, but 
used in sorting
- * @param {Object} details The details about the command, used in display
- */
-ve.ui.CommandHelpDialog.static.registerCommand = function ( groupName, 
commandName, details ) {
-       var group = ve.ui.CommandHelpDialog.static.commandGroups[ groupName ];
-       if ( !group.commands[ commandName ] ) {
-               group.commands[ commandName ] = details;
-               return;
-       }
-       if ( details.trigger ) {
-               group.commands[ commandName ].trigger = details.trigger;
-       }
-       if ( details.shortcuts ) {
-               group.commands[ commandName ].shortcuts = details.shortcuts;
-       }
-       if ( details.sequence ) {
-               group.commands[ commandName ].sequence = ( group.commands[ 
commandName ].sequence || [] ).concat( details.sequence );
-       }
-       if ( details.promote ) {
-               group.promote.push( commandName );
-       } else if ( details.demote ) {
-               group.demote.push( commandName );
-       }
-};
-
-/**
  * Extract a properly sorted list of commands from a command-group
  *
  * @static
- * @param {Object} group Group of related commands
- * @return {string[]} List of commands
+ * @param {string} groupName The dialog-category in which to display this
+ * @param {string[]} promote Commands which should be displayed first
+ * @param {string[]} demote Commands which should be displayed last
+ * @return {Object[]} List of commands in order
  */
-ve.ui.CommandHelpDialog.static.sortedCommandsFromGroup = function ( group ) {
+ve.ui.CommandHelpDialog.static.sortedCommandsFromGroup = function ( groupName, 
promote, demote ) {
        var i,
-               keys = Object.keys( group.commands ),
+               commands = ve.ui.commandHelpRegistry.lookupByGroup( groupName ),
+               keys = Object.keys( commands )
                used = {},
                auto = [],
                promoted = [],
                demoted = [];
        keys.sort();
-       for ( i = 0; i < group.promote.length; i++ ) {
-               promoted.push( group.commands[ group.promote[ i ] ] );
-               used[ group.promote[ i ] ] = true;
+       for ( i = 0; i < promote.length; i++ ) {
+               promoted.push( commands[ promote[ i ] ] );
+               used[ promote[ i ] ] = true;
        }
-       for ( i = 0; i < group.demote.length; i++ ) {
-               if ( used[ group.demote[ i ] ] ) {
+       for ( i = 0; i < demote.length; i++ ) {
+               if ( used[ demote[ i ] ] ) {
                        continue;
                }
-               demoted.push( group.commands[ group.demote[ i ] ] );
-               used[ group.demote[ i ] ] = true;
+               demoted.push( commands[ demote[ i ] ] );
+               used[ demote[ i ] ] = true;
        }
        for ( i = 0; i < keys.length; i++ ) {
                if ( used[ keys[ i ] ] ) {
                        continue;
                }
-               auto.push( group.commands[ keys[ i ] ] );
+               auto.push( commands[ keys[ i ] ] );
        }
        return promoted.concat( auto, demoted );
 };
diff --git a/src/ui/ve.ui.CommandHelpRegistry.js 
b/src/ui/ve.ui.CommandHelpRegistry.js
new file mode 100644
index 0000000..5e73b60
--- /dev/null
+++ b/src/ui/ve.ui.CommandHelpRegistry.js
@@ -0,0 +1,136 @@
+/*!
+ * VisualEditor CommandRegistry class.
+ *
+ * @copyright 2011-2015 VisualEditor Team and others; see 
http://ve.mit-license.org
+ */
+
+/**
+ * Command registry.
+ *
+ * @class
+ * @extends OO.Registry
+ * @constructor
+ */
+ve.ui.CommandHelpRegistry = function VeUiCommandHelpRegistry() {
+       // Parent constructor
+       OO.Registry.call( this );
+};
+
+/* Inheritance */
+
+OO.inheritClass( ve.ui.CommandHelpRegistry, OO.Registry );
+
+/* Methods */
+
+/**
+ * Register a command for display in the dialog.
+ *
+ * @static
+ * @param {string} groupName Dialog-category in which to display this
+ * @param {string} commandName Name of the command
+ * @param {Object} details Details about the command
+ */
+ve.ui.CommandHelpRegistry.prototype.register = function ( groupName, 
commandName, details ) {
+       var existingCommand;
+
+       existingCommand = this.registry[ commandName ];
+       if ( existingCommand ) {
+               // This is _almost_ just doing extend(existingCommand, details)
+               // But some values need special handling, so we can't do that.
+               if ( details.msg ) {
+                       existingCommand.msg = details.msg;
+               }
+               if ( details.trigger ) {
+                       existingCommand.trigger = details.trigger;
+               }
+               if ( details.shortcuts ) {
+                       existingCommand.shortcuts = details.shortcuts;
+               }
+               if ( details.sequence ) {
+                       existingCommand.sequence = ( existingCommand.sequence 
|| [] ).concat( details.sequence );
+               }
+               details = existingCommand;
+       }
+
+       details.group = groupName;
+
+       OO.Registry.prototype.register.call( this, commandName, details );
+};
+
+/**
+ * Get data for a given group of commands.
+ *
+ * @param {string} groupName Group name
+ * @return {Object} Commands associated with the group
+ */
+ve.ui.CommandHelpRegistry.prototype.lookupByGroup = function ( groupName ) {
+       var commandName, matches = {};
+       for ( commandName in this.registry ) {
+               if ( groupName === this.registry[ commandName ].group ) {
+                       matches[ commandName ] = this.registry[ commandName ];
+               }
+       }
+       return matches;
+};
+
+/* Initialization */
+
+ve.ui.commandHelpRegistry = new ve.ui.CommandHelpRegistry();
+
+/* Registrations */
+
+// Text styles
+ve.ui.commandHelpRegistry.register( 'textStyle', 'bold', { trigger: 'bold', 
msg: 'visualeditor-annotationbutton-bold-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'textStyle', 'italic', { trigger: 
'italic', msg: 'visualeditor-annotationbutton-italic-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'textStyle', 'link', { trigger: 'link', 
msg: 'visualeditor-annotationbutton-link-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'textStyle', 'superscript', { trigger: 
'superscript', msg: 'visualeditor-annotationbutton-superscript-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'textStyle', 'subscript', { trigger: 
'subscript', msg: 'visualeditor-annotationbutton-subscript-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'textStyle', 'underline', { trigger: 
'underline', msg: 'visualeditor-annotationbutton-underline-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'textStyle', 'code', { trigger: 'code', 
msg: 'visualeditor-annotationbutton-code-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'textStyle', 'strikethrough', { trigger: 
'strikethrough', msg: 'visualeditor-annotationbutton-strikethrough-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'textStyle', 'clear', { trigger: 'clear', 
msg: 'visualeditor-clearbutton-tooltip' } );
+
+// Clipboard
+ve.ui.commandHelpRegistry.register( 'clipboard', 'cut', {
+       shortcuts: [ {
+               mac: 'cmd+x',
+               pc: 'ctrl+x'
+       } ],
+       msg: 'visualeditor-clipboard-cut'
+} );
+ve.ui.commandHelpRegistry.register( 'clipboard', 'copy', {
+       shortcuts: [ {
+               mac: 'cmd+c',
+               pc: 'ctrl+c'
+       } ],
+       msg: 'visualeditor-clipboard-copy'
+} );
+ve.ui.commandHelpRegistry.register( 'clipboard', 'paste', {
+       shortcuts: [ {
+               mac: 'cmd+v',
+               pc: 'ctrl+v'
+       } ],
+       msg: 'visualeditor-clipboard-paste'
+} );
+ve.ui.commandHelpRegistry.register( 'clipboard', 'pasteSpecial', { trigger: 
'pasteSpecial', msg: 'visualeditor-clipboard-paste-special' } );
+
+// Formatting
+ve.ui.commandHelpRegistry.register( 'formatting', 'paragraph', { trigger: 
'paragraph', msg: 'visualeditor-formatdropdown-format-paragraph' } );
+ve.ui.commandHelpRegistry.register( 'formatting', 'heading', { shortcuts: [ 
'ctrl+1-6' ], msg: 'visualeditor-formatdropdown-format-heading-label' } );
+ve.ui.commandHelpRegistry.register( 'formatting', 'pre', { trigger: 
'preformatted', msg: 'visualeditor-formatdropdown-format-preformatted' } );
+ve.ui.commandHelpRegistry.register( 'formatting', 'blockquote', { trigger: 
'blockquote', msg: 'visualeditor-formatdropdown-format-blockquote' } );
+ve.ui.commandHelpRegistry.register( 'formatting', 'indentIn', { trigger: 
'indent', msg: 'visualeditor-indentationbutton-indent-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'formatting', 'indentOut', { trigger: 
'outdent', msg: 'visualeditor-indentationbutton-outdent-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'formatting', 'listBullet', { sequence: [ 
'bulletStar' ], msg: 'visualeditor-listbutton-bullet-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'formatting', 'listNumber', { sequence: [ 
'numberDot' ], msg: 'visualeditor-listbutton-number-tooltip' } );
+
+// History
+ve.ui.commandHelpRegistry.register( 'history', 'undo', { trigger: 'undo', msg: 
'visualeditor-historybutton-undo-tooltip' } );
+ve.ui.commandHelpRegistry.register( 'history', 'redo', { trigger: 'redo', msg: 
'visualeditor-historybutton-redo-tooltip' } );
+
+// Other
+ve.ui.commandHelpRegistry.register( 'other', 'findAndReplace', { trigger: 
'findAndReplace', msg: 'visualeditor-find-and-replace-title' } );
+ve.ui.commandHelpRegistry.register( 'other', 'findNext', { trigger: 
'findNext', msg: 'visualeditor-find-and-replace-next-button' } );
+ve.ui.commandHelpRegistry.register( 'other', 'findPrevious', { trigger: 
'findPrevious', msg: 'visualeditor-find-and-replace-previous-button' } );
+ve.ui.commandHelpRegistry.register( 'other', 'selectAll', { trigger: 
'selectAll', msg: 'visualeditor-content-select-all' } );
+ve.ui.commandHelpRegistry.register( 'other', 'commandHelp', { trigger: 
'commandHelp', msg: 'visualeditor-dialog-command-help-title' } );
diff --git a/src/ui/ve.ui.js b/src/ui/ve.ui.js
index d06b4c3..7e07476 100644
--- a/src/ui/ve.ui.js
+++ b/src/ui/ve.ui.js
@@ -13,6 +13,7 @@
 ve.ui = {
        // 'actionFactory' instantiated in ve.ui.ActionFactory.js
        // 'commandRegistry' instantiated in ve.ui.CommandRegistry.js
+       // 'commandHelpRegistry' instantiated in ve.ui.CommandHelpRegistry.js
        // 'triggerRegistry' instantiated in ve.ui.TriggerRegistry.js
        // 'toolFactory' instantiated in ve.ui.ToolFactory.js
        // 'contextItemFactory' instantiated in ve.ui.ContextItemFactory.js
diff --git a/tests/index.html b/tests/index.html
index 3310e91..724343d 100644
--- a/tests/index.html
+++ b/tests/index.html
@@ -293,6 +293,7 @@
                <script src="../src/ui/ve.ui.ToolFactory.js"></script>
                <script src="../src/ui/ve.ui.Command.js"></script>
                <script src="../src/ui/ve.ui.CommandRegistry.js"></script>
+               <script src="../src/ui/ve.ui.CommandHelpRegistry.js"></script>
                <script src="../src/ui/ve.ui.Trigger.js"></script>
                <script src="../src/ui/ve.ui.TriggerRegistry.js"></script>
                <script src="../src/ui/ve.ui.Sequence.js"></script>

-- 
To view, visit https://gerrit.wikimedia.org/r/250881
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id68383d7f5b44031ee41597b3c81ebbba54696ce
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: DLynch <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to