From e5580d038dd17cdc715724052170096fd17bb116 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 14 Mar 2015 12:00:17 -0700 Subject: [PATCH] Improve Conpherence column textarea JS behaviors Summary: - Don't show a loading state on the whole column while sending chat. We could show some kind of minor loading state, but standard JX.Busy stuff will kick in after a couple seconds anyway. - Blank the textarea immediately on submit so you can start typing more text. - Don't disable the form while submiting; disabling it prevents you from typing more text. - Hide the placeholder while the textarea is focused. If we don't do this, the placeholder reappearing after submitting text feels weird to me. Test Plan: - Sent a lot of text. - Real fast. - Focused and unfocused the area. Reviewers: btrahan, chad Reviewed By: chad Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D12086 --- resources/celerity/map.php | 76 +++++++++---------- .../rsrc/externals/javelin/lib/Workflow.js | 28 ++++--- .../conpherence/ConpherenceThreadManager.js | 7 +- .../conpherence/behavior-durable-column.js | 47 ++++++++++-- 4 files changed, 99 insertions(+), 59 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 2ded5185cf..3aa9f05851 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,7 +8,7 @@ return array( 'names' => array( 'core.pkg.css' => 'a7f3dc55', - 'core.pkg.js' => '31bc6546', + 'core.pkg.js' => '69f7e542', 'darkconsole.pkg.js' => '8ab24e01', 'differential.pkg.css' => '1940be3f', 'differential.pkg.js' => 'be1e5f9b', @@ -212,7 +212,7 @@ return array( 'rsrc/externals/javelin/lib/URI.js' => '6eff08aa', 'rsrc/externals/javelin/lib/Vector.js' => '2caa8fb8', 'rsrc/externals/javelin/lib/WebSocket.js' => 'e292eaf4', - 'rsrc/externals/javelin/lib/Workflow.js' => '84d6aea0', + 'rsrc/externals/javelin/lib/Workflow.js' => '5b2e3e2b', 'rsrc/externals/javelin/lib/__tests__/Cookie.js' => '5ed109e8', 'rsrc/externals/javelin/lib/__tests__/DOM.js' => 'c984504b', 'rsrc/externals/javelin/lib/__tests__/JSON.js' => '837a7d68', @@ -352,8 +352,8 @@ return array( 'rsrc/js/application/aphlict/behavior-aphlict-status.js' => 'ea681761', 'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18', 'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de', - 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => '0324970d', - 'rsrc/js/application/conpherence/behavior-durable-column.js' => '217c5ea5', + 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'cff1902b', + 'rsrc/js/application/conpherence/behavior-durable-column.js' => '332ac18d', 'rsrc/js/application/conpherence/behavior-menu.js' => 'c4151295', 'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861', 'rsrc/js/application/conpherence/behavior-quicksand-blacklist.js' => '7927a7d3', @@ -517,7 +517,7 @@ return array( 'conpherence-menu-css' => 'c6ac5299', 'conpherence-message-pane-css' => '5930260a', 'conpherence-notification-css' => '04a6e10a', - 'conpherence-thread-manager' => '0324970d', + 'conpherence-thread-manager' => 'cff1902b', 'conpherence-update-css' => '1099a660', 'conpherence-widget-pane-css' => '3d575438', 'differential-changeset-view-css' => '6a8b172a', @@ -583,7 +583,7 @@ return array( 'javelin-behavior-diffusion-locate-file' => '6d3e1947', 'javelin-behavior-diffusion-pull-lastmodified' => '2b228192', 'javelin-behavior-doorkeeper-tag' => 'e5822781', - 'javelin-behavior-durable-column' => '217c5ea5', + 'javelin-behavior-durable-column' => '332ac18d', 'javelin-behavior-error-log' => '6882e80a', 'javelin-behavior-fancy-datepicker' => 'c51ae228', 'javelin-behavior-global-drag-and-drop' => 'bbdf75ca', @@ -697,7 +697,7 @@ return array( 'javelin-view-renderer' => '6c2b09a2', 'javelin-view-visitor' => 'efe49472', 'javelin-websocket' => 'e292eaf4', - 'javelin-workflow' => '84d6aea0', + 'javelin-workflow' => '5b2e3e2b', 'lightbox-attachment-css' => '7acac05d', 'maniphest-batch-editor' => '8f380ebc', 'maniphest-report-css' => 'f6931fdf', @@ -843,16 +843,6 @@ return array( '029a133d' => array( 'aphront-dialog-view-css', ), - '0324970d' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - ), '03d6ed07' => array( 'javelin-behavior', 'javelin-stratcom', @@ -973,16 +963,6 @@ return array( 'phabricator-phtize', 'changeset-view-manager', ), - '217c5ea5' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-behavior-device', - 'javelin-scrollbar', - 'javelin-quicksand', - 'phabricator-keyboard-shortcut', - 'conpherence-thread-manager', - ), '21ba5861' => array( 'javelin-behavior', 'javelin-dom', @@ -1074,6 +1054,16 @@ return array( '331b1611' => array( 'javelin-install', ), + '332ac18d' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-behavior-device', + 'javelin-scrollbar', + 'javelin-quicksand', + 'phabricator-keyboard-shortcut', + 'conpherence-thread-manager', + ), '3ab51e2c' => array( 'javelin-behavior', 'javelin-behavior-device', @@ -1216,6 +1206,17 @@ return array( 'javelin-vector', 'javelin-dom', ), + '5b2e3e2b' => array( + 'javelin-stratcom', + 'javelin-request', + 'javelin-dom', + 'javelin-vector', + 'javelin-install', + 'javelin-util', + 'javelin-mask', + 'javelin-uri', + 'javelin-routable', + ), '5c1c758c' => array( 'javelin-install', ), @@ -1454,17 +1455,6 @@ return array( 'javelin-workflow', 'phabricator-draggable-list', ), - '84d6aea0' => array( - 'javelin-stratcom', - 'javelin-request', - 'javelin-dom', - 'javelin-vector', - 'javelin-install', - 'javelin-util', - 'javelin-mask', - 'javelin-uri', - 'javelin-routable', - ), '85ea0626' => array( 'javelin-install', ), @@ -1776,6 +1766,16 @@ return array( 'javelin-stratcom', 'phabricator-phtize', ), + 'cff1902b' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + ), 'd19198c8' => array( 'javelin-install', 'javelin-dom', diff --git a/webroot/rsrc/externals/javelin/lib/Workflow.js b/webroot/rsrc/externals/javelin/lib/Workflow.js index 8921c862ab..b640adecab 100644 --- a/webroot/rsrc/externals/javelin/lib/Workflow.js +++ b/webroot/rsrc/externals/javelin/lib/Workflow.js @@ -29,22 +29,27 @@ JX.install('Workflow', { statics : { _stack : [], - newFromForm : function(form, data) { + newFromForm : function(form, data, keep_enabled) { var pairs = JX.DOM.convertFormToListOfPairs(form); for (var k in data) { pairs.push([k, data[k]]); } - // Disable form elements during the request - var inputs = [].concat( - JX.DOM.scry(form, 'input'), - JX.DOM.scry(form, 'button'), - JX.DOM.scry(form, 'textarea')); - for (var ii = 0; ii < inputs.length; ii++) { - if (inputs[ii].disabled) { - delete inputs[ii]; - } else { - inputs[ii].disabled = true; + var inputs; + if (keep_enabled) { + inputs = []; + } else { + // Disable form elements during the request + inputs = [].concat( + JX.DOM.scry(form, 'input'), + JX.DOM.scry(form, 'button'), + JX.DOM.scry(form, 'textarea')); + for (var ii = 0; ii < inputs.length; ii++) { + if (inputs[ii].disabled) { + delete inputs[ii]; + } else { + inputs[ii].disabled = true; + } } } @@ -57,6 +62,7 @@ JX.install('Workflow', { inputs[ii] && (inputs[ii].disabled = false); } }); + return workflow; }, newFromLink : function(link) { diff --git a/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js b/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js index cb9473516b..30b5dab5b3 100644 --- a/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js +++ b/webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js @@ -245,13 +245,16 @@ JX.install('ConpherenceThreadManager', { sendMessage: function(form, params) { params = this._getParams(params); - this._willSendMessageCallback(); - var workflow = JX.Workflow.newFromForm(form, params) + var keep_enabled = true; + + var workflow = JX.Workflow.newFromForm(form, params, keep_enabled) .setHandler(JX.bind(this, function(r) { this._latestTransactionID = r.latest_transaction_id; this._didSendMessageCallback(r); })); this.syncWorkflow(workflow, 'finally'); + + this._willSendMessageCallback(); }, handleDraftKeydown: function(e) { diff --git a/webroot/rsrc/js/application/conpherence/behavior-durable-column.js b/webroot/rsrc/js/application/conpherence/behavior-durable-column.js index a9b5152677..346df6a3e0 100644 --- a/webroot/rsrc/js/application/conpherence/behavior-durable-column.js +++ b/webroot/rsrc/js/application/conpherence/behavior-durable-column.js @@ -102,21 +102,20 @@ JX.behavior('durable-column', function(config, statics) { JX.DOM.appendContent(messages, JX.$H(r.transactions)); scrollbar.scrollTo(messages.scrollHeight); }); + threadManager.setWillSendMessageCallback(function() { - _markLoading(true); + // Wipe the textarea immediately so the user can start typing more text. + var textarea = _getColumnTextareaNode(); + textarea.value = ''; + _focusColumnTextareaNode(); }); + threadManager.setDidSendMessageCallback(function(r) { var messages = _getColumnMessagesNode(); JX.DOM.appendContent(messages, JX.$H(r.transactions)); scrollbar.scrollTo(messages.scrollHeight); - - var textarea = _getColumnTextareaNode(); - textarea.value = ''; - - _markLoading(false); - - _focusColumnTextareaNode(); }); + threadManager.setWillUpdateWorkflowCallback(function() { JX.Stratcom.invoke('notification-panel-close'); }); @@ -289,10 +288,20 @@ JX.behavior('durable-column', function(config, statics) { if (e.getSpecialKey() != 'return') { return; } + var raw = e.getRawEvent(); if (raw.shiftKey) { + // If the shift key is pressed, let the browser write a newline into + // the textarea. return; } + + var textarea = _getColumnTextareaNode(); + if (!textarea.value.length) { + // If there's no text, don't try to submit the form. + return; + } + _sendMessage(e); }); @@ -303,6 +312,28 @@ JX.behavior('durable-column', function(config, statics) { threadManager.handleDraftKeydown(e); }); + // HTML5 placeholders are rendered as long as the input is empty, even if the + // input is currently focused. This is undesirable for the chat input, + // especially immediately after sending a message. Hide the placeholder while + // the input is focused. + JX.Stratcom.listen( + ['focus', 'blur'], + 'conpherence-durable-column-textarea', + function (e) { + var node = e.getTarget(); + if (e.getType() == 'focus') { + if (node.placeholder) { + node.placeholderStorage = node.placeholder; + node.placeholder = ''; + } + } else { + if (node.placeholderStorage) { + node.placeholder = node.placeholderStorage; + node.placeholderStorage = ''; + } + } + }); + if (config.visible) { var device = JX.Device.getDevice(); if (device == 'desktop') {