diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 70b982beae..a06069612a 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ return array( 'core.pkg.js' => '2ff7879f', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '58712637', - 'differential.pkg.js' => '6375358e', + 'differential.pkg.js' => 'e6129b80', 'diffusion.pkg.css' => 'b93d9b8c', 'diffusion.pkg.js' => '84c8f8fd', 'favicon.ico' => '30672e08', @@ -390,15 +390,14 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '408bf173', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', - 'rsrc/js/application/diff/DiffChangeset.js' => '4c9c47ad', - 'rsrc/js/application/diff/DiffChangesetList.js' => '589a30aa', + 'rsrc/js/application/diff/DiffChangeset.js' => '3d4b3c5e', + 'rsrc/js/application/diff/DiffChangesetList.js' => 'e2c315d9', 'rsrc/js/application/diff/DiffInline.js' => '98c12b2f', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d', 'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '89d11432', - 'rsrc/js/application/differential/behavior-keyboard-nav.js' => '92904457', 'rsrc/js/application/differential/behavior-populate.js' => '5e41c819', 'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb', 'rsrc/js/application/differential/behavior-user-select.js' => 'a8d8459d', @@ -624,7 +623,6 @@ return array( 'javelin-behavior-differential-diff-radios' => 'e1ff79b1', 'javelin-behavior-differential-edit-inline-comments' => '89d11432', 'javelin-behavior-differential-feedback-preview' => 'b064af76', - 'javelin-behavior-differential-keyboard-navigation' => '92904457', 'javelin-behavior-differential-populate' => '5e41c819', 'javelin-behavior-differential-toggle-files' => 'ca3f91eb', 'javelin-behavior-differential-user-select' => 'a8d8459d', @@ -783,8 +781,8 @@ return array( 'phabricator-darklog' => 'c8e1ffe3', 'phabricator-darkmessage' => 'c48cccdd', 'phabricator-dashboard-css' => 'fe5b1869', - 'phabricator-diff-changeset' => '4c9c47ad', - 'phabricator-diff-changeset-list' => '589a30aa', + 'phabricator-diff-changeset' => '3d4b3c5e', + 'phabricator-diff-changeset-list' => 'e2c315d9', 'phabricator-diff-inline' => '98c12b2f', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', @@ -1160,6 +1158,17 @@ return array( 'javelin-util', 'javelin-uri', ), + '3d4b3c5e' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + ), '3dbf94d5' => array( 'javelin-behavior', 'javelin-dom', @@ -1270,17 +1279,6 @@ return array( 'javelin-uri', 'phabricator-notification', ), - '4c9c47ad' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - ), '4d863052' => array( 'javelin-dom', 'javelin-util', @@ -1348,9 +1346,6 @@ return array( 'javelin-vector', 'javelin-dom', ), - '589a30aa' => array( - 'javelin-install', - ), '58dea2fa' => array( 'javelin-install', 'javelin-util', @@ -1629,12 +1624,6 @@ return array( 'javelin-dom', 'javelin-request', ), - 92904457 => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - 'phabricator-keyboard-shortcut', - ), '92b9ec77' => array( 'javelin-behavior', 'javelin-stratcom', @@ -2119,6 +2108,9 @@ return array( 'javelin-stratcom', 'javelin-dom', ), + 'e2c315d9' => array( + 'javelin-install', + ), 'e2e0a072' => array( 'javelin-behavior', 'javelin-stratcom', @@ -2440,7 +2432,6 @@ return array( 'javelin-behavior-differential-populate', 'javelin-behavior-differential-diff-radios', 'javelin-behavior-differential-comment-jump', - 'javelin-behavior-differential-keyboard-navigation', 'javelin-behavior-aphront-drag-and-drop-textarea', 'javelin-behavior-phabricator-object-selector', 'javelin-behavior-repository-crossreference', diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index 08752c1b53..364a695f18 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -197,7 +197,6 @@ return array( 'javelin-behavior-differential-populate', 'javelin-behavior-differential-diff-radios', 'javelin-behavior-differential-comment-jump', - 'javelin-behavior-differential-keyboard-navigation', 'javelin-behavior-aphront-drag-and-drop-textarea', 'javelin-behavior-phabricator-object-selector', 'javelin-behavior-repository-crossreference', diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 6cb39c1510..769ac37e1a 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -465,7 +465,6 @@ final class DifferentialRevisionViewController extends DifferentialController { } Javelin::initBehavior('differential-user-select'); - Javelin::initBehavior('differential-keyboard-navigation'); $view = id(new PHUITwoColumnView()) ->setHeader($header) diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index eb60a3dee1..715865d7b2 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -234,6 +234,16 @@ final class DifferentialChangesetListView extends AphrontView { 'Highlight As...' => pht('Highlight As...'), 'Loading...' => pht('Loading...'), + + 'Jump to next change.' => pht('Jump to next change.'), + 'Jump to previous change.' => pht('Jump to previous change.'), + 'Jump to next file.' => pht('Jump to next file.'), + 'Jump to previous file.' => pht('Jump to previous file.'), + 'Jump to next inline comment.' => pht('Jump to next inline comment.'), + 'Jump to previous inline comment.' => + pht('Jump to previous inline comment.'), + 'Jump to the table of contents.' => + pht('Jump to the table of contents.'), ), )); diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 10677b762f..554ad64e5e 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -720,8 +720,6 @@ final class DiffusionCommitController extends DiffusionController { $request = $this->getRequest(); $viewer = $request->getUser(); - Javelin::initBehavior('differential-keyboard-navigation'); - // TODO: This is pretty awkward, unify the CSS between Diffusion and // Differential better. require_celerity_resource('differential-core-view-css'); diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js index b1552cf8ce..b592eb0cec 100644 --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -318,6 +318,88 @@ JX.install('DiffChangeset', { return this._highlight; }, + getSelectableItems: function() { + var items = []; + + items.push({ + type: 'file', + changeset: this, + target: this, + nodes: { + begin: this._node, + end: null + } + }); + + var rows = JX.DOM.scry(this._node, 'tr'); + + var blocks = []; + var block; + var ii; + for (ii = 0; ii < rows.length; ii++) { + var type = this._getRowType(rows[ii]); + + if (!block || (block.type !== type)) { + block = { + type: type, + items: [] + }; + blocks.push(block); + } + + block.items.push(rows[ii]); + } + + for (ii = 0; ii < blocks.length; ii++) { + block = blocks[ii]; + + if (block.type == 'change') { + items.push({ + type: block.type, + changeset: this, + target: block.items[0], + nodes: { + begin: block.items[0], + end: block.items[block.items.length - 1] + } + }); + } + + if (block.type == 'comment') { + for (var jj = 0; jj < block.items.length; jj++) { + items.push({ + type: block.type, + changeset: this, + target: block.items[jj], + nodes: { + begin: block.items[jj], + end: block.items[jj] + } + }); + } + } + } + + return items; + }, + + _getRowType: function(row) { + // NOTE: Don't do "className.indexOf()" elsewhere. This is evil legacy + // magic. + + if (row.className.indexOf('inline') !== -1) { + return 'comment'; + } + + var cells = JX.DOM.scry(row, 'td'); + for (var ii = 0; ii < cells.length; ii++) { + if (cells[ii].className.indexOf('old') !== -1 || + cells[ii].className.indexOf('new') !== -1) { + return 'change'; + } + } + }, + _getNodeData: function() { return JX.Stratcom.getData(this._node); }, diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js index 0f07f4447b..39bc6aa289 100644 --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -55,15 +55,49 @@ JX.install('DiffChangesetList', { }, members: { + _initialized: false, _asleep: true, _changesets: null, + _cursorItem: null, + _lastKeyboardManager: null, + sleep: function() { this._asleep = true; }, wake: function() { this._asleep = false; + + if (this._initialized) { + return; + } + + this._initialized = true; + var pht = this.getTranslations(); + + var label; + + label = pht('Jump to next change.'); + this._installJumpKey('j', label, 1); + + label = pht('Jump to previous change.'); + this._installJumpKey('k', label, -1); + + label = pht('Jump to next file.'); + this._installJumpKey('J', label, 1, 'file'); + + label = pht('Jump to previous file.'); + this._installJumpKey('K', label, -1, 'file'); + + label = pht('Jump to next inline comment.'); + this._installJumpKey('n', label, 1, 'comment'); + + label = pht('Jump to previous inline comment.'); + this._installJumpKey('p', label, -1, 'comment'); + + label = pht('Jump to the table of contents.'); + this._installKey('t', label, this._ontoc); }, isAsleep: function() { @@ -132,6 +166,134 @@ JX.install('DiffChangesetList', { } }, + _installKey: function(key, label, handler) { + handler = JX.bind(this, this._ifawake, handler); + + return new JX.KeyboardShortcut(key, label) + .setHandler(handler) + .register(); + }, + + _installJumpKey: function(key, label, delta, filter) { + filter = filter || null; + var handler = JX.bind(this, this._onjumpkey, delta, filter); + return this._installKey(key, label, handler); + }, + + _ontoc: function(manager) { + var toc = JX.$('toc'); + manager.scrollTo(toc); + }, + + _onjumpkey: function(delta, filter, manager) { + var state = this._getSelectionState(); + + var cursor = state.cursor; + var items = state.items; + + // If there's currently no selection and the user tries to go back, + // don't do anything. + if ((cursor === null) && (delta < 0)) { + return; + } + + while (true) { + if (cursor === null) { + cursor = 0; + } else { + cursor = cursor + delta; + } + + // If we've gone backward past the first change, bail out. + if (cursor < 0) { + return; + } + + // If we've gone forward off the end of the list, bail out. + if (cursor >= items.length) { + return; + } + + // If we're selecting things of a particular type (like only files) + // and the next item isn't of that type, move past it. + if (filter !== null) { + if (items[cursor].type !== filter) { + continue; + } + } + + // Otherwise, we've found a valid item to select. + break; + } + + this._setSelectionState(items[cursor], manager); + }, + + _getSelectionState: function() { + var items = this._getSelectableItems(); + + var cursor = null; + if (this._cursorItem !== null) { + for (var ii = 0; ii < items.length; ii++) { + var item = items[ii]; + if (this._cursorItem.target === item.target) { + cursor = ii; + break; + } + } + } + + return { + cursor: cursor, + items: items + }; + }, + + _setSelectionState: function(item, manager) { + this._cursorItem = item; + + this._redrawSelection(manager, true); + + return this; + }, + + _redrawSelection: function(manager, scroll) { + manager = manager || this._lastKeyboardManager; + this._lastKeyboardManager = manager; + + if (this.isAsleep()) { + manager.focusOn(null); + return; + } + + var cursor = this._cursorItem; + if (!cursor) { + manager.focusOn(null); + return; + } + + manager.focusOn(cursor.nodes.begin, cursor.nodes.end); + + if (scroll) { + manager.scrollTo(cursor.nodes.begin); + } + + return this; + }, + + _getSelectableItems: function() { + var result = []; + + for (var ii = 0; ii < this._changesets.length; ii++) { + var items = this._changesets[ii].getSelectableItems(); + for (var jj = 0; jj < items.length; jj++) { + result.push(items[jj]); + } + } + + return result; + }, + _onmore: function(e) { e.kill(); diff --git a/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js b/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js deleted file mode 100644 index f48df4d3dc..0000000000 --- a/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js +++ /dev/null @@ -1,264 +0,0 @@ -/** - * @provides javelin-behavior-differential-keyboard-navigation - * @requires javelin-behavior - * javelin-dom - * javelin-stratcom - * phabricator-keyboard-shortcut - */ - -JX.behavior('differential-keyboard-navigation', function(config) { - - var cursor = -1; - var changesets; - - var selection_begin = null; - var selection_end = null; - - var refreshFocus = function() {}; - - function init() { - if (changesets) { - return; - } - changesets = JX.DOM.scry(document.body, 'div', 'differential-changeset'); - } - - function getBlocks(cursor) { - // TODO: This might not be terribly fast; we can't currently memoize it - // because it can change as ajax requests come in (e.g., content loads). - - var rows = JX.DOM.scry(changesets[cursor], 'tr'); - var blocks = [[changesets[cursor], changesets[cursor]]]; - var start = null; - var type; - var ii; - - // Don't show code blocks inside a collapsed file. - var diff = JX.DOM.scry(changesets[cursor], 'table', 'differential-diff'); - if (diff.length == 1 && JX.Stratcom.getData(diff[0]).hidden) { - return blocks; - } - - function push() { - if (start) { - blocks.push([start, rows[ii - 1]]); - } - start = null; - } - - for (ii = 0; ii < rows.length; ii++) { - type = getRowType(rows[ii]); - if (type == 'comment') { - // If we see these types of rows, make a block for each one. - push(); - } - if (!type) { - push(); - } else if (type && !start) { - start = rows[ii]; - } - } - push(); - - return blocks; - } - - function getRowType(row) { - // NOTE: Being somewhat over-general here to allow other types of objects - // to be easily focused in the future (inline comments, 'show more..'). - - if (row.className.indexOf('inline') !== -1) { - return 'comment'; - } - - if (row.className.indexOf('differential-changeset') !== -1) { - return 'file'; - } - - var cells = JX.DOM.scry(row, 'td'); - - for (var ii = 0; ii < cells.length; ii++) { - // NOTE: The semantic use of classnames here is for performance; don't - // emulate this elsewhere since it's super terrible. - if (cells[ii].className.indexOf('old') !== -1 || - cells[ii].className.indexOf('new') !== -1) { - return 'change'; - } - } - - return null; - } - - function jump(manager, delta, jump_to_type) { - init(); - - if (cursor < 0) { - if (delta < 0) { - // If the user goes "back" without a selection, just reject the action. - return; - } else { - cursor = 0; - } - } - - while (true) { - var blocks = getBlocks(cursor); - var focus; - if (delta < 0) { - focus = blocks.length; - } else { - focus = -1; - } - - for (var ii = 0; ii < blocks.length; ii++) { - if (blocks[ii][0] == selection_begin) { - focus = ii; - break; - } - } - - while (true) { - focus += delta; - - if (blocks[focus]) { - var row_type = getRowType(blocks[focus][0]); - if (jump_to_type && row_type != jump_to_type) { - continue; - } - - selection_begin = blocks[focus][0]; - selection_end = blocks[focus][1]; - - manager.scrollTo(selection_begin); - - refreshFocus = function() { - manager.focusOn(selection_begin, selection_end); - }; - - refreshFocus(); - - return; - } else { - var adjusted = (cursor + delta); - if (adjusted < 0 || adjusted >= changesets.length) { - // Stop cursor movement when the user reaches either end. - return; - } - cursor = adjusted; - - // Break the inner loop and go to the next file. - break; - } - } - } - - } - - // When inline comments are updated, wipe out our cache of blocks since - // comments may have been added or deleted. - JX.Stratcom.listen( - null, - 'differential-inline-comment-update', - function() { - changesets = null; - }); - // Same thing when a file is hidden or shown; don't want to highlight - // invisible code. - JX.Stratcom.listen( - 'differential-toggle-file-toggled', - null, - function() { - changesets = null; - init(); - refreshFocus(); - }); - - new JX.KeyboardShortcut('j', 'Jump to next change.') - .setHandler(function(manager) { - jump(manager, 1); - }) - .register(); - - new JX.KeyboardShortcut('k', 'Jump to previous change.') - .setHandler(function(manager) { - jump(manager, -1); - }) - .register(); - - new JX.KeyboardShortcut('J', 'Jump to next file.') - .setHandler(function(manager) { - jump(manager, 1, 'file'); - }) - .register(); - - new JX.KeyboardShortcut('K', 'Jump to previous file.') - .setHandler(function(manager) { - jump(manager, -1, 'file'); - }) - .register(); - - new JX.KeyboardShortcut('n', 'Jump to next inline comment.') - .setHandler(function(manager) { - jump(manager, 1, 'comment'); - }) - .register(); - - new JX.KeyboardShortcut('p', 'Jump to previous inline comment.') - .setHandler(function(manager) { - jump(manager, -1, 'comment'); - }) - .register(); - - - new JX.KeyboardShortcut('t', 'Jump to the table of contents.') - .setHandler(function(manager) { - var toc = JX.$('toc'); - manager.scrollTo(toc); - }) - .register(); - - new JX.KeyboardShortcut( - 'h', - 'Collapse or expand the file display (after jump).') - .setHandler(function() { - if (!changesets || !changesets[cursor]) { - return; - } - JX.Stratcom.invoke('differential-toggle-file', null, { - diff: JX.DOM.scry(changesets[cursor], 'table', 'differential-diff') - }); - }) - .register(); - - - function inline_op(node, op) { - // nothing selected - if (!node) { - return; - } - if (!JX.DOM.scry(node, 'a', 'differential-inline-' + op)) { - // No link for this operation, e.g. editing a comment you can't edit. - return; - } - - var data = { - node: JX.DOM.find(node, 'div', 'differential-inline-comment'), - op: op - }; - - JX.Stratcom.invoke('differential-inline-action', null, data); - } - - new JX.KeyboardShortcut('r', 'Reply to selected inline comment.') - .setHandler(function() { - inline_op(selection_begin, 'reply'); - }) - .register(); - - new JX.KeyboardShortcut('e', 'Edit selected inline comment.') - .setHandler(function() { - inline_op(selection_begin, 'edit'); - }) - .register(); - -});