jenkins-bot has submitted this change and it was merged.

Change subject: [BREAKING CHANGE] Include sequences in the command help dialog
......................................................................


[BREAKING CHANGE] Include sequences in the command help dialog

The command help dialog currently only shows keyboard shortcuts. If we
use the sequence registry, we can add the sequences to the relevant
commands. As part of this, enhance the dialog to better distinguish
between key-combinations and key-sequences.

Currently ignored in this is a way to represent "you must be at the
start of a paragraph".

Bug: T116013
Change-Id: I21fd47f5809dce36d10edc483121ecbdc12071f6
---
M i18n/en.json
M i18n/qqq.json
M src/ui/dialogs/ve.ui.CommandHelpDialog.js
M src/ui/styles/dialogs/ve.ui.CommandHelpDialog.css
M src/ui/ve.ui.Sequence.js
M src/ui/ve.ui.Trigger.js
6 files changed, 265 insertions(+), 97 deletions(-)

Approvals:
  Esanders: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/i18n/en.json b/i18n/en.json
index 4f293e7..e6cafcd 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -108,6 +108,7 @@
        "visualeditor-shortcuts-formatting": "Paragraph formatting",
        "visualeditor-shortcuts-history": "History",
        "visualeditor-shortcuts-other": "Other",
+       "visualeditor-shortcuts-sequence-notice": "Type",
        "visualeditor-shortcuts-text-style": "Text styling",
        "visualeditor-slug-insert": "Insert paragraph",
        "visualeditor-specialcharacter-button-tooltip": "Special character",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index d46786c..1e56a55 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -116,6 +116,7 @@
        "visualeditor-shortcuts-formatting": "Heading for paragraph formatting 
shortcuts",
        "visualeditor-shortcuts-history": "Heading for history 
shortcuts.\n\nSee also:\n* 
{{msg-mw|Visualeditor-historybutton-redo-tooltip}}\n* 
{{msg-mw|Visualeditor-historybutton-undo-tooltip}}\n{{Identical|History}}",
        "visualeditor-shortcuts-other": "Heading for other 
shortcuts.\n{{Identical|Other}}",
+       "visualeditor-shortcuts-sequence-notice": "Notice before a 
text-sequence shortcut that it should be typed out instead of being several 
keys to press at once",
        "visualeditor-shortcuts-text-style": "Heading for text formatting 
shortcuts",
        "visualeditor-slug-insert": "Label for slug to insert paragraph on an 
otherwise unreachable line",
        "visualeditor-specialcharacter-button-tooltip": "Tooltip text for the 
insert character button.\n{{Identical|Special character}}",
diff --git a/src/ui/dialogs/ve.ui.CommandHelpDialog.js 
b/src/ui/dialogs/ve.ui.CommandHelpDialog.js
index f3c0b49..76347d5 100644
--- a/src/ui/dialogs/ve.ui.CommandHelpDialog.js
+++ b/src/ui/dialogs/ve.ui.CommandHelpDialog.js
@@ -38,6 +38,90 @@
        }
 ];
 
