From 48ec1f6d9888deb522542c954a53d6fadc7a77f9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 28 May 2011 11:36:00 -0700 Subject: [PATCH] Provide basic structure for keyboard shortcuts Summary: Implements a simple infrastructure for keyboard shortcuts, see T184, and a "help" shortcut. There's a lot of room for refinement here but I think it basically works. Each shortcut can also provide a "tooltip" handler which allows it to show help when the alt/option key is held down. Test Plan: Pressed "?" and got help. Pressed "?" in various contexts where it should not activate (modifier keys, text input focused) and didn't get help. Reviewers: aran, tuomaspelkonen, jungejason CC: moskov Differential Revision: 362 --- scripts/celerity_mapper.php | 4 + src/__celerity_resource_map__.php | 130 ++++++++++++------ src/__phutil_library_map__.php | 4 + ...AphrontDefaultApplicationConfiguration.php | 3 + .../base/PhabricatorHelpController.php | 34 +++++ .../help/controller/base/__init__.php | 15 ++ ...bricatorHelpKeyboardShortcutController.php | 61 ++++++++ .../controller/keyboardshortcut/__init__.php | 18 +++ src/view/dialog/AphrontDialogView.php | 6 +- .../standard/PhabricatorStandardPageView.php | 5 + .../application/base/standard-page-view.css | 20 ++- .../js/application/core/KeyboardShortcut.js | 35 +++++ .../core/KeyboardShortcutManager.js | 100 ++++++++++++++ .../core/behavior-keyboard-shortcuts.js | 21 +++ 14 files changed, 411 insertions(+), 45 deletions(-) create mode 100644 src/applications/help/controller/base/PhabricatorHelpController.php create mode 100644 src/applications/help/controller/base/__init__.php create mode 100644 src/applications/help/controller/keyboardshortcut/PhabricatorHelpKeyboardShortcutController.php create mode 100644 src/applications/help/controller/keyboardshortcut/__init__.php create mode 100644 webroot/rsrc/js/application/core/KeyboardShortcut.js create mode 100644 webroot/rsrc/js/application/core/KeyboardShortcutManager.js create mode 100644 webroot/rsrc/js/application/core/behavior-keyboard-shortcuts.js diff --git a/scripts/celerity_mapper.php b/scripts/celerity_mapper.php index b13058c015..87f8ed1f32 100755 --- a/scripts/celerity_mapper.php +++ b/scripts/celerity_mapper.php @@ -27,6 +27,10 @@ $package_spec = array( 'javelin-mask', 'javelin-workflow', 'javelin-behavior-workflow', + 'javelin-behavior-aphront-form-disable-on-submit', + 'phabricator-keyboard-shortcut-manager', + 'phabricator-keyboard-shortcut', + 'javelin-behavior-phabricator-keyboard-shortcuts', ), 'core.pkg.css' => array( 'phabricator-core-css', diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 0a072a6af5..65c37615a7 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -188,6 +188,15 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/css/application/differential/revision-comment-list.css', ), + 'differential-revision-detail-css' => + array( + 'uri' => '/res/ea9de420/rsrc/css/application/differential/revision-detail.css', + 'type' => 'css', + 'requires' => + array( + ), + 'disk' => '/rsrc/css/application/differential/revision-detail.css', + ), 0 => array( 'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js', @@ -198,15 +207,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/javelin/docs/Base.js', ), - 'differential-revision-detail-css' => - array( - 'uri' => '/res/ea9de420/rsrc/css/application/differential/revision-detail.css', - 'type' => 'css', - 'requires' => - array( - ), - 'disk' => '/rsrc/css/application/differential/revision-detail.css', - ), 'differential-revision-history-css' => array( 'uri' => '/res/0d7d515d/rsrc/css/application/differential/revision-history.css', @@ -546,6 +546,19 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/owners/owners-path-editor.js', ), + 'javelin-behavior-phabricator-keyboard-shortcuts' => + array( + 'uri' => '/res/5a23bcc8/rsrc/js/application/core/behavior-keyboard-shortcuts.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-workflow', + 2 => 'javelin-json', + 3 => 'phabricator-keyboard-shortcut', + ), + 'disk' => '/rsrc/js/application/core/behavior-keyboard-shortcuts.js', + ), 'javelin-behavior-phabricator-object-selector' => array( 'uri' => '/res/12d4d90d/rsrc/js/application/core/behavior-object-selector.js', @@ -923,6 +936,31 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/core/DragAndDropFileUpload.js', ), + 'phabricator-keyboard-shortcut' => + array( + 'uri' => '/res/beed38cd/rsrc/js/application/core/KeyboardShortcut.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-install', + 1 => 'javelin-util', + 2 => 'phabricator-keyboard-shortcut-manager', + ), + 'disk' => '/rsrc/js/application/core/KeyboardShortcut.js', + ), + 'phabricator-keyboard-shortcut-manager' => + array( + 'uri' => '/res/b32845bd/rsrc/js/application/core/KeyboardShortcutManager.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-install', + 1 => 'javelin-util', + 2 => 'javelin-stratcom', + 3 => 'javelin-dom', + ), + 'disk' => '/rsrc/js/application/core/KeyboardShortcutManager.js', + ), 'phabricator-object-selector-css' => array( 'uri' => '/res/ced4098a/rsrc/css/application/objectselector/object-selector.css', @@ -965,7 +1003,7 @@ celerity_register_resource_map(array( ), 'phabricator-standard-page-view' => array( - 'uri' => '/res/4db19fcb/rsrc/css/application/base/standard-page-view.css', + 'uri' => '/res/02ae6920/rsrc/css/application/base/standard-page-view.css', 'type' => 'css', 'requires' => array( @@ -1003,18 +1041,6 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/03ef179e/diffusion.pkg.css', 'type' => 'css', ), - '122a6b6d' => - array ( - 'name' => 'workflow.pkg.js', - 'symbols' => - array ( - 0 => 'javelin-mask', - 1 => 'javelin-workflow', - 2 => 'javelin-behavior-workflow', - ), - 'uri' => '/res/pkg/122a6b6d/workflow.pkg.js', - 'type' => 'js', - ), '33f413ef' => array ( 'name' => 'typeahead.pkg.js', @@ -1048,7 +1074,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/613cf273/differential.pkg.css', 'type' => 'css', ), - 'c9c3eee2' => + '64383b02' => array ( 'name' => 'core.pkg.css', 'symbols' => @@ -1069,7 +1095,7 @@ celerity_register_resource_map(array( 13 => 'phabricator-remarkup-css', 14 => 'syntax-highlighting-css', ), - 'uri' => '/res/pkg/c9c3eee2/core.pkg.css', + 'uri' => '/res/pkg/64383b02/core.pkg.css', 'type' => 'css', ), 'db95a6d0' => @@ -1091,6 +1117,22 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/db95a6d0/javelin.pkg.js', 'type' => 'js', ), + 'e26c5e06' => + array ( + 'name' => 'workflow.pkg.js', + 'symbols' => + array ( + 0 => 'javelin-mask', + 1 => 'javelin-workflow', + 2 => 'javelin-behavior-workflow', + 3 => 'javelin-behavior-aphront-form-disable-on-submit', + 4 => 'phabricator-keyboard-shortcut-manager', + 5 => 'phabricator-keyboard-shortcut', + 6 => 'javelin-behavior-phabricator-keyboard-shortcuts', + ), + 'uri' => '/res/pkg/e26c5e06/workflow.pkg.js', + 'type' => 'js', + ), 'ed383f69' => array ( 'name' => 'differential.pkg.js', @@ -1108,15 +1150,15 @@ celerity_register_resource_map(array( ), 'reverse' => array ( - 'aphront-crumbs-view-css' => 'c9c3eee2', - 'aphront-dialog-view-css' => 'c9c3eee2', - 'aphront-form-view-css' => 'c9c3eee2', - 'aphront-list-filter-view-css' => 'c9c3eee2', - 'aphront-panel-view-css' => 'c9c3eee2', - 'aphront-side-nav-view-css' => 'c9c3eee2', - 'aphront-table-view-css' => 'c9c3eee2', - 'aphront-tokenizer-control-css' => 'c9c3eee2', - 'aphront-typeahead-control-css' => 'c9c3eee2', + 'aphront-crumbs-view-css' => '64383b02', + 'aphront-dialog-view-css' => '64383b02', + 'aphront-form-view-css' => '64383b02', + 'aphront-list-filter-view-css' => '64383b02', + 'aphront-panel-view-css' => '64383b02', + 'aphront-side-nav-view-css' => '64383b02', + 'aphront-table-view-css' => '64383b02', + 'aphront-tokenizer-control-css' => '64383b02', + 'aphront-typeahead-control-css' => '64383b02', 'differential-changeset-view-css' => '613cf273', 'differential-core-view-css' => '613cf273', 'differential-revision-add-comment-css' => '613cf273', @@ -1128,17 +1170,19 @@ celerity_register_resource_map(array( 'diffusion-commit-view-css' => '03ef179e', 'javelin-behavior' => 'db95a6d0', 'javelin-behavior-aphront-basic-tokenizer' => '33f413ef', + 'javelin-behavior-aphront-form-disable-on-submit' => 'e26c5e06', 'javelin-behavior-differential-diff-radios' => 'ed383f69', 'javelin-behavior-differential-edit-inline-comments' => 'ed383f69', 'javelin-behavior-differential-feedback-preview' => 'ed383f69', 'javelin-behavior-differential-populate' => 'ed383f69', 'javelin-behavior-differential-show-more' => 'ed383f69', - 'javelin-behavior-workflow' => '122a6b6d', + 'javelin-behavior-phabricator-keyboard-shortcuts' => 'e26c5e06', + 'javelin-behavior-workflow' => 'e26c5e06', 'javelin-dom' => 'db95a6d0', 'javelin-event' => 'db95a6d0', 'javelin-install' => 'db95a6d0', 'javelin-json' => 'db95a6d0', - 'javelin-mask' => '122a6b6d', + 'javelin-mask' => 'e26c5e06', 'javelin-request' => 'db95a6d0', 'javelin-stratcom' => 'db95a6d0', 'javelin-tokenizer' => '33f413ef', @@ -1150,12 +1194,14 @@ celerity_register_resource_map(array( 'javelin-uri' => 'db95a6d0', 'javelin-util' => 'db95a6d0', 'javelin-vector' => 'db95a6d0', - 'javelin-workflow' => '122a6b6d', - 'phabricator-core-buttons-css' => 'c9c3eee2', - 'phabricator-core-css' => 'c9c3eee2', - 'phabricator-directory-css' => 'c9c3eee2', - 'phabricator-remarkup-css' => 'c9c3eee2', - 'phabricator-standard-page-view' => 'c9c3eee2', - 'syntax-highlighting-css' => 'c9c3eee2', + 'javelin-workflow' => 'e26c5e06', + 'phabricator-core-buttons-css' => '64383b02', + 'phabricator-core-css' => '64383b02', + 'phabricator-directory-css' => '64383b02', + 'phabricator-keyboard-shortcut' => 'e26c5e06', + 'phabricator-keyboard-shortcut-manager' => 'e26c5e06', + 'phabricator-remarkup-css' => '64383b02', + 'phabricator-standard-page-view' => '64383b02', + 'syntax-highlighting-css' => '64383b02', ), )); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0aca76d524..0b2c60a1e1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -330,6 +330,8 @@ phutil_register_library_map(array( 'PhabricatorFileViewController' => 'applications/files/controller/view', 'PhabricatorGoodForNothingWorker' => 'infrastructure/daemon/workers/worker/goodfornothing', 'PhabricatorHandleObjectSelectorDataView' => 'applications/phid/handle/view/selector', + 'PhabricatorHelpController' => 'applications/help/controller/base', + 'PhabricatorHelpKeyboardShortcutController' => 'applications/help/controller/keyboardshortcut', 'PhabricatorIRCBot' => 'infrastructure/daemon/irc/bot', 'PhabricatorIRCHandler' => 'infrastructure/daemon/irc/handler/base', 'PhabricatorIRCMessage' => 'infrastructure/daemon/irc/message', @@ -777,6 +779,8 @@ phutil_register_library_map(array( 'PhabricatorFileUploadController' => 'PhabricatorFileController', 'PhabricatorFileViewController' => 'PhabricatorFileController', 'PhabricatorGoodForNothingWorker' => 'PhabricatorWorker', + 'PhabricatorHelpController' => 'PhabricatorController', + 'PhabricatorHelpKeyboardShortcutController' => 'PhabricatorHelpController', 'PhabricatorIRCBot' => 'PhabricatorDaemon', 'PhabricatorIRCObjectNameHandler' => 'PhabricatorIRCHandler', 'PhabricatorIRCProtocolHandler' => 'PhabricatorIRCHandler', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index d198df326e..519aed0c70 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -305,6 +305,9 @@ class AphrontDefaultApplicationConfiguration '/status/$' => 'PhabricatorStatusController', + '/help/' => array( + 'keyboardshortcut/$' => 'PhabricatorHelpKeyboardShortcutController', + ), ); } diff --git a/src/applications/help/controller/base/PhabricatorHelpController.php b/src/applications/help/controller/base/PhabricatorHelpController.php new file mode 100644 index 0000000000..46ef2482c3 --- /dev/null +++ b/src/applications/help/controller/base/PhabricatorHelpController.php @@ -0,0 +1,34 @@ +buildStandardPageView(); + + $page->setApplicationName('Help'); + $page->setBaseURI('/help/'); + $page->setTitle(idx($data, 'title')); + $page->setGlyph('?'); + $page->appendChild($view); + + $response = new AphrontWebpageResponse(); + return $response->setContent($page->render()); + } + +} diff --git a/src/applications/help/controller/base/__init__.php b/src/applications/help/controller/base/__init__.php new file mode 100644 index 0000000000..01ba7fc5ed --- /dev/null +++ b/src/applications/help/controller/base/__init__.php @@ -0,0 +1,15 @@ +getRequest(); + $user = $request->getUser(); + + $keys = $request->getStr('keys'); + $keys = json_decode($keys, true); + if (!is_array($keys)) { + return new Aphront400Response(); + } + + $rows = array(); + foreach ($keys as $shortcut) { + $keystrokes = array(); + foreach ($shortcut['keys'] as $stroke) { + $keystrokes[] = ''.phutil_escape_html($stroke).''; + } + $keystrokes = implode(' or ', $keystrokes); + $rows[] = + ''. + ''.$keystrokes.''. + ''.phutil_escape_html($shortcut['description']).''. + ''; + } + + $table = + ''. + implode('', $rows). + '
'; + + $dialog = id(new AphrontDialogView()) + ->setUser($user) + ->setTitle('Keyboard Shortcuts') + ->appendChild($table) + ->addCancelButton('#', 'Close'); + + return id(new AphrontDialogResponse()) + ->setDialog($dialog); + } + +} diff --git a/src/applications/help/controller/keyboardshortcut/__init__.php b/src/applications/help/controller/keyboardshortcut/__init__.php new file mode 100644 index 0000000000..67661a0bc4 --- /dev/null +++ b/src/applications/help/controller/keyboardshortcut/__init__.php @@ -0,0 +1,18 @@ +cancelURI = $uri; + $this->cancelText = $text; return $this; } @@ -101,7 +103,7 @@ class AphrontDialogView extends AphrontView { 'name' => '__cancel__', 'sigil' => 'jx-workflow-button', ), - 'Cancel'); + phutil_escape_html($this->cancelText)); } $buttons = implode('', $buttons); diff --git a/src/view/page/standard/PhabricatorStandardPageView.php b/src/view/page/standard/PhabricatorStandardPageView.php index a5ae06e64b..d4fc62e6fc 100644 --- a/src/view/page/standard/PhabricatorStandardPageView.php +++ b/src/view/page/standard/PhabricatorStandardPageView.php @@ -100,6 +100,11 @@ class PhabricatorStandardPageView extends AphrontPageView { require_celerity_resource('phabricator-standard-page-view'); Javelin::initBehavior('workflow', array()); + Javelin::initBehavior( + 'phabricator-keyboard-shortcuts', + array( + 'helpURI' => '/help/keyboardshortcut/', + )); if ($console) { require_celerity_resource('aphront-dark-console-css'); diff --git a/webroot/rsrc/css/application/base/standard-page-view.css b/webroot/rsrc/css/application/base/standard-page-view.css index f8cd0fc6be..c6704f7157 100644 --- a/webroot/rsrc/css/application/base/standard-page-view.css +++ b/webroot/rsrc/css/application/base/standard-page-view.css @@ -91,7 +91,6 @@ td.phabricator-login-details { color: #f3f3f3; } - .phabricator-admin-page-view .phabricator-standard-header { background: #aa0000; } @@ -103,3 +102,22 @@ td.phabricator-login-details { .phabricator-admin-page-view .phabricator-logo a { background-image: url('/rsrc/image/phabricator_logo_admin.png'); } + +.keyboard-shortcut-help td, +.keyboard-shortcut-help th { + padding: 8px; + vertical-align: middle; +} + +.keyboard-shortcut-help th { + white-space: nowrap; + color: #666666; +} + +.keyboard-shortcut-help kbd { + background: #222222; + padding: 6px; + color: #ffffff; + font-weight: bold; + border: 1px solid #555555; +} diff --git a/webroot/rsrc/js/application/core/KeyboardShortcut.js b/webroot/rsrc/js/application/core/KeyboardShortcut.js new file mode 100644 index 0000000000..173a7b8ee2 --- /dev/null +++ b/webroot/rsrc/js/application/core/KeyboardShortcut.js @@ -0,0 +1,35 @@ +/** + * @provides phabricator-keyboard-shortcut + * @requires javelin-install + * javelin-util + * phabricator-keyboard-shortcut-manager + * @javelin + */ + +/** + * Register a keyboard shortcut, which does something when the user presses a + * key with no other inputs focused. + */ +JX.install('KeyboardShortcut', { + + construct : function(keys, description) { + keys = JX.$AX(keys); + this.setKeys(keys); + this.setDescription(description); + }, + + properties : { + keys : null, + description : null, + handler : null, + tooltipHandler : null + }, + + members : { + register : function() { + JX.KeyboardShortcutManager.getInstance().addKeyboardShortcut(this); + return this; + } + } + +}); diff --git a/webroot/rsrc/js/application/core/KeyboardShortcutManager.js b/webroot/rsrc/js/application/core/KeyboardShortcutManager.js new file mode 100644 index 0000000000..4a5be89c31 --- /dev/null +++ b/webroot/rsrc/js/application/core/KeyboardShortcutManager.js @@ -0,0 +1,100 @@ +/** + * @provides phabricator-keyboard-shortcut-manager + * @requires javelin-install + * javelin-util + * javelin-stratcom + * javelin-dom + * @javelin + */ + +JX.install('KeyboardShortcutManager', { + + construct : function() { + this._shortcuts = []; + + JX.Stratcom.listen('keypress', null, JX.bind(this, this._onkeypress)); + JX.Stratcom.listen('keydown', null, JX.bind(this, this._onkeydown)); + JX.Stratcom.listen('keyup', null, JX.bind(this, this._onkeyup)); + }, + + statics : { + _instance : null, + getInstance : function() { + if (!JX.KeyboardShortcutManager._instance) { + JX.KeyboardShortcutManager._instance = new JX.KeyboardShortcutManager(); + } + return JX.KeyboardShortcutManager._instance; + } + }, + + members : { + _shortcuts : null, + + /** + * Instead of calling this directly, you should call + * KeyboardShortcut.register(). + */ + addKeyboardShortcut : function(s) { + this._shortcuts.push(s); + }, + getShortcutDescriptions : function() { + var desc = []; + for (var ii = 0; ii < this._shortcuts.length; ii++) { + desc.push({ + keys : this._shortcuts[ii].getKeys(), + description : this._shortcuts[ii].getDescription() + }); + } + return desc; + }, + _onkeypress : function(e) { + var raw = e.getRawEvent(); + + if (raw.altKey || raw.ctrlKey || raw.metaKey) { + // Never activate keyboard shortcuts if modifier keys are also + // depressed. + return; + } + + var target = e.getTarget(); + var ignore = ['input', 'select', 'textarea', 'object', 'embed']; + if (JX.DOM.isType(target, ignore)) { + // Never activate keyboard shortcuts if the user has some other control + // focused. + return; + } + // TODO: This likely needs to be refined to deal with arrow keys, etc. + var key = String.fromCharCode(raw.charCode); + + var shortcuts = this._shortcuts; + for (var ii = 0; ii < shortcuts.length; ii++) { + var keys = shortcuts[ii].getKeys(); + for (var jj = 0; jj < keys.length; jj++) { + if (keys[jj] == key) { + shortcuts[ii].getHandler()(this); + return; + } + } + } + }, + _onkeydown : function(e) { + this._handleTooltipKeyEvent(e, true); + }, + _onkeyup : function(e) { + this._handleTooltipKeyEvent(e, false); + }, + _handleTooltipKeyEvent : function(e, is_keydown) { + if (e.getRawEvent().keyCode != 18) { + // If this isn't the alt/option key, don't do anything. + return; + } + // Fire all the shortcut handlers. + var shortcuts = this._shortcuts; + for (var ii = 0; ii < shortcuts.length; ii++) { + var handler = shortcuts[ii].getTooltipHandler(); + handler && handler(this, is_keydown); + } + } + + } +}); diff --git a/webroot/rsrc/js/application/core/behavior-keyboard-shortcuts.js b/webroot/rsrc/js/application/core/behavior-keyboard-shortcuts.js new file mode 100644 index 0000000000..d9a8b3c545 --- /dev/null +++ b/webroot/rsrc/js/application/core/behavior-keyboard-shortcuts.js @@ -0,0 +1,21 @@ +/** + * @provides javelin-behavior-phabricator-keyboard-shortcuts + * @requires javelin-behavior + * javelin-workflow + * javelin-json + * phabricator-keyboard-shortcut + */ + +/** + * Define global keyboard shortcuts. + */ +JX.behavior('phabricator-keyboard-shortcuts', function(config) { + var desc = 'Show keyboard shortcut help for the current page.'; + new JX.KeyboardShortcut('?', desc) + .setHandler(function(manager) { + var desc = manager.getShortcutDescriptions(); + new JX.Workflow(config.helpURI, {keys : JX.JSON.serialize(desc)}) + .start(); + }) + .register(); +});