Opened 3 years ago

Closed 3 years ago

#9771 closed bug (fixed)

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;
   }

Change History (13)

comment:1 Changed 3 years ago by Scott González

Owner: set to Yermo2
Status: newpending

It doesn't really make sense to trigger a focus event, since there isn't an item that is being focused. Do you have an alternate proposal?

comment:2 Changed 3 years ago by Yermo2

Status: pendingnew

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 );
      }

comment:3 Changed 3 years ago by Scott González

Status: newpending

What if we just don't set the value when wrapping for multi-line inputs? That seems reasonable.

comment:4 Changed 3 years ago by Yermo2

Status: pendingnew

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.

comment:5 Changed 3 years ago by Yermo2

What is the downside of firing a focus event on the input in this case?

comment:6 Changed 3 years ago by Scott González

Status: newpending

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.

comment:7 Changed 3 years ago by Yermo2

Status: pendingnew

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.

comment:8 Changed 3 years ago by Scott González

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.

comment:9 Changed 3 years ago by Yermo2

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?

comment:10 Changed 3 years ago by Yermo2

Scratch that. Answered my own question. I see the property in the code.

comment:12 Changed 3 years ago by Scott González

Milestone: none1.11.0
Status: newopen
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

comment:13 Changed 3 years ago by Yermo

Resolution: fixed
Status: openclosed

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

Note: See TracTickets for help on using tickets.