mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-10 23:01:04 +01:00
Fix minor inline comment header button behaviors
Summary: Fixes T12806. Ref T12733. - Don't count synthetic (lint) comments as anything. - When you begin writing an inline then cancel it, don't count it as anything. - When we would show "0 / X", just show "X". Test Plan: - Viewed a diff with synthetic comments, no button. - Wrote, then cancelled an inline. No "X comments". - Clicked / unlicked "Done", saw "X" -> "1 / X". Reviewers: chad Reviewed By: chad Maniphest Tasks: T12806, T12733 Differential Revision: https://secure.phabricator.com/D18103
This commit is contained in:
parent
9ef726a22b
commit
8692d673c8
3 changed files with 42 additions and 12 deletions
|
@ -107,6 +107,11 @@ final class PHUIDiffInlineCommentDetailView
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$is_synthetic = false;
|
||||||
|
if ($inline->getSyntheticAuthor()) {
|
||||||
|
$is_synthetic = true;
|
||||||
|
}
|
||||||
|
|
||||||
$metadata = array(
|
$metadata = array(
|
||||||
'id' => $inline->getID(),
|
'id' => $inline->getID(),
|
||||||
'phid' => $inline->getPHID(),
|
'phid' => $inline->getPHID(),
|
||||||
|
@ -120,6 +125,7 @@ final class PHUIDiffInlineCommentDetailView
|
||||||
'isDraft' => $inline->isDraft(),
|
'isDraft' => $inline->isDraft(),
|
||||||
'isFixed' => $is_fixed,
|
'isFixed' => $is_fixed,
|
||||||
'isGhost' => $inline->getIsGhost(),
|
'isGhost' => $inline->getIsGhost(),
|
||||||
|
'isSynthetic' => $is_synthetic,
|
||||||
);
|
);
|
||||||
|
|
||||||
$sigil = 'differential-inline-comment';
|
$sigil = 'differential-inline-comment';
|
||||||
|
@ -136,11 +142,6 @@ final class PHUIDiffInlineCommentDetailView
|
||||||
|
|
||||||
$links = array();
|
$links = array();
|
||||||
|
|
||||||
$is_synthetic = false;
|
|
||||||
if ($inline->getSyntheticAuthor()) {
|
|
||||||
$is_synthetic = true;
|
|
||||||
}
|
|
||||||
|
|
||||||
$draft_text = null;
|
$draft_text = null;
|
||||||
if (!$is_synthetic) {
|
if (!$is_synthetic) {
|
||||||
// This display is controlled by CSS
|
// This display is controlled by CSS
|
||||||
|
|
|
@ -1352,8 +1352,16 @@ JX.install('DiffChangesetList', {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (inline.isSynthetic()) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
if (inline.isEditing()) {
|
if (inline.isEditing()) {
|
||||||
unsaved.push(inline);
|
unsaved.push(inline);
|
||||||
|
} else if (!inline.getID()) {
|
||||||
|
// These are new comments which have been cancelled, and do not
|
||||||
|
// count as anything.
|
||||||
|
continue;
|
||||||
} else if (inline.isDraft()) {
|
} else if (inline.isDraft()) {
|
||||||
unsubmitted.push(inline);
|
unsubmitted.push(inline);
|
||||||
} else if (!inline.isDone()) {
|
} else if (!inline.isDone()) {
|
||||||
|
@ -1395,13 +1403,28 @@ JX.install('DiffChangesetList', {
|
||||||
}
|
}
|
||||||
|
|
||||||
if (done.length || undone.length) {
|
if (done.length || undone.length) {
|
||||||
done_button.setText([
|
// If you haven't marked any comments as "Done", we just show text
|
||||||
done.length,
|
// like "3 Comments". If you've marked at least one done, we show
|
||||||
' / ',
|
// "1 / 3 Comments".
|
||||||
(done.length + undone.length),
|
|
||||||
' ',
|
var done_text;
|
||||||
pht('Comments')
|
if (done.length) {
|
||||||
]);
|
done_text = [
|
||||||
|
done.length,
|
||||||
|
' / ',
|
||||||
|
(done.length + undone.length),
|
||||||
|
' ',
|
||||||
|
pht('Comments')
|
||||||
|
];
|
||||||
|
} else {
|
||||||
|
done_text = [
|
||||||
|
undone.length,
|
||||||
|
' ',
|
||||||
|
pht('Comments')
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
done_button.setText(done_text);
|
||||||
|
|
||||||
JX.DOM.show(done_button.getNode());
|
JX.DOM.show(done_button.getNode());
|
||||||
|
|
||||||
|
|
|
@ -34,6 +34,7 @@ JX.install('DiffInline', {
|
||||||
_isFixed: null,
|
_isFixed: null,
|
||||||
_isEditing: false,
|
_isEditing: false,
|
||||||
_isNew: false,
|
_isNew: false,
|
||||||
|
_isSynthetic: false,
|
||||||
|
|
||||||
bindToRow: function(row) {
|
bindToRow: function(row) {
|
||||||
this._row = row;
|
this._row = row;
|
||||||
|
@ -71,6 +72,7 @@ JX.install('DiffInline', {
|
||||||
this._isDraft = data.isDraft;
|
this._isDraft = data.isDraft;
|
||||||
this._isFixed = data.isFixed;
|
this._isFixed = data.isFixed;
|
||||||
this._isGhost = data.isGhost;
|
this._isGhost = data.isGhost;
|
||||||
|
this._isSynthetic = data.isSynthetic;
|
||||||
|
|
||||||
this._changesetID = data.changesetID;
|
this._changesetID = data.changesetID;
|
||||||
this._isNew = false;
|
this._isNew = false;
|
||||||
|
@ -97,6 +99,10 @@ JX.install('DiffInline', {
|
||||||
return this._isDeleted;
|
return this._isDeleted;
|
||||||
},
|
},
|
||||||
|
|
||||||
|
isSynthetic: function() {
|
||||||
|
return this._isSynthetic;
|
||||||
|
},
|
||||||
|
|
||||||
bindToRange: function(data) {
|
bindToRange: function(data) {
|
||||||
this._displaySide = data.displaySide;
|
this._displaySide = data.displaySide;
|
||||||
this._number = parseInt(data.number, 10);
|
this._number = parseInt(data.number, 10);
|
||||||
|
|
Loading…
Reference in a new issue