Separate label/value for each suggestion on the list.

[vlazar]

This is an attempt to make a cleaner version of #16851

To all interested in testing it here is the link to raw version https://raw.githubusercontent.com/vlazar/awesomplete/feature/separate-label-and-value/awesomplete.js
Earlier filter(), sort(), item() and replace() were receiving list items as strings. Now they are receiving item as an object with a label and a value. But old API still works via private Suggestion() wrapper which pretends to be a String.

Known issue with current implementation: suggestion[0] won't work, everything other you expect from String should work fine.

It is now possible to show a suggestion label in completer, but insert a suggestion value into the input instead.

In addition to String as before, each list item now can also be:

• a { label, value } Object
• a [ label, value ] Array

To show full country name in completer, but insert country code into the input you can use these items:

• { label: "United States", value: "US" }
• [ "United States", "US" ]

Despite this data format change, old code will continue to work as before. This is taken care by Suggestion(). It uses label property automatically when string is expected anywhere in the API.

In addition to default support for String/Object/Array items, we also add data method, which can be used to support any additional custom item formats and to generate data dynamically, as in changed Email example. The only thing you need to do in this case is to return item in any of String/Array/Object formats supported by default.

It's also possible now to have a different label/value via <datalist> element with <option label="United States" value="US">, <option value="US">United States</option> or <option label="United States">US</option>.

[LeaVerou]

This PR is pretty much exactly what I was talking about! Thank you @vlazar!
Should I go ahead and merge?

[LeaVerou]

Better to use Object.create(String.prototype) here I think, that way the constructor is not called pointlessly. But that’s minor (and more of a general best practice).

