From af8cc2a5fc758221ffd33239f833876bc23bddb4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 22 Dec 2015 07:48:29 -0800 Subject: [PATCH] Don't select {F...} text after adding it to textareas Summary: Fixes T10039. We add text to textareas in two cases: - Users clicking assitance buttons in Remarkup text areas. - Drag-and-drop file uploads. In case (1), it makes sense to highlight the text (it shows the user what we inserted, and lets them undo the action easily if it isn't what they wanted). In case (2), it does not. Users almost never want to delete or edit a file reference. It is slightly nice to have the reference as a visual callout, but I don't think this is a big deal. Change the behavior so that we highlight only for remarkup buttons, not for drag-and-drop files. Test Plan: - Clicked an "isnert quote" button on remarkup assist area, got highlighted example text. - Dragged and dropped a file, got text inserted with no highlight. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10039 Differential Revision: https://secure.phabricator.com/D14851 --- resources/celerity/map.php | 56 +++++++++---------- webroot/rsrc/js/core/TextAreaUtils.js | 11 +++- .../core/behavior-drag-and-drop-textarea.js | 16 +++--- .../behavior-phabricator-remarkup-assist.js | 2 +- 4 files changed, 46 insertions(+), 39 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 3b15ec78a0..9a55c3fe61 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,10 +8,10 @@ return array( 'names' => array( 'core.pkg.css' => '4cf32aa0', - 'core.pkg.js' => 'e0379ae5', + 'core.pkg.js' => '821768c9', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '2de124c9', - 'differential.pkg.js' => '6223dd9d', + 'differential.pkg.js' => '64e69521', 'diffusion.pkg.css' => 'f45955ed', 'diffusion.pkg.js' => 'ca1c8b5a', 'maniphest.pkg.css' => '4845691a', @@ -459,7 +459,7 @@ return array( 'rsrc/js/core/Notification.js' => 'ccf1cbf8', 'rsrc/js/core/Prefab.js' => '666c80c5', 'rsrc/js/core/ShapedRequest.js' => '7cbe244b', - 'rsrc/js/core/TextAreaUtils.js' => '5c93c52c', + 'rsrc/js/core/TextAreaUtils.js' => '9e54692d', 'rsrc/js/core/Title.js' => 'df5e11d2', 'rsrc/js/core/ToolTip.js' => '1d298e3a', 'rsrc/js/core/behavior-active-nav.js' => 'e379b58e', @@ -469,7 +469,7 @@ return array( 'rsrc/js/core/behavior-crop.js' => 'fa0f4fc2', 'rsrc/js/core/behavior-dark-console.js' => 'f411b6ae', 'rsrc/js/core/behavior-device.js' => 'a205cf28', - 'rsrc/js/core/behavior-drag-and-drop-textarea.js' => '6d49590e', + 'rsrc/js/core/behavior-drag-and-drop-textarea.js' => '4f6a4b4e', 'rsrc/js/core/behavior-error-log.js' => '6882e80a', 'rsrc/js/core/behavior-fancy-datepicker.js' => '8ae55229', 'rsrc/js/core/behavior-file-tree.js' => '88236f00', @@ -487,7 +487,7 @@ return array( 'rsrc/js/core/behavior-object-selector.js' => '49b73b36', 'rsrc/js/core/behavior-oncopy.js' => '2926fff2', 'rsrc/js/core/behavior-phabricator-nav.js' => '56a1ca03', - 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => 'c14b5995', + 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => '461fd61b', 'rsrc/js/core/behavior-refresh-csrf.js' => 'ab2f381b', 'rsrc/js/core/behavior-remarkup-preview.js' => 'f7379f45', 'rsrc/js/core/behavior-reorder-applications.js' => '76b9fc3e', @@ -565,7 +565,7 @@ return array( 'javelin-behavior-aphlict-status' => 'ea681761', 'javelin-behavior-aphront-basic-tokenizer' => 'b3a4b884', 'javelin-behavior-aphront-crop' => 'fa0f4fc2', - 'javelin-behavior-aphront-drag-and-drop-textarea' => '6d49590e', + 'javelin-behavior-aphront-drag-and-drop-textarea' => '4f6a4b4e', 'javelin-behavior-aphront-form-disable-on-submit' => '5c54cbf3', 'javelin-behavior-aphront-more' => 'a80d0378', 'javelin-behavior-audio-source' => '59b251eb', @@ -640,7 +640,7 @@ return array( 'javelin-behavior-phabricator-notification-example' => '8ce821c5', 'javelin-behavior-phabricator-object-selector' => '49b73b36', 'javelin-behavior-phabricator-oncopy' => '2926fff2', - 'javelin-behavior-phabricator-remarkup-assist' => 'c14b5995', + 'javelin-behavior-phabricator-remarkup-assist' => '461fd61b', 'javelin-behavior-phabricator-reveal-content' => '60821bc7', 'javelin-behavior-phabricator-search-typeahead' => '048330fa', 'javelin-behavior-phabricator-show-older-transactions' => 'dbbf48b6', @@ -766,7 +766,7 @@ return array( 'phabricator-slowvote-css' => 'da0afb1b', 'phabricator-source-code-view-css' => 'cbeef983', 'phabricator-standard-page-view' => '3c99cdf4', - 'phabricator-textareautils' => '5c93c52c', + 'phabricator-textareautils' => '9e54692d', 'phabricator-title' => 'df5e11d2', 'phabricator-tooltip' => '1d298e3a', 'phabricator-ui-example-css' => '528b19de', @@ -1100,6 +1100,15 @@ return array( 'javelin-behavior', 'javelin-dom', ), + '461fd61b' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'phabricator-phtize', + 'phabricator-textareautils', + 'javelin-workflow', + 'javelin-vector', + ), '469c0d9e' => array( 'javelin-behavior', 'javelin-dom', @@ -1137,6 +1146,12 @@ return array( 'javelin-stratcom', 'javelin-dom', ), + '4f6a4b4e' => array( + 'javelin-behavior', + 'javelin-dom', + 'phabricator-drag-and-drop-file-upload', + 'phabricator-textareautils', + ), '4fdb476d' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1238,11 +1253,6 @@ return array( 'javelin-stratcom', 'javelin-dom', ), - '5c93c52c' => array( - 'javelin-install', - 'javelin-dom', - 'javelin-vector', - ), '5d7c9f33' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1337,12 +1347,6 @@ return array( 'javelin-typeahead', 'javelin-uri', ), - '6d49590e' => array( - 'javelin-behavior', - 'javelin-dom', - 'phabricator-drag-and-drop-file-upload', - 'phabricator-textareautils', - ), '70baed2f' => array( 'javelin-install', 'javelin-dom', @@ -1557,6 +1561,11 @@ return array( 'javelin-dom', 'javelin-reactor-dom', ), + '9e54692d' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-vector', + ), '9f36c42d' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1765,15 +1774,6 @@ return array( 'javelin-install', 'javelin-dom', ), - 'c14b5995' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'phabricator-phtize', - 'phabricator-textareautils', - 'javelin-workflow', - 'javelin-vector', - ), 'c1700f6f' => array( 'javelin-install', 'javelin-util', diff --git a/webroot/rsrc/js/core/TextAreaUtils.js b/webroot/rsrc/js/core/TextAreaUtils.js index b96099e0e5..f8c84c97a7 100644 --- a/webroot/rsrc/js/core/TextAreaUtils.js +++ b/webroot/rsrc/js/core/TextAreaUtils.js @@ -38,14 +38,21 @@ JX.install('TextAreaUtils', { } }, - setSelectionText : function(area, text) { + setSelectionText : function(area, text, select) { var v = area.value; var r = JX.TextAreaUtils.getSelectionRange(area); v = v.substring(0, r.start) + text + v.substring(r.end, v.length); area.value = v; - JX.TextAreaUtils.setSelectionRange(area, r.start, r.start + text.length); + var start = r.start; + var end = r.start + text.length; + + if (!select) { + start = end; + } + + JX.TextAreaUtils.setSelectionRange(area, start, end); }, /** diff --git a/webroot/rsrc/js/core/behavior-drag-and-drop-textarea.js b/webroot/rsrc/js/core/behavior-drag-and-drop-textarea.js index 03d3d5f54f..7ee536edb4 100644 --- a/webroot/rsrc/js/core/behavior-drag-and-drop-textarea.js +++ b/webroot/rsrc/js/core/behavior-drag-and-drop-textarea.js @@ -11,18 +11,18 @@ JX.behavior('aphront-drag-and-drop-textarea', function(config) { var target = JX.$(config.target); function onupload(f) { - var text = JX.TextAreaUtils.getSelectionText(target); var ref = '{F' + f.getID() + '}'; - // If the user has dragged and dropped multiple files, we'll get an event - // each time an upload completes. Rather than overwriting the first - // reference, append the new reference if the selected text looks like an - // existing file reference. - if (text.match(/^\{F/)) { - ref = text + '\n\n' + ref; + // If we're inserting immediately after a "}" (usually, another file + // reference), put some newlines before our token so that multiple file + // uploads get laid out more nicely. + var range = JX.TextAreaUtils.getSelectionRange(target); + var before = target.value.substring(0, range.start); + if (before.match(/\}$/)) { + ref = '\n\n' + ref; } - JX.TextAreaUtils.setSelectionText(target, ref); + JX.TextAreaUtils.setSelectionText(target, ref, false); } if (JX.PhabricatorDragAndDropFileUpload.isSupported()) { diff --git a/webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js b/webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js index e1407cd9f5..e0747dce47 100644 --- a/webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js +++ b/webroot/rsrc/js/core/behavior-phabricator-remarkup-assist.js @@ -81,7 +81,7 @@ JX.behavior('phabricator-remarkup-assist', function(config) { function update(area, l, m, r) { // Replace the selection with the entire assisted text. - JX.TextAreaUtils.setSelectionText(area, l + m + r); + JX.TextAreaUtils.setSelectionText(area, l + m + r, true); // Now, select just the middle part. For instance, if the user clicked // "B" to create bold text, we insert '**bold**' but just select the word