+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' ]
+       }
+};
+
 /* Methods */
 
 /**
@@ -52,14 +136,14 @@
  */
 ve.ui.CommandHelpDialog.prototype.initialize = function () {
        var i, j, jLen, k, kLen, triggerList, commands, shortcut, platform, 
platformKey,
-               $list, $shortcut, commandGroups;
+               $list, $shortcut, commandGroups, sequence;
 
        // Parent method
        ve.ui.CommandHelpDialog.super.prototype.initialize.call( this );
 
        platform = ve.getSystemPlatform();
        platformKey = platform === 'mac' ? 'mac' : 'pc';
-       commandGroups = this.constructor.static.getCommandGroups();
+       commandGroups = ve.ui.CommandHelpDialog.static.commandGroups;
 
        this.contentLayout = new OO.ui.PanelLayout( {
                scrollable: true,
@@ -69,28 +153,43 @@
        this.$container = $( '<div>' ).addClass( 
've-ui-commandHelpDialog-container' );
 
        for ( i in commandGroups ) {
-               commands = commandGroups[ i ].commands;
+               commands = 
ve.ui.CommandHelpDialog.static.sortedCommandsFromGroup( commandGroups[ i ] );
                $list = $( '<dl>' ).addClass( 've-ui-commandHelpDialog-list' );
                for ( j = 0, jLen = commands.length; j < jLen; j++ ) {
                        if ( commands[ j ].trigger ) {
                                triggerList = ve.ui.triggerRegistry.lookup( 
commands[ j ].trigger );
                        } else {
                                triggerList = [];
-                               for ( k = 0, kLen = commands[ j 
].shortcuts.length; k < kLen; k++ ) {
-                                       shortcut = commands[ j ].shortcuts[ k ];
-                                       triggerList.push(
-                                               new ve.ui.Trigger(
-                                                       ve.isPlainObject( 
shortcut ) ? shortcut[ platformKey ] : shortcut,
-                                                       true
-                                               )
-                                       );
+                               if ( commands[ j ].shortcuts ) {
+                                       for ( k = 0, kLen = commands[ j 
].shortcuts.length; k < kLen; k++ ) {
+                                               shortcut = commands[ j 
].shortcuts[ k ];
+                                               triggerList.push(
+                                                       new ve.ui.Trigger(
+                                                               
ve.isPlainObject( shortcut ) ? shortcut[ platformKey ] : shortcut,
+                                                               true
+                                                       )
+                                               );
+                                       }
                                }
                        }
                        $shortcut = $( '<dt>' );
                        for ( k = 0, kLen = triggerList.length; k < kLen; k++ ) 
{
-                               $shortcut.append( $( '<kbd>' ).text(
-                                       triggerList[ k ].getMessage().replace( 
/\+/g, ' + ' )
-                               ) );
+                               $shortcut.append( $( '<kbd>' ).append(
+                                       triggerList[ k ].getMessage( true 
).map( ve.ui.CommandHelpDialog.static.buildKeyNode )
+                               ).find( 'kbd + kbd' ).before( '+' ).end() );
+                       }
+                       if ( commands[ j ].sequence ) {
+                               for ( k = 0, kLen = commands[ j 
].sequence.length; k < kLen; k++ ) {
+                                       sequence = 
ve.ui.sequenceRegistry.lookup( commands[ j ].sequence[ k ] );
+                                       if ( sequence ) {
+                                               $shortcut.append( $( '<kbd 
class="ve-ui-commandHelpDialog-sequence">' )
+                                                       .attr( 'data-label', 
ve.msg( 'visualeditor-shortcuts-sequence-notice' ) )
+                                                       .append(
+                                                               
sequence.getMessage( true ).map( ve.ui.CommandHelpDialog.static.buildKeyNode )
+                                                       )
+                                               );
+                                       }
+                               }
                        }
                        $list.append(
                                $shortcut,
@@ -114,83 +213,84 @@
 /* Static methods */
 
 /**
- * Get the list of commands, grouped by type
+ * Wrap a key (as provided by a Trigger) in a node, for display
  *
  * @static
- * @return {Object} Object containing command groups, consist of a title 
message and array of commands
+ * @return {jQuery} A kbd wrapping the key text
  */
-ve.ui.CommandHelpDialog.static.getCommandGroups = function () {
-       return {
-               textStyle: {
-                       title: 'visualeditor-shortcuts-text-style',
-                       commands: [
-                               { trigger: 'bold', msg: 
'visualeditor-annotationbutton-bold-tooltip' },
-                               { trigger: 'italic', msg: 
'visualeditor-annotationbutton-italic-tooltip' },
-                               { trigger: 'link', msg: 
'visualeditor-annotationbutton-link-tooltip' },
-                               { trigger: 'superscript', msg: 
'visualeditor-annotationbutton-superscript-tooltip' },
-                               { trigger: 'subscript', msg: 
'visualeditor-annotationbutton-subscript-tooltip' },
-                               { trigger: 'underline', msg: 
'visualeditor-annotationbutton-underline-tooltip' },
-                               { trigger: 'code', msg: 
'visualeditor-annotationbutton-code-tooltip' },
-                               { trigger: 'strikethrough', msg: 
'visualeditor-annotationbutton-strikethrough-tooltip' },
-                               { trigger: 'clear', msg: 
'visualeditor-clearbutton-tooltip' }
-                       ]
-               },
-               clipboard: {
-                       title: 'visualeditor-shortcuts-clipboard',
-                       commands: [
-                               {
-                                       shortcuts: [ {
-                                               mac: 'cmd+x',
-                                               pc: 'ctrl+x'
-                                       } ],
-                                       msg: 'visualeditor-clipboard-cut'
-                               },
-                               {
-                                       shortcuts: [ {
-                                               mac: 'cmd+c',
-                                               pc: 'ctrl+c'
-                                       } ],
-                                       msg: 'visualeditor-clipboard-copy'
-                               },
-                               {
-                                       shortcuts: [ {
-                                               mac: 'cmd+v',
-                                               pc: 'ctrl+v'
-                                       } ],
-                                       msg: 'visualeditor-clipboard-paste'
-                               },
-                               { trigger: 'pasteSpecial', msg: 
'visualeditor-clipboard-paste-special' }
-                       ]
-               },
-               formatting: {
-                       title: 'visualeditor-shortcuts-formatting',
-                       commands: [
-                               { trigger: 'paragraph', msg: 
'visualeditor-formatdropdown-format-paragraph' },
-                               { shortcuts: [ 'ctrl+(1-6)' ], msg: 
'visualeditor-formatdropdown-format-heading-label' },
-                               { trigger: 'preformatted', msg: 
'visualeditor-formatdropdown-format-preformatted' },
-                               { trigger: 'blockquote', msg: 
'visualeditor-formatdropdown-format-blockquote' },
-                               { trigger: 'indent', msg: 
'visualeditor-indentationbutton-indent-tooltip' },
-                               { trigger: 'outdent', msg: 
'visualeditor-indentationbutton-outdent-tooltip' }
-                       ]
-               },
-               history: {
-                       title: 'visualeditor-shortcuts-history',
-                       commands: [
-                               { trigger: 'undo', msg: 
'visualeditor-historybutton-undo-tooltip' },
-                               { trigger: 'redo', msg: 
'visualeditor-historybutton-redo-tooltip' }
-                       ]
-               },
-               other: {
-                       title: 'visualeditor-shortcuts-other',
-                       commands: [
-                               { trigger: 'findAndReplace', msg: 
'visualeditor-find-and-replace-title' },
-                               { trigger: 'findNext', msg: 
'visualeditor-find-and-replace-next-button' },
-                               { trigger: 'findPrevious', msg: 
'visualeditor-find-and-replace-previous-button' },
-                               { trigger: 'selectAll', msg: 
'visualeditor-content-select-all' },
-                               { trigger: 'commandHelp', msg: 
'visualeditor-dialog-command-help-title' }
-                       ]
+ve.ui.CommandHelpDialog.static.buildKeyNode = function ( key ) {
+       if ( key === ' ' ) {
+               // Might need to expand this if other keys show up, but 
currently things like
+               // the tab-character only come from Triggers and are 
pre-localized there into
+               // "tab" anyway.
+               key = 'space';
+       }
+       return $( '<kbd>' ).attr( 'data-key', key ).text( key );
+};
+
+/**
+ * Register a command for display in the dialog
+ *
+ * @static
+ * @param {string} category The dialog-category 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 ( category, 
commandName, details ) {
+       var group = ve.ui.CommandHelpDialog.static.commandGroups[ category ];
+       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
+ */
+ve.ui.CommandHelpDialog.static.sortedCommandsFromGroup = function ( group ) {
+       var i,
+               keys = Object.keys( group.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 < group.demote.length; i++ ) {
+               if ( used[ group.demote[ i ] ] ) {
+                       continue;
                }
-       };
+               demoted.push( group.commands[ group.demote[ i ] ] );
+               used[ group.demote[ i ] ] = true;
+       }
+       for ( i = 0; i < keys.length; i++ ) {
+               if ( used[ keys[ i ] ] ) {
+                       continue;
+               }
+               auto.push( group.commands[ keys[ i ] ] );
+       }
+       return promoted.concat( auto, demoted );
 };
 
 /* Registration */
diff --git a/src/ui/styles/dialogs/ve.ui.CommandHelpDialog.css 
b/src/ui/styles/dialogs/ve.ui.CommandHelpDialog.css
index 41f4e92..2712d31 100644
--- a/src/ui/styles/dialogs/ve.ui.CommandHelpDialog.css
+++ b/src/ui/styles/dialogs/ve.ui.CommandHelpDialog.css
@@ -45,23 +45,61 @@
        width: 55%;
        padding-right: 1em;
        text-align: right;
-       font-weight: bold;
 }
 
 .ve-ui-commandHelpDialog-list dt kbd {
        font-family: monospace;
+       font-weight: bold;
+       font-style: normal;
+}
+
+.ve-ui-commandHelpDialog-list dt > kbd {
        display: block;
        clear: right;
 }
 
 /* Enlarge vertical spacing in a list of shortcuts for one action */
 .ve-ui-commandHelpDialog-list dt kbd ~ kbd {
-       margin-top: 0.2em;
+       margin-top: 0.5em;
+}
+
+.ve-ui-commandHelpDialog-list dt kbd > kbd {
+       background-color: #f7f7f7;
+       border: 1px solid #ccc;
+       border-radius: 3px;
+       box-shadow: 0 1px 0 rgba( 0, 0, 0, 0.2 ), 0 0 0 2px #fff inset;
+       color: #333;
+       display: inline-block;
+       line-height: 1.4;
+       padding: 0.1em 0.4em;
+       margin: -0.1em 0.3em 0;
+       text-shadow: 0 1px 0 #fff;
+       text-transform: uppercase;
+       text-align: center;
+}
+
+.ve-ui-commandHelpDialog-list dt kbd[ data-label ]::before {
+       content: attr( data-label );
+       font-weight: normal;
+       font-style: italic;
+       padding-right: 3px;
+}
+
+.ve-ui-commandHelpDialog-sequence kbd:not( [ data-key="space" ] ) {
+       box-shadow: none;
+       text-transform: none;
+}
+.ve-ui-commandHelpDialog-sequence kbd:not( [ data-key="space" ] ) + kbd:not( [ 
data-key="space" ] ) {
+       margin-left: -0.5em;
+       padding-left: 0;
+       border-left: 0;
+       border-top-left-radius: 0;
+       border-bottom-left-radius: 0;
 }
 
 .ve-ui-commandHelpDialog-list dd,
 .ve-ui-commandHelpDialog-list dt {
-       line-height: 110%;
+       line-height: 1.4;
        margin-top: 0.5em;
        margin-bottom: 0.5em;
 }
diff --git a/src/ui/ve.ui.Sequence.js b/src/ui/ve.ui.Sequence.js
index fe3f428..a4b5d93 100644
--- a/src/ui/ve.ui.Sequence.js
+++ b/src/ui/ve.ui.Sequence.js
@@ -116,3 +116,28 @@
 ve.ui.Sequence.prototype.getCommandName = function () {
        return this.commandName;
 };
+
+/**
+ * Get a representation of the sequence useful for display
+ *
+ * What this means depends a bit on how the sequence was defined:
+ * * It strips out undisplayable things like the paragraph-start marker.
+ * * Regexps are just returned as a toString of the regexp.
+ *
+ * @param {boolean} explode Whether to return the message split up into some
+ *        reasonable sequence of inputs required to trigger the sequence 
(regexps
+ *        in sequences will be considered a single "input" as a toString of
+ *        the regexp, because they're hard to display no matter what...)
+ * @return {string} Message for display
+ */
+ve.ui.Sequence.prototype.getMessage = function ( explode ) {
+       var data;
+       if ( typeof this.data === 'string' ) {
+               data = this.data.split( '' );
+       } else if ( this.data instanceof RegExp ) {
+               data = [ this.data.toString() ];
+       } else {
+               data = this.data.filter( function ( key ) { return 
!ve.isPlainObject( key ); } );
+       }
+       return explode ? data : data.join( '' );
+};
diff --git a/src/ui/ve.ui.Trigger.js b/src/ui/ve.ui.Trigger.js
index 9d2fcf0..d3a59b7 100644
--- a/src/ui/ve.ui.Trigger.js
+++ b/src/ui/ve.ui.Trigger.js
@@ -184,12 +184,12 @@
                        alt: '⎇',
                        escape: '⎋'
                };
-               return function ( keys ) {
+               return function ( keys, explode ) {
                        var i, len;
                        for ( i = 0, len = keys.length; i < len; i++ ) {
-                               keys[ i ] = names[ keys[ i ] ] || keys[ i ];
+                               keys[ i ] = ( names[ keys[ i ] ] || keys[ i ] 
).toUpperCase();
                        }
-                       return keys.join( '' ).toUpperCase();
+                       return explode ? keys : keys.join( '' );
                };
        } )()
 };
@@ -383,18 +383,21 @@
  * This is similar to #toString but the resulting string will be formatted in 
a way that makes it
  * appear more native for the platform.
  *
+ * @param {boolean} explode Whether to return the message split up into some
+ *        reasonable sequence of inputs required
  * @return {string} Message for trigger
  */
-ve.ui.Trigger.prototype.getMessage = function () {
+ve.ui.Trigger.prototype.getMessage = function ( explode ) {
        var keys,
                platformFilters = ve.ui.Trigger.static.platformFilters,
                platform = ve.getSystemPlatform();
 
        keys = this.toString().split( '+' );
        if ( Object.prototype.hasOwnProperty.call( platformFilters, platform ) 
) {
-               return platformFilters[ platform ]( keys );
+               return platformFilters[ platform ]( keys, explode );
        }
-       return keys.map( function ( key ) {
+       keys = keys.map( function ( key ) {
                return key[ 0 ].toUpperCase() + key.slice( 1 ).toLowerCase();
-       } ).join( '+' );
+       } );
+       return explode ? keys : keys.join( '+' );
 };

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I21fd47f5809dce36d10edc483121ecbdc12071f6
Gerrit-PatchSet: 12
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: DLynch <[email protected]>
Gerrit-Reviewer: DLynch <[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

Reply via email to