From dba1b107203318ac7b0b0c7b39f0e1fc01717ac4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 1 Apr 2019 14:13:36 -0700 Subject: [PATCH] Deactivate the remarkup autosuggest once text can't match "[[" or "((" rules Summary: See PHI1185, which reports a performance issue with "(" in remarkup in certain contexts. I can't reproduce the performance issue, but I can reproduce the autosuggester incorrectly remaining active and swallowing return characters. When the user types `(` or `[`, we wait for a prefix for the `((` (Phurl) or `[[` (Phriction) rules. We currently continue looking for that prefix until a character is entered that explicitly interrupts the search. For example, typing `(xxx` does not insert a return character, because we're stuck on matching the prefix. Instead, as soon as the user has entered text that we know won't ever match the prefix, deactivate the autocomplete. We can slightly cheat through this by just looking for at least one character of text, since all prefixes are exactly one character long. If we eventually have some kind of `~~@(xyz)` rule we might need to add a more complicated piece of rejection logic. Test Plan: Typed `(xxx`, got a return. Used `((` and `[[` autosuggest rules normally. Used `JX.log()` to sanity check that nothing too crazy seems to be happening. Reviewers: amckinley Reviewed By: amckinley Differential Revision: https://secure.phabricator.com/D20365 --- resources/celerity/map.php | 16 ++++++++-------- webroot/rsrc/js/phuix/PHUIXAutocomplete.js | 7 +++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 1f36bf03b4..20d648f549 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -516,7 +516,7 @@ return array( 'rsrc/js/phui/behavior-phui-timer-control.js' => 'f84bcbf4', 'rsrc/js/phuix/PHUIXActionListView.js' => 'c68f183f', 'rsrc/js/phuix/PHUIXActionView.js' => 'aaa08f3b', - 'rsrc/js/phuix/PHUIXAutocomplete.js' => '8f139ef0', + 'rsrc/js/phuix/PHUIXAutocomplete.js' => '2fbe234d', 'rsrc/js/phuix/PHUIXButtonView.js' => '55a24e84', 'rsrc/js/phuix/PHUIXDropdownMenu.js' => 'bdce4d78', 'rsrc/js/phuix/PHUIXExample.js' => 'c2c500a7', @@ -872,7 +872,7 @@ return array( 'phui-workpanel-view-css' => '3ae89b20', 'phuix-action-list-view' => 'c68f183f', 'phuix-action-view' => 'aaa08f3b', - 'phuix-autocomplete' => '8f139ef0', + 'phuix-autocomplete' => '2fbe234d', 'phuix-button-view' => '55a24e84', 'phuix-dropdown-menu' => 'bdce4d78', 'phuix-form-control-view' => '38c1f3fb', @@ -1173,6 +1173,12 @@ return array( 'phuix-autocomplete', 'javelin-mask', ), + '2fbe234d' => array( + 'javelin-install', + 'javelin-dom', + 'phuix-icon-view', + 'phabricator-prefab', + ), '308f9fe4' => array( 'javelin-install', 'javelin-util', @@ -1634,12 +1640,6 @@ return array( '8e2d9a28' => array( 'phui-theme-css', ), - '8f139ef0' => array( - 'javelin-install', - 'javelin-dom', - 'phuix-icon-view', - 'phabricator-prefab', - ), '8f959ad0' => array( 'javelin-behavior', 'javelin-dom', diff --git a/webroot/rsrc/js/phuix/PHUIXAutocomplete.js b/webroot/rsrc/js/phuix/PHUIXAutocomplete.js index deb9f9d100..2eaa9bafe1 100644 --- a/webroot/rsrc/js/phuix/PHUIXAutocomplete.js +++ b/webroot/rsrc/js/phuix/PHUIXAutocomplete.js @@ -555,6 +555,13 @@ JX.install('PHUIXAutocomplete', { if (prefix) { var pattern = new RegExp(prefix); if (!trim.match(pattern)) { + // If the prefix pattern can not match the text, deactivate. (This + // check might need to be more careful if we have a more varied + // set of prefixes in the future, but for now they're all a single + // prefix character.) + if (trim.length) { + this._deactivate(); + } return; } trim = trim.replace(pattern, '');