Skip to content

Commit 44c3283

Browse files
author
epriestley
committedNov 16, 2018
When you "Request Review" of a draft revision, change the button text from "Submit Quietly" to "Publish Revision"
Summary: See PHI975. Ref T13216. Ref T2543. Previously, see D19204 and PHI433. When you're acting on a draft revision, we change the button text to "Submit Quietly" as a hint that your actions don't generate notifications yet. However, this isn't accurate when one of your actions is "Request Review", which causes the revision to publish. Allow actions to override the submit button text, and make the "Request Review" action change the button text to "Publish Revision". The alternative change I considered was to remove the word "Quietly" in all cases. I'm not //thrilled// about how complex this change is to adjust one word, but the various pieces are all fairly clean individually. I'm not sure we'll ever be able to use it for anything else, but I do suspect that the word "Quietly" was the change in D19204 with the largest effect by far (see T10000). Test Plan: - Created a draft revision. Saw "Submit Quietly" text. - Added a "Request Review" action, saw it change to "Publish Revision". - Reloaded page, saw stack saved and "Publish Revision". - Removed action, saw "Submit Quietly". - Repeated on a non-draft revision, button stayed put as "Submit". - Submitted the various actions, saw them have the desired effects. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13216, T2543 Differential Revision: https://secure.phabricator.com/D19810
1 parent ec452e5 commit 44c3283

File tree

9 files changed

+99
-18
lines changed

9 files changed

+99
-18
lines changed
 

‎resources/celerity/map.php

+11-11
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@
422422
'rsrc/js/application/repository/repository-crossreference.js' => '9a860428',
423423
'rsrc/js/application/search/behavior-reorder-profile-menu-items.js' => 'e2e0a072',
424424
'rsrc/js/application/search/behavior-reorder-queries.js' => 'e9581f08',
425-
'rsrc/js/application/transactions/behavior-comment-actions.js' => '038bf27f',
425+
'rsrc/js/application/transactions/behavior-comment-actions.js' => '59e27e74',
426426
'rsrc/js/application/transactions/behavior-reorder-configs.js' => 'd7a74243',
427427
'rsrc/js/application/transactions/behavior-reorder-fields.js' => 'b59e1e96',
428428
'rsrc/js/application/transactions/behavior-show-older-transactions.js' => '8f29b364',
@@ -574,7 +574,7 @@
574574
'javelin-behavior-bulk-job-reload' => 'edf8a145',
575575
'javelin-behavior-calendar-month-view' => 'fe33e256',
576576
'javelin-behavior-choose-control' => '327a00d1',
577-
'javelin-behavior-comment-actions' => '038bf27f',
577+
'javelin-behavior-comment-actions' => '59e27e74',
578578
'javelin-behavior-config-reorder-fields' => 'b6993408',
579579
'javelin-behavior-conpherence-menu' => '4047cd35',
580580
'javelin-behavior-conpherence-participant-pane' => 'd057e45a',
@@ -903,15 +903,6 @@
903903
'javelin-behavior',
904904
'javelin-uri',
905905
),
906-
'038bf27f' => array(
907-
'javelin-behavior',
908-
'javelin-stratcom',
909-
'javelin-workflow',
910-
'javelin-dom',
911-
'phuix-form-control-view',
912-
'phuix-icon-view',
913-
'javelin-behavior-phabricator-gesture',
914-
),
915906
'040fce04' => array(
916907
'javelin-behavior',
917908
'javelin-request',
@@ -1319,6 +1310,15 @@
13191310
'javelin-vector',
13201311
'javelin-dom',
13211312
),
1313+
'59e27e74' => array(
1314+
'javelin-behavior',
1315+
'javelin-stratcom',
1316+
'javelin-workflow',
1317+
'javelin-dom',
1318+
'phuix-form-control-view',
1319+
'phuix-icon-view',
1320+
'javelin-behavior-phabricator-gesture',
1321+
),
13221322
'5c54cbf3' => array(
13231323
'javelin-behavior',
13241324
'javelin-stratcom',

‎src/applications/differential/xaction/DifferentialRevisionActionTransaction.php

+8
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ protected function getRevisionActionDescription(
5757
return null;
5858
}
5959

60+
protected function getRevisionActionSubmitButtonText(
61+
DifferentialRevision $revision) {
62+
return null;
63+
}
64+
6065
public static function loadAllActions() {
6166
return id(new PhutilClassMapQuery())
6267
->setAncestorClass(__CLASS__)
@@ -110,6 +115,9 @@ public function newEditField(
110115
$group_key = $this->getRevisionActionGroupKey();
111116
$field->setCommentActionGroupKey($group_key);
112117

118+
$button_text = $this->getRevisionActionSubmitButtonText($revision);
119+
$field->setActionSubmitButtonText($button_text);
120+
113121
// Currently, every revision action conflicts with every other
114122
// revision action: for example, you can not simultaneously Accept and
115123
// Reject a revision.

‎src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php

+13
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,19 @@ protected function getRevisionActionDescription(
1919
}
2020
}
2121

22+
protected function getRevisionActionSubmitButtonText(
23+
DifferentialRevision $revision) {
24+
25+
// See PHI975. When the action stack will promote the revision out of
26+
// draft, change the button text from "Submit Quietly".
27+
if ($revision->isDraft()) {
28+
return pht('Publish Revision');
29+
}
30+
31+
return null;
32+
}
33+
34+
2235
public function getColor() {
2336
return 'sky';
2437
}

‎src/applications/owners/query/PhabricatorOwnersPackageQuery.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
228228
$repository_phid,
229229
$indexes);
230230
}
231-
$where[] = implode(' OR ', $clauses);
231+
$where[] = qsprintf($conn, '%LO', $clauses);
232232
}
233233

