Page MenuHomePhabricator
Files F15414336

D17859.diff
No OneTemporary
Actions

D17859.diff
View Options

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -13,7 +13,7 @@
'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 @@
'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 @@
'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 @@
'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 @@
'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 @@
'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 @@
'javelin-vector',
'javelin-dom',
),
- '589a30aa' => array(
- 'javelin-install',
- ),
'58dea2fa' => array(
'javelin-install',
'javelin-util',
@@ -1629,12 +1624,6 @@
'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 @@
'javelin-stratcom',
'javelin-dom',
),
+ 'e2c315d9' => array(
+ 'javelin-install',
+ ),
'e2e0a072' => array(
'javelin-behavior',
'javelin-stratcom',
@@ -2440,7 +2432,6 @@
'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
--- a/resources/celerity/packages.php
+++ b/resources/celerity/packages.php
@@ -197,7 +197,6 @@
'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
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -465,7 +465,6 @@
}
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
--- a/src/applications/differential/view/DifferentialChangesetListView.php
+++ b/src/applications/differential/view/DifferentialChangesetListView.php
@@ -234,6 +234,16 @@
'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
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -720,8 +720,6 @@
$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
--- a/webroot/rsrc/js/application/diff/DiffChangeset.js
+++ b/webroot/rsrc/js/application/diff/DiffChangeset.js
@@ -318,6 +318,88 @@
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
--- a/webroot/rsrc/js/application/diff/DiffChangesetList.js
+++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js
@@ -55,15 +55,49 @@
},
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 @@
}
},
+ _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
--- 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();
-
-});

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 21, 12:10 AM (1 d, 15 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7673309
Default Alt Text
D17859.diff (21 KB)

Event Timeline

Phabricator · Built by Phacility