Also, I’m not sure if this is in general a good way to subclass builtins, since builtins are special in a number of ways. E.g. I suspect (haven't tested it) that length will not work properly so we might need to define our own getter. Check this out, notably the section about __proto__.

[vlazar]

Changed to Object.create(String.prototype) and provided correct length in 61666cf
Sure, length is not live updated, but then we can also modify value and label while pretending to be just a String.

So should we care about not being exactly the same as String here?
I believe it's not fully doable in ES5.

Will we eventually remove Suggestion in 2.0 or do you want to keep it forever?

I'll check the link you provided and explore other solutions as well.

[vlazar]

Another one based on __proto__ 2471773

Browser support http://kangax.github.io/compat-table/es6/#test-Object.prototype.__proto__

[LeaVerou]

We should also probably wrap with new Suggestion() again after the chain, since people might be returning strings from the methods they are overriding.

[vlazar]

We should also probably wrap with new Suggestion() again after the chain, since people might be returning strings from the methods they are overriding.

Neither filter nor sort return suggestion items for array. The item method returns DOM element. The replace method is just for replacing input value. It doesn't look like we need to wrap with new Suggestion() somewhere else.

[vlazar]

@LeaVerou See my 2 versions of Suggestion changed based on your comments
vlazar@61666cf
vlazar@2471773 (current state)

And my comments on PR as well.

Let me know if we go with one of these implementations, but don't merge PR please yet. I'll make a clean version instead once we agree. Also we probably need to add the section about the new data method to the docs.

[vlazar]

Created LGTM label. Please add it to this PR, once it is good to merge.

[LeaVerou]

Let's go with 61666cf for now, we can always change it to use __proto__ if we run into issues.

[LeaVerou]

Apart from my comment about length being static, LGTM.

[vlazar]

Let's go with 61666cf for now, we can always change it to use __proto__ if we run into issues.

FYI, this won't work:

var sg = new Suggestion("Hello");
sg[0]; // undefined

With __proto__ everything except subclassing should work without issues. But it's IE11+.

[vlazar]

@LeaVerou If my previous comment changes your decision, please just remove LGTM label. Otherwise I'll go with cleanup, docs and tests.

[LeaVerou]

Can we do both? (we need to check if __proto__ exists before using it though, to avoid messing with other people’s feature detection)
That way, browsers that support __proto__ will get a nicely subclassed string, and browsers that don't will still get something that works for most cases.

[vlazar]

Can we do both?

Here is how it can look then:

var o = {};
o.__proto__ = Array.prototype;
var protoSupported = o instanceof Array;

function Suggestion(data) {
    var o = Array.isArray(data)
      ? { label: data[0], value: data[1] }
      : typeof data === "object" && "label" in data && "value" in data ? data : { label: data, value: data };

    var me;
    if (protoSupported) {
        me = new String(o.label);
        me.__proto__ = Suggestion.prototype;
    } else {
        me = this;
    }

    me.label = o.label;
    me.value = o.value;
    return me;
}
Suggestion.prototype = Object.create(String.prototype);
if (!protoSupported) {
    Suggestion.prototype.toString = Suggestion.prototype.valueOf = function () {
        return this.label;
    };
    Object.defineProperty(Suggestion.prototype, "length", {
        get: function() { return this.label.length; }
    });
}

Doesn't if feel like a too clumsy solution? Also in case without __prototype__ support the length property works, but access with square brackets like in strings does not work.

If we loosen your requirement for length property to be updated dynamically, we can just use:

function suggestion(data) {
    var o = Array.isArray(data)
      ? { label: data[0], value: data[1] }
      : typeof data === "object" && "label" in data && "value" in data ? data : { label: data, value: data };

    var str = new String(o.label);
    str.label = o.label;
    str.value = o.value;

    // Uncaught TypeError: Cannot redefine property: length(…)
    // Object.defineProperty(str, "length", {
    //  get: function() { return this.label.length; }
    // });

    return str;
}

This is the simplest of all solutions. It's concise and it will work in all browsers without any __proto__ magic, even the square brackets access. Provided we gave up the live length property.

We can use this for 1.x releases and in 2.x we can deprecate an old API and require explicit use of suggestion.label or suggestion.value instead of just suggestion in filter(), sort() and replace() functions.

[LeaVerou]

We can use this for 1.x releases and in 2.x we can deprecate an old API and require explicit use of suggestion.label or suggestion.value instead of just suggestion in filter(), sort() and replace() functions

@vlazar, I think we’ve discussed this before. We are not going to stop letting people use simple strings because it makes our code easier. So far Awesomplete has supported only simple strings and people do use it, meaning there is a ton of use cases where label == value. Most, I would say. Enabling people to do more complex things (like having label != value) does not mean we have to complicate everybody else's code. The possibility should be there for when they need it, but it shouldn't get in the way when they don't. Please don't see those who want label = value as this rare edge case that we don't care much about and are only supporting for a bit while they're transitioning to this new superior API. These are all our current users. Everyone who uses Awesomplete today is using it in cases where label = value. These use cases are our PRIMARY priority. It's the label != value that is an on the side thing.

I hate having a Suggestion class or function just as much as you. If we can think of something better now or in the future, that would be fantastic! But if we can't think of something better, delegating the problem to our users and complicating their code even one iota is NOT a solution. That is not how good APIs are designed.

Usability should be a priority. There are tons of other autocomplete widgets that do way more than Awesomplete. The reason people embraced Awesomplete is not because it has the most features, but because it's small, usable and extensible. Let's try to keep these traits.

Regarding the code, yes, the first one is indeed too clumsy. However, we cannot give up the length property. Not to mention that in your second example, if a user updates o.label in a function, the returned string (if they use as a string) won't be updated. Also, length matters more than square brackets usage, as it’s used way more often. We cannot give length up. I think we should go back to the original subclassing, with a dynamic length property for now.

Also, once we finalize the design, it might be a good idea to have a few people test this out in real use cases before we deploy. Maybe we should first add it as a separate branch and ping those who requested it to try it out? Just a thought. What do you think?

[vlazar]

I'll go back to the original subclassing, with a dynamic length property then.

Just to make sure we are on the same page. This is an explicit API I was talking about:

filter: function (suggestion, input) {
    return RegExp($.regExpEscape(input.trim()), "i").test(suggestion); // 1.x
    return RegExp($.regExpEscape(input.trim()), "i").test(suggestion.label); // 2.x
}

Sorry, if we've already discussed this and I've missed your point. Just wanted to explore all possible options, as I don't like already discussed ones too much. The part that bothers me most, is not the clumsiness, but that not everything will work, while we are trying to do absolutely no changes in API.

Sad, it's only an IE10 support that holds us back. Otherwise __prototype__ is a way to go without any API changes.

[vlazar]

Also, once we finalize the design, it might be a good idea to have a few people test this out in real use cases before we deploy. Maybe we should first add it as a separate branch and ping those who requested it to try it out? Just a thought. What do you think?

I was planning to make sure everything, including all the old examples and suggested features, will work before accepting this PR as LGTM by myself, but sure, we can ask others to check this out as well. Don't know how many will bother to check it though. It might have been easier with continuous npm releases.

[vlazar]

Since we are going to invite others to test this, I've also added support for list with separate label/value via <datalist>.

[LeaVerou]

Wow, that's a lot of work, thanks! People will not manually merge this PR to test the new functionality out and might not realize they can get the full file from your repo. We need to add a branch so that they can just download a file and use it, then give them a link. The less work they have to do and the less knowledge of git(hub) they need to try it out, the more people actually will try it.

Also, I'm more interested in the usability of the JS API right now. If people use it with <datalist>, it will be usable regardless of what we do with the JS API. All our disagreements/discussion have been about the JS API.

[vlazar]

There is a short summary in bold in PR description with a link to JS file.

[LeaVerou]

Oh cool, I hadn't noticed you edited the PR description :)
Thanks again for all your efforts on this!

[nixprosoft]

I'm using awesomemplete for quick navigation between huge list of organizations without any modification of your code. See how it forks for me now:

My datalist looks like this:
`

ИАЦ Elite Rentals ИП Кочикян М.М. `

Click on QuickSearch button:

Entering label and selecting result:

Label becomes value of label (showing in field):

and my script navigates to target page:
$(awesompleteted).on('awesomplete-selectcomplete', function (e) { window.location.href = '/organizations/edit/' + e.target.value; });

Can you tell me, how not to show item value in awesomplete?
P.S.Thanks for the great work!

[vlazar]

@nixprosoft You need to provide custom replace method, since the default replace just inserts item's value into the input. You can use #16774 (comment)

[marcobeierer]

Thank you both for the great work.

A short feedback on the changes: If have tested the script today in a simple use case where I work with the value when the awesomplete-selectcomplete event is fired and everything works as expected.

[nixprosoft]

@vlazar Thanks! Understood clearly!

[vlazar]

@webguerilla Thanks. You probably meant awesomplete-select event. The awesomplete-selectcomplete wasn't changed in this PR.

There was a discussion with @LeaVerou about having item's data accessible in events like awesomplete-select and awesomplete-selectcomplete. We'll get to this part eventually too and change event properties a bit. For now I think it's safe to say you can rely on event.text property in awesomplete-select.

[marcobeierer]

@vlazar I actually did use awesomplete-selectcomplete and then read and cleared the value from the input field. I had now a closer look at awesomplete-select and implemented a cleaner solution with the suggestion stored in event.text and a custom replace function. Thank you for the hint.

[vlazar]

Closing in favor of #16866