Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added replacing feature #27

Closed
wants to merge 2 commits into from
Closed

Added replacing feature #27

wants to merge 2 commits into from

Conversation

becorey
Copy link

@becorey becorey commented Apr 4, 2014

You can specify in the helper if you would like autocomplete to replace only the current word (original behavior), or replace the full input box. I needed this use case and think it would be useful.

@mizzao
Copy link
Collaborator

mizzao commented Apr 4, 2014

Can you elaborate a bit more on why you would like it to replace the whole input box?

@becorey
Copy link
Author

becorey commented Apr 4, 2014

I'd like an form to accept any value, as well as suggest from a standard list of options.
I'm making an order form and there are about 120 standard options. Doing an autocomplete popup like this is much cleaner than a select.

It was not working as desired before because it would only replace the current word being typed. For example if you did HOT WATER RETURN, then pressed backspace a few times, and selected HOT WATER SUPPLY, the input box would contain HOT WATER HOT WATER SUPPLY, which was not what the user meant.
image

I know it is a different use case from your original intent, with tagging @ people and # issues, which is why I added it as an option in the helper.

@mizzao
Copy link
Collaborator

mizzao commented Apr 4, 2014

I see.

A lot of people have actually suggested random options for autocomplete given how it is configured right now. I'm trying to control things a bit and avoid feature creep :)

However, I see your request as part of a principled way to achieve what was requested in #4. Namely, when doing a single-field autocompletion without the use of symbols, the natural replacement of the text is the whole field. The problem is that the replacement logic doesn't think of spaces as part of the match right now, even though the regexp logic appears to act correctly - otherwise it would replace the right text even without having to have the hacky option of replacing the whole field.

My intuition would be to try and fix this function: https://github.com/mizzao/meteor-autocomplete/blob/master/autocomplete-client.coffee#L217, which was basically copy-pasted from jquery-sew. Would you be up for making a simple demo app with the words you are trying to use there, so I can use it to debug this? Basically, just copy some of those documents into a collection and use the same template that you have there now.

@becorey
Copy link
Author

becorey commented Apr 4, 2014

You could use if token == '' instead of the replacing: 'full' option I added.
When there is no token, how could you determine what was the "right text" to replace? With a token like @ you can scan backwards over spaces until you hit @ , but with token='' there is no distinction of different space separated words. Maybe you could do a similarity match with items in the collection, scan backwards as long as you have high similarity with some item in the collection.

@mizzao
Copy link
Collaborator

mizzao commented Apr 4, 2014

I actually don't use the token = '' behavior right now. Does it match backward until the first space, or until the beginning of the field? I thought it was the latter.

I think the no token replacement should always match the entire field, and so the field always just matches some entry. I don't envision this being a tagging package where multiple things can be selected - that needs to be able to render DOM and can be someone else's job :)

Another way you can just fix this is to use the part number instead of those words with spaces, which seem to be not unique and hard to post-process anyway. Or use the callback that someone else sent in a PR.

@becorey
Copy link
Author

becorey commented Apr 4, 2014

Here is a demo app: https://github.com/becorey/meteor-autocomplete-demo
I agree it would be good to match the entire field when token==''

@mizzao
Copy link
Collaborator

mizzao commented Apr 23, 2014

I've implemented what we discussed and also pulled your demo code into the example app at http://autocomplete.meteor.com/.

It's not perfect but it's a big improvement on not supporting this type of behavior before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants