Detect pending text insertion when inserting from hint list #5228
Conversation
| } else { | ||
| // ...resulting text doesn't match anything in list, so | ||
| // let the event bubble. | ||
| return false; |
There was a problem hiding this comment.
This is the case described in #5003 -- resulting text after pending text insertion does not exist in hint list.
ghost
assigned 
| }; | ||
|
|
||
| /** | ||
| * Rebuilds the list items for the hint list. |
There was a problem hiding this comment.
Incorrect description. You forgot to update after you duplicated it from the function below this one.
|
@RaymondLim I pushed changes for all of your comments so far. I also found and fixed a typo that I copied and pasted in several places :) I am still not sure about the question I asked about |
| hintList.some(function (listItem, index) { | ||
| if (listItem[0].innerText.indexOf(itemText) === 0) { | ||
| found = true; | ||
| return true; |
There was a problem hiding this comment.
I think you want to return 'found' here.
There was a problem hiding this comment.
Need to return true; here to exit some() method when item is found, otherwise code loops through all elements. Code could be more explicit and return false; in else case.
Value of found gets returned from outer function.
Actually, I'm hoping that we decide to implement a simpler fix and this function will get removed, anyway.
|
Pushed changed for a much simpler fix. See comment in CodeHintList.js line 409 for details. This simplified fix means that there are no changes to HintProvider code, but I did leave in some cleanup I did to the comments -- let me know if I should remove those. Ready for another review. |
| @@ -165,8 +167,7 @@ define(function (require, exports, module) { | |||
| return { | |||
| hints: formattedHints, | |||
| match: null, // the CodeHintManager should not format the results | |||
| selectInitial: true, | |||
| handleWideResults: hints.handleWideResults | |||
There was a problem hiding this comment.
Oops... you're removing something that was intentionally added to widen the code hints list. And I think you should also put back your documentation changes (in CodeHintmanger.js) for this extra property even though it is only used in JS code hints.
There was a problem hiding this comment.
Good catch! Fixed.
|
Done with the review. Just have one comment regarding the handleWideResults property. |
|
I restored handleWideResults documentation and code. Ready for re-review. |
|
Looks good. Merging. |
Detect pending text insertion when inserting from hint list
This is for #5003.
CodeHintList now keeps track of pending text insertions so it can make a better decision about when to insert when it receives an
Enterkey.