Skip to content

Commit

Permalink
Disable autosuggest
Browse files Browse the repository at this point in the history
Summary:
There is currently a regression bug with autosuggest that will not get fixed in the next few weeks,
so we've decided to disable it entirely.
I saw a few different places I could disable it -- I could also do it in live-editor.js,
tooltipEngine.setEnabledStatus. That code confuses me since setEnabledStatus actually only
sets the enabled status of autosuggest, from what I can tell, but it might be the better place
to disable it. What do you think?

Test Plan:
- Create new program at /cs/new/pjs. Type ba, don't see autosuggest.
 Type rect, see documentation tooltip popup. Complete rect command, be able to use number scrubbers.
 Type fill, use color picker successfully.

Reviewers: kevinb

Reviewed By: kevinb

Subscribers: #cs

Differential Revision: https://phabricator.khanacademy.org/D23704
  • Loading branch information
pamelafox committed Dec 1, 2015
1 parent c2d84d5 commit 4e028ca
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
5 changes: 4 additions & 1 deletion build/js/live-editor.tooltips.js
Expand Up @@ -1432,7 +1432,10 @@ window.TooltipEngine = Backbone.View.extend({

this.requestTooltipDefaultCallback = (function () {
//Fallback to hiding
ScratchpadAutosuggest.enableLiveCompletion(this.enabled);
// We are disabling autosuggest for now until issue #408 is fixed
// We may also consider doing A/B tests with partial lists of
// commands in the autocomplete to new programmers
ScratchpadAutosuggest.enableLiveCompletion(false);
if (this.currentTooltip && this.currentTooltip.$el) {
this.currentTooltip.$el.hide();
this.currentTooltip = undefined;
Expand Down
5 changes: 4 additions & 1 deletion js/ui/tooltip-engine.js
Expand Up @@ -111,7 +111,10 @@ window.TooltipEngine = Backbone.View.extend({


this.requestTooltipDefaultCallback = function() { //Fallback to hiding
ScratchpadAutosuggest.enableLiveCompletion(this.enabled);
// We are disabling autosuggest for now until issue #408 is fixed
// We may also consider doing A/B tests with partial lists of
// commands in the autocomplete to new programmers
ScratchpadAutosuggest.enableLiveCompletion(false);
if (this.currentTooltip && this.currentTooltip.$el) {
this.currentTooltip.$el.hide();
this.currentTooltip = undefined;
Expand Down

2 comments on commit 4e028ca

@bytorbrynden
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If autosuggest is disabled, shouldn't these tests be disabled for now?

@kevinbarabash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly those tests passed: https://travis-ci.org/Khan/live-editor/builds/94219850. Not sure what's going on.

Please sign in to comment.