Search and Top Navigation
#9771 closed bug (fixed)
Opened January 25, 2014 08:33PM UTC
Closed January 28, 2014 05:38PM UTC
Autocomplete: The input value should not be set when moving past the beginning or end or a menu for contenteditable
| Reported by: | Yermo2 | Owned by: | Yermo2 |
|---|---|---|---|
| Priority: | minor | Milestone: | 1.11.0 |
| Component: | ui.autocomplete | Version: | 1.10.4 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
I am building a rich text editor that features facebook like in-line tags and @mentions. https://github.com/Yermo/rich_textarea
When the focus: event is cancelled (event.preventDefault()) the documentation says the input value should not be replaced when menu items are focused.
This works correctly until you go past the end or beginning of the menu, at which point it incorrectly resets the value. (Chrome 31.0.1650.63 under Ubuntu Linux)
See this jsfiddle: http://jsfiddle.net/2256A/
To reproduce, type the word cold in the editable div and move the arrow key down past the end of the menu. You will see the cursor jump to the left.
The correct behavior should be that the cursor does not move.
The offending line is in jquery.ui.autocomplete.js in the _move: method at line 537.
Unfortunately, the editable div does not get a focus event so there doesn't seem to be a way to intercept this.
I suggest firing the focus() event before calling this._value() as in:
if ( this.menu.isFirstItem() && /^previous/.test( direction ) ||
this.menu.isLastItem() && /^next/.test( direction ) ) {
if ( false !== this._trigger( "focus", event, { item: null } ) ) {
this._value( this.term );
}
this.menu.blur();
return;
}
Attachments (0)
Change History (13)
Changed January 25, 2014 08:38PM UTC by comment:1
| owner: | → Yermo2 |
|---|---|
| status: | new → pending |
Changed January 25, 2014 08:44PM UTC by comment:2
| status: | pending → new |
|---|
That's a good point.
According to the autocomplete docs:
''UP - Move focus to the previous item. If on first item, move focus to the input. If on the input, move focus to last item.''
However, in my testing, the focus event is not fired on the input, at least in the case of a contenteditable div. Can we fire it explicitly with the input element as in:
if ( false !== this._trigger( "focus", event, { item: this.element } ) ) {
this._value( this.term );
}
Changed January 25, 2014 08:50PM UTC by comment:3
| status: | new → pending |
|---|
What if we just don't set the value when wrapping for multi-line inputs? That seems reasonable.
Changed January 25, 2014 08:53PM UTC by comment:4
| status: | pending → new |
|---|
I may be missing something, but one might want to to do multiple in-line autocompletes in a single input field, no? Imagine a comma separated list in a simple input field.
Also the documentation says the input gets focus when you go past the end of the menu, but the focus event is never fired on the input element.
Changed January 25, 2014 08:54PM UTC by comment:5
What is the downside of firing a focus event on the input in this case?
Changed January 25, 2014 09:20PM UTC by comment:6
| status: | new → pending |
|---|
I may be missing something, but one might want to to do multiple in-line autocompletes in a single input field, no?
Yes, but what is the problem you're trying to solve? Our multiple value demo seems to work just fine.
Also the documentation says the input gets focus when you go past the end of the menu, but the focus event is never fired on the input element.
Correct. DOM focus and autocomplete focus are not the same thing. The documentation for the focus event specifically says "Triggered when focus is moved to an item (not selecting)."
What is the downside of firing a focus event on the input in this case?
It's inconsistent with what the focus event is used for, which is to handle an actual item, with associated data, being focused.
Changed January 25, 2014 09:37PM UTC by comment:7
| status: | pending → new |
|---|
My apology. I misunderstood what you meant by multi-line inputs.
It isn't immediately clear to me how to determine whether or not we are doing a "multi-line input" in autocomplete.js. But yes, for my purposes, I just need it to not change the value when it goes past the last or before the first menu item.
Since cancelling the focus event will not work for the case where you've gone past the end of the menu, we may want to consider adding an explicit option for the "replace text" vs "don't replace text" case.
Changed January 25, 2014 09:48PM UTC by comment:8
It isn't immediately clear to me how to determine whether or not we are doing a "multi-line input" in autocomplete.js.
Multi-line is defined to be textareas and contenteditable elements. They get special treatment so that you can actually move between lines without triggering the autocomplete menu.
we may want to consider adding an explicit option for the "replace text" vs "don't replace text" case.
I'd rather not add an option for something that has been requested once in the past four years.
Changed January 25, 2014 09:52PM UTC by comment:9
Ah, I understand now.
So you are suggesting to simply wrap the call to this._value( this.term ); with an if that determines whether or not it's a multi-line element. Yes, that would work perfectly.
Is there a single property to determine whether it's multi-line or do we need to do a test for contenteditable div or textarea explicitly?
Changed January 25, 2014 09:58PM UTC by comment:10
Scratch that. Answered my own question. I see the property in the code.
Changed January 25, 2014 10:50PM UTC by comment:11
Changed January 28, 2014 05:37PM UTC by comment:12
| milestone: | none → 1.11.0 |
|---|---|
| status: | new → open |
| summary: | autocomplete sets input/editable div value on end/beginning of menu when focus event cancelled. → Autocomplete: The input value should not be set when moving past the beginning or end or a menu for contenteditable |
Changed January 28, 2014 05:38PM UTC by comment:13
| resolution: | → fixed |
|---|---|
| status: | open → closed |
Autocomplete: Do not set value on multi-line input
This fixes an issue where contenteditable text was getting overwritten when
wrapping past the bottom or top of the autocomplete menu.
Fixes #9771
Closes gh-1184
Changeset: 605a20ef06b0bae2d2ffd8d96e49c2a297add80a
It doesn't really make sense to trigger a
focusevent, since there isn't an item that is being focused. Do you have an alternate proposal?