From 44c32839a68cd8f98fa38a1e321cd5526b213d92 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 15 Nov 2018 05:02:29 -0800 Subject: [PATCH] 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 --- resources/celerity/map.php | 22 +++++++++---------- .../DifferentialRevisionActionTransaction.php | 8 +++++++ ...entialRevisionRequestReviewTransaction.php | 13 +++++++++++ .../query/PhabricatorOwnersPackageQuery.php | 2 +- .../PhabricatorEditEngineCommentAction.php | 10 +++++++++ .../editfield/PhabricatorApplyEditField.php | 20 +++++++++++++---- ...catorApplicationTransactionCommentView.php | 4 +++- .../form/control/AphrontFormSubmitControl.php | 16 +++++++++++++- .../transactions/behavior-comment-actions.js | 22 +++++++++++++++++++ 9 files changed, 99 insertions(+), 18 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index e147d89128..7d6e766c44 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -422,7 +422,7 @@ return array( 'rsrc/js/application/repository/repository-crossreference.js' => '9a860428', 'rsrc/js/application/search/behavior-reorder-profile-menu-items.js' => 'e2e0a072', 'rsrc/js/application/search/behavior-reorder-queries.js' => 'e9581f08', - 'rsrc/js/application/transactions/behavior-comment-actions.js' => '038bf27f', + 'rsrc/js/application/transactions/behavior-comment-actions.js' => '59e27e74', 'rsrc/js/application/transactions/behavior-reorder-configs.js' => 'd7a74243', 'rsrc/js/application/transactions/behavior-reorder-fields.js' => 'b59e1e96', 'rsrc/js/application/transactions/behavior-show-older-transactions.js' => '8f29b364', @@ -574,7 +574,7 @@ return array( 'javelin-behavior-bulk-job-reload' => 'edf8a145', 'javelin-behavior-calendar-month-view' => 'fe33e256', 'javelin-behavior-choose-control' => '327a00d1', - 'javelin-behavior-comment-actions' => '038bf27f', + 'javelin-behavior-comment-actions' => '59e27e74', 'javelin-behavior-config-reorder-fields' => 'b6993408', 'javelin-behavior-conpherence-menu' => '4047cd35', 'javelin-behavior-conpherence-participant-pane' => 'd057e45a', @@ -903,15 +903,6 @@ return array( 'javelin-behavior', 'javelin-uri', ), - '038bf27f' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-workflow', - 'javelin-dom', - 'phuix-form-control-view', - 'phuix-icon-view', - 'javelin-behavior-phabricator-gesture', - ), '040fce04' => array( 'javelin-behavior', 'javelin-request', @@ -1319,6 +1310,15 @@ return array( 'javelin-vector', 'javelin-dom', ), + '59e27e74' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-workflow', + 'javelin-dom', + 'phuix-form-control-view', + 'phuix-icon-view', + 'javelin-behavior-phabricator-gesture', + ), '5c54cbf3' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php index cf2b557764..99dc8bebc6 100644 --- a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php @@ -57,6 +57,11 @@ abstract class DifferentialRevisionActionTransaction return null; } + protected function getRevisionActionSubmitButtonText( + DifferentialRevision $revision) { + return null; + } + public static function loadAllActions() { return id(new PhutilClassMapQuery()) ->setAncestorClass(__CLASS__) @@ -110,6 +115,9 @@ abstract class DifferentialRevisionActionTransaction $group_key = $this->getRevisionActionGroupKey(); $field->setCommentActionGroupKey($group_key); + $button_text = $this->getRevisionActionSubmitButtonText($revision); + $field->setActionSubmitButtonText($button_text); + // Currently, every revision action conflicts with every other // revision action: for example, you can not simultaneously Accept and // Reject a revision. diff --git a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php index a3d2699c74..169e41dec5 100644 --- a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php @@ -19,6 +19,19 @@ final class DifferentialRevisionRequestReviewTransaction } } + protected function getRevisionActionSubmitButtonText( + DifferentialRevision $revision) { + + // See PHI975. When the action stack will promote the revision out of + // draft, change the button text from "Submit Quietly". + if ($revision->isDraft()) { + return pht('Publish Revision'); + } + + return null; + } + + public function getColor() { return 'sky'; } diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php index ff0665b344..6d6ccb2ed2 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php @@ -228,7 +228,7 @@ final class PhabricatorOwnersPackageQuery $repository_phid, $indexes); } - $where[] = implode(' OR ', $clauses); + $where[] = qsprintf($conn, '%LO', $clauses); } return $where; diff --git a/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php b/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php index 4d3b9ca5df..d363e5a14e 100644 --- a/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php +++ b/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php @@ -9,6 +9,7 @@ abstract class PhabricatorEditEngineCommentAction extends Phobject { private $order; private $groupKey; private $conflictKey; + private $submitButtonText; abstract public function getPHUIXControlType(); abstract public function getPHUIXControlSpecification(); @@ -81,4 +82,13 @@ abstract class PhabricatorEditEngineCommentAction extends Phobject { return $this->initialValue; } + public function setSubmitButtonText($text) { + $this->submitButtonText = $text; + return $this; + } + + public function getSubmitButtonText() { + return $this->submitButtonText; + } + } diff --git a/src/applications/transactions/editfield/PhabricatorApplyEditField.php b/src/applications/transactions/editfield/PhabricatorApplyEditField.php index d349767f94..02d4cad31f 100644 --- a/src/applications/transactions/editfield/PhabricatorApplyEditField.php +++ b/src/applications/transactions/editfield/PhabricatorApplyEditField.php @@ -5,6 +5,7 @@ final class PhabricatorApplyEditField private $actionDescription; private $actionConflictKey; + private $actionSubmitButtonText; private $options; protected function newControl() { @@ -29,6 +30,15 @@ final class PhabricatorApplyEditField return $this->actionConflictKey; } + public function setActionSubmitButtonText($text) { + $this->actionSubmitButtonText = $text; + return $this; + } + + public function getActionSubmitButtonText() { + return $this->actionSubmitButtonText; + } + public function setOptions(array $options) { $this->options = $options; return $this; @@ -59,14 +69,16 @@ final class PhabricatorApplyEditField protected function newCommentAction() { $options = $this->getOptions(); if ($options) { - return id(new PhabricatorEditEngineCheckboxesCommentAction()) - ->setConflictKey($this->getActionConflictKey()) + $action = id(new PhabricatorEditEngineCheckboxesCommentAction()) ->setOptions($options); } else { - return id(new PhabricatorEditEngineStaticCommentAction()) - ->setConflictKey($this->getActionConflictKey()) + $action = id(new PhabricatorEditEngineStaticCommentAction()) ->setDescription($this->getActionDescription()); } + + return $action + ->setConflictKey($this->getActionConflictKey()) + ->setSubmitButtonText($this->getActionSubmitButtonText()); } } diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php index c0f514c904..227854c79f 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php @@ -329,7 +329,6 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { $key = $comment_action->getKey(); $label = $comment_action->getLabel(); - $action_map[$key] = array( 'key' => $key, 'label' => $label, @@ -339,6 +338,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { 'groupKey' => $comment_action->getGroupKey(), 'conflictKey' => $comment_action->getConflictKey(), 'auralLabel' => pht('Remove Action: %s', $label), + 'buttonText' => $comment_action->getSubmitButtonText(), ); $type_map[$key] = $comment_action; @@ -404,6 +404,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { 'showPreview' => $this->getShowPreview(), 'actionURI' => $this->getAction(), 'drafts' => $draft_keys, + 'defaultButtonText' => $this->getSubmitButtonName(), )); } @@ -426,6 +427,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { id(new AphrontFormSubmitControl()) ->addClass('phui-comment-fullwidth-control') ->addClass('phui-comment-submit-control') + ->addSigil('submit-transactions') ->setValue($this->getSubmitButtonName())); return $form; diff --git a/src/view/form/control/AphrontFormSubmitControl.php b/src/view/form/control/AphrontFormSubmitControl.php index c2bf716848..7e948bd135 100644 --- a/src/view/form/control/AphrontFormSubmitControl.php +++ b/src/view/form/control/AphrontFormSubmitControl.php @@ -3,6 +3,7 @@ final class AphrontFormSubmitControl extends AphrontFormControl { private $buttons = array(); + private $sigils = array(); public function addCancelButton($href, $label = null) { if (!$label) { @@ -22,6 +23,11 @@ final class AphrontFormSubmitControl extends AphrontFormControl { return $this; } + public function addSigil($sigil) { + $this->sigils[] = $sigil; + return $this; + } + protected function getCustomControlClass() { return 'aphront-form-control-submit'; } @@ -29,11 +35,19 @@ final class AphrontFormSubmitControl extends AphrontFormControl { protected function renderInput() { $submit_button = null; if ($this->getValue()) { - $submit_button = phutil_tag( + + if ($this->sigils) { + $sigils = $this->sigils; + } else { + $sigils = null; + } + + $submit_button = javelin_tag( 'button', array( 'type' => 'submit', 'name' => '__submit__', + 'sigil' => $sigils, 'disabled' => $this->getDisabled() ? 'disabled' : null, ), $this->getValue()); diff --git a/webroot/rsrc/js/application/transactions/behavior-comment-actions.js b/webroot/rsrc/js/application/transactions/behavior-comment-actions.js index 9cec374f6b..ab962592c7 100644 --- a/webroot/rsrc/js/application/transactions/behavior-comment-actions.js +++ b/webroot/rsrc/js/application/transactions/behavior-comment-actions.js @@ -43,6 +43,22 @@ JX.behavior('comment-actions', function(config) { return null; } + function redraw() { + // If any of the stacked actions specify that they change the label for + // the "Submit" button, update the button text. Otherwise, return it to + // the default text. + var button_text = config.defaultButtonText; + for (var k in rows) { + var action = action_map[k]; + if (action.buttonText) { + button_text = action.buttonText; + } + } + + var button_node = JX.DOM.find(form_node, 'button', 'submit-transactions'); + JX.DOM.setContent(button_node, button_text); + } + function remove_action(key) { var row = rows[key]; if (row) { @@ -50,6 +66,8 @@ JX.behavior('comment-actions', function(config) { row.option.disabled = false; delete rows[key]; } + + redraw(); } function serialize_actions() { @@ -90,6 +108,8 @@ JX.behavior('comment-actions', function(config) { control = add_row(option); } + + redraw(); } function onresponse(response) { @@ -209,6 +229,8 @@ JX.behavior('comment-actions', function(config) { place_node.parentNode.insertBefore(node, place_node); + redraw(); + force_preview(); return control; -- 2.51.2