In the UI, rename "Hide Inline" to "Collapse Inline"

Summary:
Ref T12733. This paves the way for a separate "hide" operation which completely hides things.

(I didn't extend this to the server side because that would require schema changes and the new "hide" state is client-only.)

Test Plan: Collapsed and expanded inlines, viewed tooltips.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18126
This commit is contained in:
epriestley
2017-06-15 04:17:01 -07:00
parent 8008ade9af
commit 887bd2d66e
7 changed files with 66 additions and 73 deletions

View File

@@ -13,7 +13,7 @@ return array(
'core.pkg.js' => '1475bd91', 'core.pkg.js' => '1475bd91',
'darkconsole.pkg.js' => '1f9a31bc', 'darkconsole.pkg.js' => '1f9a31bc',
'differential.pkg.css' => '4e99863c', 'differential.pkg.css' => '4e99863c',
'differential.pkg.js' => 'b7504037', 'differential.pkg.js' => 'ee50e5ae',
'diffusion.pkg.css' => 'b93d9b8c', 'diffusion.pkg.css' => 'b93d9b8c',
'diffusion.pkg.js' => '6134c5a1', 'diffusion.pkg.js' => '6134c5a1',
'favicon.ico' => '30672e08', 'favicon.ico' => '30672e08',
@@ -395,9 +395,9 @@ return array(
'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '408bf173', '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-query-panel-select.js' => '453c5375',
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63',
'rsrc/js/application/diff/DiffChangeset.js' => 'd498bddb', 'rsrc/js/application/diff/DiffChangeset.js' => '94f81a34',
'rsrc/js/application/diff/DiffChangesetList.js' => '29bbc02c', 'rsrc/js/application/diff/DiffChangesetList.js' => 'fc6e482d',
'rsrc/js/application/diff/DiffInline.js' => '20553f71', 'rsrc/js/application/diff/DiffInline.js' => 'a386f83c',
'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07', 'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07',
'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1',
@@ -774,9 +774,9 @@ return array(
'phabricator-darklog' => 'c8e1ffe3', 'phabricator-darklog' => 'c8e1ffe3',
'phabricator-darkmessage' => 'c48cccdd', 'phabricator-darkmessage' => 'c48cccdd',
'phabricator-dashboard-css' => 'fe5b1869', 'phabricator-dashboard-css' => 'fe5b1869',
'phabricator-diff-changeset' => 'd498bddb', 'phabricator-diff-changeset' => '94f81a34',
'phabricator-diff-changeset-list' => '29bbc02c', 'phabricator-diff-changeset-list' => 'fc6e482d',
'phabricator-diff-inline' => '20553f71', 'phabricator-diff-inline' => 'a386f83c',
'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-drag-and-drop-file-upload' => '58dea2fa',
'phabricator-draggable-list' => 'bea6e7f4', 'phabricator-draggable-list' => 'bea6e7f4',
'phabricator-fatal-config-template-css' => '8f18fa41', 'phabricator-fatal-config-template-css' => '8f18fa41',
@@ -1039,9 +1039,6 @@ return array(
'javelin-install', 'javelin-install',
'javelin-dom', 'javelin-dom',
), ),
'20553f71' => array(
'javelin-dom',
),
'2290aeef' => array( '2290aeef' => array(
'javelin-install', 'javelin-install',
'javelin-dom', 'javelin-dom',
@@ -1067,10 +1064,6 @@ return array(
'javelin-install', 'javelin-install',
'javelin-util', 'javelin-util',
), ),
'29bbc02c' => array(
'javelin-install',
'phuix-button-view',
),
'2ae077e1' => array( '2ae077e1' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-dom', 'javelin-dom',
@@ -1619,6 +1612,17 @@ return array(
'javelin-resource', 'javelin-resource',
'javelin-routable', 'javelin-routable',
), ),
'94f81a34' => array(
'javelin-dom',
'javelin-util',
'javelin-stratcom',
'javelin-install',
'javelin-workflow',
'javelin-router',
'javelin-behavior-device',
'javelin-vector',
'phabricator-diff-inline',
),
'960f6a39' => array( '960f6a39' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-dom', 'javelin-dom',
@@ -1663,6 +1667,9 @@ return array(
'javelin-install', 'javelin-install',
'javelin-dom', 'javelin-dom',
), ),
'a386f83c' => array(
'javelin-dom',
),
'a3a63478' => array( 'a3a63478' => array(
'phui-workcard-view-css', 'phui-workcard-view-css',
), ),
@@ -1986,17 +1993,6 @@ return array(
'javelin-uri', 'javelin-uri',
'javelin-util', 'javelin-util',
), ),
'd498bddb' => array(
'javelin-dom',
'javelin-util',
'javelin-stratcom',
'javelin-install',
'javelin-workflow',
'javelin-router',
'javelin-behavior-device',
'javelin-vector',
'phabricator-diff-inline',
),
'd4eecc63' => array( 'd4eecc63' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-dom', 'javelin-dom',
@@ -2158,6 +2154,10 @@ return array(
'javelin-behavior-device', 'javelin-behavior-device',
'phabricator-keyboard-shortcut', 'phabricator-keyboard-shortcut',
), ),
'fc6e482d' => array(
'javelin-install',
'phuix-button-view',
),
'fc91ab6c' => array( 'fc91ab6c' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-dom', 'javelin-dom',

View File

@@ -257,15 +257,15 @@ final class DifferentialChangesetListView extends AphrontView {
'You must select a comment to mark done.' => 'You must select a comment to mark done.' =>
pht('You must select a comment to mark done.'), pht('You must select a comment to mark done.'),
'Hide or show inline comment.' => 'Collapse or expand inline comment.' =>
pht('Hide or show inline comment.'), pht('Collapse or expand inline comment.'),
'You must select a comment to hide.' => 'You must select a comment to hide.' =>
pht('You must select a comment to hide.'), pht('You must select a comment to hide.'),
'Jump to next inline comment, including hidden comments.' => 'Jump to next inline comment, including collapsed comments.' =>
pht('Jump to next inline comment, including hidden comments.'), pht('Jump to next inline comment, including collapsed comments.'),
'Jump to previous inline comment, including hidden comments.' => 'Jump to previous inline comment, including collapsed comments.' =>
pht('Jump to previous inline comment, including hidden comments.'), pht('Jump to previous inline comment, including collapsed comments.'),
'This file content has been collapsed.' => 'This file content has been collapsed.' =>
pht('This file content has been collapsed.'), pht('This file content has been collapsed.'),

View File

@@ -265,7 +265,7 @@ final class PHUIDiffInlineCommentDetailView
if (!$this->preview && $this->canHide()) { if (!$this->preview && $this->canHide()) {
$action_buttons[] = id(new PHUIButtonView()) $action_buttons[] = id(new PHUIButtonView())
->setTag('a') ->setTag('a')
->setTooltip(pht('Hide Comment')) ->setTooltip(pht('Collapse'))
->setIcon('fa-times') ->setIcon('fa-times')
->addSigil('hide-inline') ->addSigil('hide-inline')
->setMustCapture(true); ->setMustCapture(true);

View File

@@ -8,7 +8,7 @@ final class PHUIDiffRevealIconView extends AphrontView {
->addSigil('has-tooltip') ->addSigil('has-tooltip')
->setMetadata( ->setMetadata(
array( array(
'tip' => pht('Show Hidden Comment'), 'tip' => pht('Expand'),
'align' => 'E', 'align' => 'E',
'size' => 275, 'size' => 275,
)); ));

View File

@@ -446,7 +446,7 @@ JX.install('DiffChangeset', {
type: block.type, type: block.type,
changeset: this, changeset: this,
target: inline, target: inline,
hidden: inline.isHidden(), collapsed: inline.isCollapsed(),
deleted: !inline.getID() && !inline.isEditing(), deleted: !inline.getID() && !inline.isEditing(),
nodes: { nodes: {
begin: block.items[jj], begin: block.items[jj],

View File

@@ -19,11 +19,11 @@ JX.install('DiffChangesetList', {
var onmenu = JX.bind(this, this._ifawake, this._onmenu); var onmenu = JX.bind(this, this._ifawake, this._onmenu);
JX.Stratcom.listen('click', 'differential-view-options', onmenu); JX.Stratcom.listen('click', 'differential-view-options', onmenu);
var onhide = JX.bind(this, this._ifawake, this._onhide); var oncollapse = JX.bind(this, this._ifawake, this._oncollapse, true);
JX.Stratcom.listen('click', 'hide-inline', onhide); JX.Stratcom.listen('click', 'hide-inline', oncollapse);
var onreveal = JX.bind(this, this._ifawake, this._onreveal); var onexpand = JX.bind(this, this._ifawake, this._oncollapse, false);
JX.Stratcom.listen('click', 'reveal-inline', onreveal); JX.Stratcom.listen('click', 'reveal-inline', onexpand);
var onedit = JX.bind(this, this._ifawake, this._onaction, 'edit'); var onedit = JX.bind(this, this._ifawake, this._onaction, 'edit');
JX.Stratcom.listen( JX.Stratcom.listen(
@@ -158,11 +158,11 @@ JX.install('DiffChangesetList', {
label = pht('Jump to previous inline comment.'); label = pht('Jump to previous inline comment.');
this._installJumpKey('p', label, -1, 'comment'); this._installJumpKey('p', label, -1, 'comment');
label = pht('Jump to next inline comment, including hidden comments.'); label = pht('Jump to next inline comment, including collapsed comments.');
this._installJumpKey('N', label, 1, 'comment', true); this._installJumpKey('N', label, 1, 'comment', true);
label = pht( label = pht(
'Jump to previous inline comment, including hidden comments.'); 'Jump to previous inline comment, including collapsed comments.');
this._installJumpKey('P', label, -1, 'comment', true); this._installJumpKey('P', label, -1, 'comment', true);
label = pht('Hide or show the current file.'); label = pht('Hide or show the current file.');
@@ -183,8 +183,8 @@ JX.install('DiffChangesetList', {
label = pht('Mark or unmark selected inline comment as done.'); label = pht('Mark or unmark selected inline comment as done.');
this._installKey('w', label, this._onkeydone); this._installKey('w', label, this._onkeydone);
label = pht('Hide or show inline comment.'); label = pht('Collapse or expand inline comment.');
this._installKey('q', label, this._onkeyhide); this._installKey('q', label, this._onkeycollapse);
}, },
isAsleep: function() { isAsleep: function() {
@@ -261,12 +261,12 @@ JX.install('DiffChangesetList', {
.register(); .register();
}, },
_installJumpKey: function(key, label, delta, filter, show_hidden) { _installJumpKey: function(key, label, delta, filter, show_collapsed) {
filter = filter || null; filter = filter || null;
var options = { var options = {
filter: filter, filter: filter,
hidden: show_hidden collapsed: show_collapsed
}; };
var handler = JX.bind(this, this._onjumpkey, delta, options); var handler = JX.bind(this, this._onjumpkey, delta, options);
@@ -424,16 +424,16 @@ JX.install('DiffChangesetList', {
this._warnUser(pht('You must select a file to hide or show.')); this._warnUser(pht('You must select a file to hide or show.'));
}, },
_onkeyhide: function() { _onkeycollapse: function() {
var cursor = this._cursorItem; var cursor = this._cursorItem;
if (cursor) { if (cursor) {
if (cursor.type == 'comment') { if (cursor.type == 'comment') {
var inline = cursor.target; var inline = cursor.target;
if (inline.canHide()) { if (inline.canCollapse()) {
this.setFocus(null); this.setFocus(null);
inline.setHidden(!inline.isHidden()); inline.setCollapsed(!inline.isCollapsed());
return; return;
} }
} }
@@ -455,7 +455,7 @@ JX.install('DiffChangesetList', {
var state = this._getSelectionState(); var state = this._getSelectionState();
var filter = options.filter || null; var filter = options.filter || null;
var hidden = options.hidden || false; var collapsed = options.collapsed || false;
var wrap = options.wrap || false; var wrap = options.wrap || false;
var attribute = options.attribute || null; var attribute = options.attribute || null;
@@ -507,10 +507,10 @@ JX.install('DiffChangesetList', {
} }
} }
// If the item is hidden, don't select it when iterating with jump // If the item is collapsed, don't select it when iterating with jump
// keys. It can still potentially be selected in other ways. // keys. It can still potentially be selected in other ways.
if (!hidden) { if (!collapsed) {
if (items[cursor].hidden) { if (items[cursor].collapsed) {
continue; continue;
} }
} }
@@ -851,20 +851,12 @@ JX.install('DiffChangesetList', {
menu.open(); menu.open();
}, },
_onhide: function(e) { _oncollapse: function(is_collapse, e) {
this._onhidereveal(e, true);
},
_onreveal: function(e) {
this._onhidereveal(e, false);
},
_onhidereveal: function(e, is_hide) {
e.kill(); e.kill();
var inline = this._getInlineForEvent(e); var inline = this._getInlineForEvent(e);
inline.setHidden(is_hide); inline.setCollapsed(is_collapse);
}, },
_onresize: function() { _onresize: function() {

View File

@@ -14,7 +14,6 @@ JX.install('DiffInline', {
_phid: null, _phid: null,
_changesetID: null, _changesetID: null,
_row: null, _row: null,
_hidden: false,
_number: null, _number: null,
_length: null, _length: null,
_displaySide: null, _displaySide: null,
@@ -30,6 +29,7 @@ JX.install('DiffInline', {
_changeset: null, _changeset: null,
_isCollapsed: false,
_isDraft: null, _isDraft: null,
_isFixed: null, _isFixed: null,
_isEditing: false, _isEditing: false,
@@ -41,7 +41,7 @@ JX.install('DiffInline', {
var row_data = JX.Stratcom.getData(row); var row_data = JX.Stratcom.getData(row);
row_data.inline = this; row_data.inline = this;
this._hidden = row_data.hidden || false; this._isCollapsed = row_data.hidden || false;
// TODO: Get smarter about this once we do more editing, this is pretty // TODO: Get smarter about this once we do more editing, this is pretty
// hacky. // hacky.
@@ -225,7 +225,7 @@ JX.install('DiffInline', {
return true; return true;
}, },
canHide: function() { canCollapse: function() {
if (!JX.DOM.scry(this._row, 'a', 'hide-inline').length) { if (!JX.DOM.scry(this._row, 'a', 'hide-inline').length) {
return false; return false;
} }
@@ -254,20 +254,18 @@ JX.install('DiffInline', {
this._id = null; this._id = null;
this._phid = null; this._phid = null;
this._hidden = false; this._isCollapsed = false;
this._originalText = null; this._originalText = null;
return row; return row;
}, },
setHidden: function(hidden) { setCollapsed: function(collapsed) {
this._hidden = hidden; this._isCollapsed = collapsed;
JX.DOM.alterClass(this._row, 'inline-hidden', this._hidden);
var op; var op;
if (hidden) { if (collapsed) {
op = 'hide'; op = 'hide';
} else { } else {
op = 'show'; op = 'show';
@@ -280,11 +278,12 @@ JX.install('DiffInline', {
.setHandler(JX.bag) .setHandler(JX.bag)
.start(); .start();
this._redraw();
this._didUpdate(true); this._didUpdate(true);
}, },
isHidden: function() { isCollapsed: function() {
return this._hidden; return this._isCollapsed;
}, },
toggleDone: function() { toggleDone: function() {
@@ -703,11 +702,13 @@ JX.install('DiffInline', {
_redraw: function() { _redraw: function() {
var is_invisible = (this._isInvisible || this._isDeleted); var is_invisible = (this._isInvisible || this._isDeleted);
var is_loading = (this._isLoading); var is_loading = this._isLoading;
var is_collapsed = this._isCollapsed;
var row = this._row; var row = this._row;
JX.DOM.alterClass(row, 'differential-inline-hidden', is_invisible); JX.DOM.alterClass(row, 'differential-inline-hidden', is_invisible);
JX.DOM.alterClass(row, 'differential-inline-loading', is_loading); JX.DOM.alterClass(row, 'differential-inline-loading', is_loading);
JX.DOM.alterClass(row, 'inline-hidden', is_collapsed);
}, },
_removeRow: function(row) { _removeRow: function(row) {