1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00

In Differential standalone views, disable some keyboard shortcuts which don't work

Summary:
Ref T13164. See PHI693. In Differential, you can {nav View Options > View Standalone} to get a standalone view of a single changeset. You can also arrive here via the big changeset list for revisions affecting a huge number of files.

We currently suggest that all the keyboard shortcuts work, but some do not. In particular, the "Next File" and "Previous File" keyboard shortcuts (and some similar shortcuts) do not work. In the main view, the next/previous files are on the same page. In the standalone view, we'd need to actually change the URI.

Ideally, we should do this (and, e.g., put prev/next links on the page). As a first step toward that, hide the nonfunctional shortcuts to stop users from being misled.

Test Plan:
  - Viewed a revision in normal and standalone views.
  - No changes in normal view, and all keys still work ("N", "P", etc).
  - In standalone view, "?" no longer shows nonfunctional key commands.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19571
This commit is contained in:
epriestley 2018-08-13 08:21:16 -07:00
parent a6951a0a5a
commit e5906f4e12
5 changed files with 50 additions and 26 deletions

View file

@ -12,7 +12,7 @@ return array(
'core.pkg.css' => 'f515619b',
'core.pkg.js' => '2058ec09',
'differential.pkg.css' => '06dc617c',
'differential.pkg.js' => 'ef19e026',
'differential.pkg.js' => '11a08e85',
'diffusion.pkg.css' => 'a2d17c7d',
'diffusion.pkg.js' => '6134c5a1',
'maniphest.pkg.css' => '4845691a',
@ -373,12 +373,12 @@ return array(
'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' => 'b49b59d6',
'rsrc/js/application/diff/DiffChangesetList.js' => 'f0ffe8c3',
'rsrc/js/application/diff/DiffChangesetList.js' => '7b95a80a',
'rsrc/js/application/diff/DiffInline.js' => 'e83d28f3',
'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07',
'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1',
'rsrc/js/application/differential/behavior-populate.js' => '419998ab',
'rsrc/js/application/differential/behavior-populate.js' => 'f0eb6708',
'rsrc/js/application/differential/behavior-user-select.js' => 'a8d8459d',
'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => '00676f00',
'rsrc/js/application/diffusion/behavior-audit-preview.js' => 'd835b03a',
@ -594,7 +594,7 @@ return array(
'javelin-behavior-diff-preview-link' => '051c7832',
'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
'javelin-behavior-differential-feedback-preview' => '51c5ad07',
'javelin-behavior-differential-populate' => '419998ab',
'javelin-behavior-differential-populate' => 'f0eb6708',
'javelin-behavior-differential-user-select' => 'a8d8459d',
'javelin-behavior-diffusion-commit-branches' => 'bdaf4d04',
'javelin-behavior-diffusion-commit-graph' => '75b83cbb',
@ -752,7 +752,7 @@ return array(
'phabricator-darkmessage' => 'c48cccdd',
'phabricator-dashboard-css' => 'fe5b1869',
'phabricator-diff-changeset' => 'b49b59d6',
'phabricator-diff-changeset-list' => 'f0ffe8c3',
'phabricator-diff-changeset-list' => '7b95a80a',
'phabricator-diff-inline' => 'e83d28f3',
'phabricator-drag-and-drop-file-upload' => '58dea2fa',
'phabricator-draggable-list' => 'bea6e7f4',
@ -1133,14 +1133,6 @@ return array(
'javelin-workflow',
'phabricator-draggable-list',
),
'419998ab' => array(
'javelin-behavior',
'javelin-dom',
'javelin-stratcom',
'phabricator-tooltip',
'phabricator-diff-changeset-list',
'phabricator-diff-changeset',
),
'4250a34e' => array(
'javelin-behavior',
'javelin-dom',
@ -1508,6 +1500,10 @@ return array(
'owners-path-editor',
'javelin-behavior',
),
'7b95a80a' => array(
'javelin-install',
'phuix-button-view',
),
'7cbe244b' => array(
'javelin-install',
'javelin-util',
@ -2117,9 +2113,13 @@ return array(
'javelin-workflow',
'javelin-json',
),
'f0ffe8c3' => array(
'javelin-install',
'phuix-button-view',
'f0eb6708' => array(
'javelin-behavior',
'javelin-dom',
'javelin-stratcom',
'phabricator-tooltip',
'phabricator-diff-changeset-list',
'phabricator-diff-changeset',
),
'f1ff5494' => array(
'phui-button-css',

View file

@ -276,6 +276,7 @@ final class DifferentialChangesetViewController extends DifferentialController {
->setDiff($diff)
->setTitle(pht('Standalone View'))
->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
->setIsStandalone(true)
->setParser($parser);
if ($revision_id) {

View file

@ -10,6 +10,7 @@ final class DifferentialChangesetListView extends AphrontView {
private $whitespace;
private $background;
private $header;
private $isStandalone;
private $standaloneURI;
private $leftRawFileURI;
@ -124,6 +125,15 @@ final class DifferentialChangesetListView extends AphrontView {
return $this;
}
public function setIsStandalone($is_standalone) {
$this->isStandalone = $is_standalone;
return $this;
}
public function getIsStandalone() {
return $this->isStandalone;
}
public function setBackground($background) {
$this->background = $background;
return $this;
@ -219,6 +229,7 @@ final class DifferentialChangesetListView extends AphrontView {
'changesetViewIDs' => $ids,
'inlineURI' => $this->inlineURI,
'inlineListURI' => $this->inlineListURI,
'isStandalone' => $this->getIsStandalone(),
'pht' => array(
'Open in Editor' => pht('Open in Editor'),
'Show All Context' => pht('Show All Context'),

View file

@ -89,7 +89,8 @@ JX.install('DiffChangesetList', {
properties: {
translations: null,
inlineURI: null,
inlineListURI: null
inlineListURI: null,
isStandalone: false
},
members: {
@ -149,6 +150,12 @@ JX.install('DiffChangesetList', {
this._initialized = true;
var pht = this.getTranslations();
// We may be viewing the normal "/D123" view (with all the changesets)
// or the standalone view (with just one changeset). In the standalone
// view, some options (like jumping to next or previous file) do not
// make sense and do not function.
var standalone = this.getIsStandalone();
var label;
label = pht('Jump to next change.');
@ -157,11 +164,13 @@ JX.install('DiffChangesetList', {
label = pht('Jump to previous change.');
this._installJumpKey('k', label, -1);
if (!standalone) {
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');
@ -176,11 +185,13 @@ JX.install('DiffChangesetList', {
'Jump to previous inline comment, including collapsed comments.');
this._installJumpKey('P', label, -1, 'comment', true);
if (!standalone) {
label = pht('Hide or show the current file.');
this._installKey('h', label, this._onkeytogglefile);
label = pht('Jump to the table of contents.');
this._installKey('t', label, this._ontoc);
}
label = pht('Reply to selected inline comment or change.');
this._installKey('r', label, JX.bind(this, this._onkeyreply, false));

View file

@ -61,7 +61,8 @@ JX.behavior('differential-populate', function(config, statics) {
var changeset_list = new JX.DiffChangesetList()
.setTranslations(JX.phtize(config.pht))
.setInlineURI(config.inlineURI)
.setInlineListURI(config.inlineListURI);
.setInlineListURI(config.inlineListURI)
.setIsStandalone(config.isStandalone);
// Install and activate the current page.
var page_id = JX.Quicksand.getCurrentPageID();