234234
return $where;

‎src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php

+10
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ abstract class PhabricatorEditEngineCommentAction extends Phobject {
99
private $order;
1010
private $groupKey;
1111
private $conflictKey;
12+
private $submitButtonText;
1213

1314
abstract public function getPHUIXControlType();
1415
abstract public function getPHUIXControlSpecification();
@@ -81,4 +82,13 @@ public function getInitialValue() {
8182
return $this->initialValue;
8283
}
8384

85+
public function setSubmitButtonText($text) {
86+
$this->submitButtonText = $text;
87+
return $this;
88+
}
89+
90+
public function getSubmitButtonText() {
91+
return $this->submitButtonText;
92+
}
93+
8494
}

‎src/applications/transactions/editfield/PhabricatorApplyEditField.php

+16-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ final class PhabricatorApplyEditField
55

66
private $actionDescription;
77
private $actionConflictKey;
8+
private $actionSubmitButtonText;
89
private $options;
910

1011
protected function newControl() {
@@ -29,6 +30,15 @@ public function getActionConflictKey() {
2930
return $this->actionConflictKey;
3031
}
3132

33+
public function setActionSubmitButtonText($text) {
34+
$this->actionSubmitButtonText = $text;
35+
return $this;
36+
}
37+
38+
public function getActionSubmitButtonText() {
39+
return $this->actionSubmitButtonText;
40+
}
41+
3242
public function setOptions(array $options) {
3343
$this->options = $options;
3444
return $this;
@@ -59,14 +69,16 @@ public function shouldGenerateTransactionsFromSubmit() {
5969
protected function newCommentAction() {
6070
$options = $this->getOptions();
6171
if ($options) {
62-
return id(new PhabricatorEditEngineCheckboxesCommentAction())
63-
->setConflictKey($this->getActionConflictKey())
72+
$action = id(new PhabricatorEditEngineCheckboxesCommentAction())
6473
->setOptions($options);
6574
} else {
66-
return id(new PhabricatorEditEngineStaticCommentAction())
67-
->setConflictKey($this->getActionConflictKey())
75+
$action = id(new PhabricatorEditEngineStaticCommentAction())
6876
->setDescription($this->getActionDescription());
6977
}
78+
79+
return $action
80+
->setConflictKey($this->getActionConflictKey())
81+
->setSubmitButtonText($this->getActionSubmitButtonText());
7082
}
7183

7284
}

‎src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,6 @@ private function renderCommentPanel() {
329329
$key = $comment_action->getKey();
330330
$label = $comment_action->getLabel();
331331

332-
333332
$action_map[$key] = array(
334333
'key' => $key,
335334
'label' => $label,
@@ -339,6 +338,7 @@ private function renderCommentPanel() {
339338
'groupKey' => $comment_action->getGroupKey(),
340339
'conflictKey' => $comment_action->getConflictKey(),
341340
'auralLabel' => pht('Remove Action: %s', $label),
341+
'buttonText' => $comment_action->getSubmitButtonText(),
342342
);
343343

344344
$type_map[$key] = $comment_action;
@@ -404,6 +404,7 @@ private function renderCommentPanel() {
404404
'showPreview' => $this->getShowPreview(),
405405
'actionURI' => $this->getAction(),
406406
'drafts' => $draft_keys,
407+
'defaultButtonText' => $this->getSubmitButtonName(),
407408
));
408409
}
409410

@@ -426,6 +427,7 @@ private function renderCommentPanel() {
426427
id(new AphrontFormSubmitControl())
427428
->addClass('phui-comment-fullwidth-control')
428429
->addClass('phui-comment-submit-control')
430+
->addSigil('submit-transactions')
429431
->setValue($this->getSubmitButtonName()));
430432

431433
return $form;

‎src/view/form/control/AphrontFormSubmitControl.php

+15-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
final class AphrontFormSubmitControl extends AphrontFormControl {
44

55
private $buttons = array();
6+
private $sigils = array();
67

78
public function addCancelButton($href, $label = null) {
89
if (!$label) {
@@ -22,18 +23,31 @@ public function addButton(PHUIButtonView $button) {
2223
return $this;
2324
}
2425

26+
public function addSigil($sigil) {
27+
$this->sigils[] = $sigil;
28+
return $this;
29+
}
30+
2531
protected function getCustomControlClass() {
2632
return 'aphront-form-control-submit';
2733
}
2834

2935
protected function renderInput() {
3036
$submit_button = null;
3137
if ($this->getValue()) {
32-
$submit_button = phutil_tag(
38+
39+
if ($this->sigils) {
40+
$sigils = $this->sigils;
41+
} else {
42+
$sigils = null;
43+
}
44+
45+
$submit_button = javelin_tag(
3346
'button',
3447
array(
3548
'type' => 'submit',
3649
'name' => '__submit__',
50+
'sigil' => $sigils,
3751
'disabled' => $this->getDisabled() ? 'disabled' : null,
3852
),
3953
$this->getValue());

‎webroot/rsrc/js/application/transactions/behavior-comment-actions.js

+22
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,31 @@ JX.behavior('comment-actions', function(config) {
4343
return null;
4444
}
4545

46+
function redraw() {
47+
// If any of the stacked actions specify that they change the label for
48+
// the "Submit" button, update the button text. Otherwise, return it to
49+
// the default text.
50+
var button_text = config.defaultButtonText;
51+
for (var k in rows) {
52+
var action = action_map[k];
53+
if (action.buttonText) {
54+
button_text = action.buttonText;
55+
}
56+
}
57+
58+
var button_node = JX.DOM.find(form_node, 'button', 'submit-transactions');
59+
JX.DOM.setContent(button_node, button_text);
60+
}
61+
4662
function remove_action(key) {
4763
var row = rows[key];
4864
if (row) {
4965
JX.DOM.remove(row.node);
5066
row.option.disabled = false;
5167
delete rows[key];
5268
}
69+
70+
redraw();
5371
}
5472

5573
function serialize_actions() {
@@ -90,6 +108,8 @@ JX.behavior('comment-actions', function(config) {
90108

91109
control = add_row(option);
92110
}
111+
112+
redraw();
93113
}
94114

95115
function onresponse(response) {
@@ -209,6 +229,8 @@ JX.behavior('comment-actions', function(config) {
209229

210230
place_node.parentNode.insertBefore(node, place_node);
211231

232+
redraw();
233+
212234
force_preview();
213235

214236
return control;

0 commit comments

Comments
 (0)
Failed to load comments.