From e556d205772a68a62d8dfdfbe5babe9aa4af0c2e Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 1 Mar 2014 11:23:08 -0800 Subject: [PATCH] Fix some issues where Conpherence would make to many draft requests Summary: A few minor fixes: - When we build a tag with `"meta" => null`, strip the attribute like we do for all other attributes. Previously, we would actually set the metadata to `null`. This happened with the Conpherence form. - Just respond to the draft request with an empty (but valid) response, instead of building a dialog. - `PhabricatorShapedRequest` is confusingly named and I should have caught this in review, but the basic shape of it is: - You make one object. - You call `trigger()` when stuff changes (e.g., a keystroke). - It manages making a small number of requests (e.g., one request after the user stops typing for a moment). - The way it was being used previously would incorrectly send a request for every keystroke. I think I'm going to simplify `ShapedRequest` and merge it into some larger queue for T430. Test Plan: Typed some text, no longer saw a flurry of requests. Reloaded page, still saw draft text. Reviewers: btrahan, chad Reviewed By: chad CC: aran, chad Differential Revision: https://secure.phabricator.com/D8380 --- resources/celerity/map.php | 30 +++++++++---------- .../ConpherenceUpdateController.php | 2 +- src/infrastructure/javelin/markup.php | 12 +++++--- .../application/conpherence/behavior-menu.js | 25 +++++++++------- 4 files changed, 39 insertions(+), 30 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 77a33edc28..39bf4ca4e4 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -348,7 +348,7 @@ return array( 'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => '845731b8', 'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18', 'rsrc/js/application/config/behavior-reorder-fields.js' => '938aed89', - 'rsrc/js/application/conpherence/behavior-menu.js' => '7ff0b011', + 'rsrc/js/application/conpherence/behavior-menu.js' => '7ee23816', 'rsrc/js/application/conpherence/behavior-pontificate.js' => '53f6f2dd', 'rsrc/js/application/conpherence/behavior-widget-pane.js' => 'd8ef8659', 'rsrc/js/application/countdown/timer.js' => '889c96f3', @@ -535,7 +535,7 @@ return array( 'javelin-behavior-audit-preview' => 'be81801d', 'javelin-behavior-balanced-payment-form' => '3b3e1664', 'javelin-behavior-config-reorder-fields' => '938aed89', - 'javelin-behavior-conpherence-menu' => '7ff0b011', + 'javelin-behavior-conpherence-menu' => '7ee23816', 'javelin-behavior-conpherence-pontificate' => '53f6f2dd', 'javelin-behavior-conpherence-widget-pane' => 'd8ef8659', 'javelin-behavior-countdown-timer' => '889c96f3', @@ -1225,6 +1225,13 @@ return array( 2 => 'javelin-util', 3 => 'phabricator-shaped-request', ), + '62e18640' => + array( + 0 => 'javelin-install', + 1 => 'javelin-util', + 2 => 'javelin-dom', + 3 => 'javelin-typeahead-normalizer', + ), '6453c869' => array( 0 => 'javelin-install', @@ -1258,13 +1265,6 @@ return array( 0 => 'javelin-behavior', 1 => 'javelin-dom', ), - '62e18640' => - array( - 0 => 'javelin-install', - 1 => 'javelin-util', - 2 => 'javelin-dom', - 3 => 'javelin-typeahead-normalizer', - ), '75903ee1' => array( 0 => 'javelin-behavior', @@ -1313,12 +1313,7 @@ return array( 0 => 'herald-rule-editor', 1 => 'javelin-behavior', ), - '7ee2b591' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-history', - ), - '7ff0b011' => + '7ee23816' => array( 0 => 'javelin-behavior', 1 => 'javelin-dom', @@ -1330,6 +1325,11 @@ return array( 7 => 'javelin-vector', 8 => 'phabricator-shaped-request', ), + '7ee2b591' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-history', + ), '82947dda' => array( 0 => 'javelin-behavior', diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index d548838fd0..c4f1f55916 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -51,7 +51,7 @@ final class ConpherenceUpdateController $conpherence->getPHID()); $draft->setDraft($request->getStr('text')); $draft->replaceOrDelete(); - break; + return new AphrontAjaxResponse(); case ConpherenceUpdateActions::MESSAGE: $message = $request->getStr('text'); $xactions = $editor->generateTransactionsFromText( diff --git a/src/infrastructure/javelin/markup.php b/src/infrastructure/javelin/markup.php index b845b33ae8..e5b3ab36b9 100644 --- a/src/infrastructure/javelin/markup.php +++ b/src/infrastructure/javelin/markup.php @@ -11,13 +11,17 @@ function javelin_tag( foreach ($attributes as $k => $v) { switch ($k) { case 'sigil': - $attributes['data-sigil'] = $v; + if ($v !== null) { + $attributes['data-sigil'] = $v; + } unset($attributes[$k]); break; case 'meta': - $response = CelerityAPI::getStaticResourceResponse(); - $id = $response->addMetadata($v); - $attributes['data-meta'] = $id; + if ($v !== null) { + $response = CelerityAPI::getStaticResourceResponse(); + $id = $response->addMetadata($v); + $attributes['data-meta'] = $id; + } unset($attributes[$k]); break; case 'mustcapture': diff --git a/webroot/rsrc/js/application/conpherence/behavior-menu.js b/webroot/rsrc/js/application/conpherence/behavior-menu.js index 5b06df5d62..09f71c939d 100644 --- a/webroot/rsrc/js/application/conpherence/behavior-menu.js +++ b/webroot/rsrc/js/application/conpherence/behavior-menu.js @@ -530,16 +530,21 @@ JX.behavior('conpherence-menu', function(config) { var onkeydownDraft = function (e) { var form = e.getNode('tag:form'); - var uri = config.baseURI + 'update/' + _thread.selected + '/'; - var draftRequest = new JX.PhabricatorShapedRequest( - uri, - JX.bag, - function () { - var data = JX.DOM.convertFormToDictionary(form); - data.action = 'draft'; - return data; - }); - draftRequest.start(); + var data = e.getNodeData('tag:form'); + + if (!data.preview) { + var uri = config.baseURI + 'update/' + _thread.selected + '/'; + data.preview = new JX.PhabricatorShapedRequest( + uri, + JX.bag, + function () { + var data = JX.DOM.convertFormToDictionary(form); + data.action = 'draft'; + return data; + }); + } + + data.preview.trigger(); }; JX.Stratcom.listen(