Various list data formats and separate title/value

[vlazar]

New data configuration property is function, which is called for each initial list item in any format to convert it to object like this one:

{ title: "United States", value: "US" }

This object is then used everywhere in API. So instead of item text filter(), sort(), item() and replace() functions now get an object with separate title and value properties.

Despite this data format change, old code will continue to work as before. This is what Suggestion() shim takes care of. It uses title property when string is expected everywhere in the API.

The only change is that now we have value property and it would be used in default implementation of replace() method. Suggestion title you see in the completer and the value inserted to the input, when the item is selected can now be fully independent.

[vlazar]

This is a next step for #16821 and whole key/value story.

New data() method now serves 2 purposes:

• it converts incoming list item data type, e.g. String to outcoming item data type { title: ..., value: ... } object
• it also can be used to dynamically modify item data (see simplified email example, where email is generated from input and initial domain list)

While already useful, I believe it can be improved. Lot's of PRs and issues are about support for key/value objects and arrays. With this PR you need to provide data() implementation for any list item format other than String.

We can try to separate these 2 data() responsibilities. So data() would also receive { title: ..., value: ... } instead of incoming item data as other methods. And we'll add a new data conversion method, with default converters for String, Object, Array, etc. This data conversion method can be left unchanged for most use cases.

Default data conversions will still work in that case, even if we change data() implementation just to modify title or value.

[vlazar]

Example 1. List of objects.

All you need is to provide a custom data() method in addition to list of objects itself:

new Awesomplete(urlInput, {
    data: function(item, input) {
        return { title: item.name, value: item.url };
    },
    list: [
        {
            "name": "Awesomplete",
            "url": "https://leaverou.github.io/awesomplete/"
        },
        {
            "name": "Bliss",
            "url": "https://blissfuljs.com"
        },
        {
            "name": "Stretchy",
            "url": "https://leaverou.github.io/stretchy/"
        },
        {
            "name": "chainvas",
            "url": "https://leaverou.github.com/chainvas"
        }
    ]
});

You'll see name of project in the completer and once selected, the URL of the project would be inserted into input. Of course you can just use title and value properties in the list of objects if you control data source and then data() implementation would be just an identity function:

    data: function(item, input) { return item }

[vlazar]

Example 2. List of arrays.

All you need is to provide a custom data() method in addition to list of arrays itself:

new Awesomplete(countryCodeInput, {
    data: function(item, input) {
        return { title: item[0], value: item[1] };
    },
    list: [
        [ "Belarus", "BY" ],
        [ "China", "CN" ],
        [ "United States", "US" ],
    ]
});

You'll see full name of the country in the completer and once selected, the country 2 letters code would be inserted into input.

[LeaVerou]

Thanks for working on this!!

_.DATA doesn't seem to use one of its arguments, is that intentional?

Some concerns, after briefly skimming this (so I might have missed something):

1. It seems that this allows key/value usage for people who are able to use the JS API. However, 1 in 2 people who write HTML/CSS are not comfortable with JS. Awesomplete tries to cater to those people by offering an HTML API, and I would like that to extend to key/value pairs, as they're a frequently requested feature. If users link their input to a <datalist> with <option value="a">b</option> options, they give us both the value and the title and we should use them. For more complex lists, perhaps we should get the key from whatever element has a class of key or awesomplete-key or something. Regardless of the API design choices we could make, there absolutely must be an HTML API for this, it's one of Awesomplete's design goals. I understand that this PR is just a small step in this direction, but I wanted to raise this concern early so we take it into account when making the code changes.
2. It looks like now everything is an object and the list is a list of arrays. This is nice when the key/value functionality is actually needed, but if not, the API should be the same as before and we should handle it internally. Remember, good usability is making the simple case easy and the complex one possible. Allowing for the complex case (key != value) should never get in the way of the simple one (key=value).

[vlazar]

_.DATA doesn't seem to use one of its arguments, is that intentional?
You are right. I should've just put the second argument into comment in the first place, instead of muting eslint warning. Second argument is the value of the input, same to other methods. Its 's use in the email example.

1 - Yes, getting key/value from HTML is one of requested features. To keep this PR minimal I didn't tried to implement it, but it would be trivial, once we decide to go with this one I guess.

2 - The initial list in this._list is now whatever you put into it. It can be a list of strings as before and everything just works as before, even advanced examples. It can be a list on objects, preferably in the form { title: ..., value: ... } and it will also work. Same for the list of arrays.

Can you elaborate on this please:

This is nice when the key/value functionality is actually needed, but if not, the API should be the same as before and we should handle it internally. Remember, good usability is making the simple case easy and the complex one possible. Allowing for the complex case (key != value) should never get in the way of the simple one (key=value).

I think storing this.suggestions list in form of array of { title: ..., value: ... } objects makes API very generic and very simple. The filter(), sort(), item() and replace() always gets data in the same format. This means the default implementation will work for any possible input data format. No need to override any of this methods just because of input data format change.

The Suggestion shim also makes existing code work and look the same in simple case like having a list of strings. I would prefer to go explicit in 2.0 and always refer to data.title or data.value in all of these methods. Then we can remove the Suggestion and simplify code.

[LeaVerou]

Yes, I totally didn't expect this PR to deal with the HTML API, just wanted to raise this concern early on.

About the second point, I mean that examples like these should still work:

var input = document.getElementById("myinput");
new Awesomplete(input, {
    list: ["Ada", "Java", "JavaScript", "Brainfuck", "LOLCODE", "Node.js", "Ruby on Rails"]
});

var input = document.getElementById("myinput");
var awesomplete = new Awesomplete(input);

/* ...more code... */

awesomplete.list = ["Ada", "Java", "JavaScript", "Brainfuck", "LOLCODE", "Node.js", "Ruby on Rails"];

Not just for legacy reasons, but because in these cases, it would be a huge hassle to repeat the text twice because you want to use the same text as a key and a value.
Sure, the Awesomplete code would be simpler if we didn't have to cater to these cases, but our users' convenience comes before ours.

[vlazar]

Lea, Example 1 and Example 2 are here just to show what else became possible if this PR is merged. It's the base for requested feature of using key/value or array for item data. To support this by default we can add just a little bit on top.

The default case with strings just works. It's the beauty with this PR. No breaking changes. At least I can't find one and all tests are passing. No code changed in filter(), sort() and item() methods which now work with { title: ..., value: ... } objects, but via Suggestion shim. The replace() method uses text.value directly because it's the new feature - the value and title are now separated. They can be the same, or they can be different.

[LeaVerou]

I see. Cool!

You are right. I should've just put the second argument into comment in the first place, instead of muting eslint warning.

Why not just delete it?

[vlazar]

Why not just delete it?

Deleted. I often use source as a doc and there are no JSDocs, so having commented out argument makes it obvious, that there is in fact a second argument you can use in your implementation.

[vlazar]

One more thing. I wonder if label/value pair of keys would be a better name than title/value.

[LeaVerou]

Deleted. I often use source as a doc and there are no JSDocs, so having commented out argument makes it obvious, that there is in fact a second argument you can use in your implementation.

Oh, I hadn't realized it's actually called with that argument, I thought it was just leftover. If it's an optional argument then don't remove it! Either include it as a normal argument (what I usually do) or commented out. Sorry!

Not sure about label/value, both seem equally good to me. But I like it that you devote a lot of thinking into names. Many developers don't realize how important good names are.

[vlazar]

Added deleted argument back.

Looks like everything is settled, so I also changed name of argument from text to data everywhere we have { title: ..., value: ... } object now. This will make it obvious that it's not just a text now.

The original item from this._list is item argument initially. Then we map it with data() method to { title: ..., value: ... } object and use data as argument name everywhere after that.

[vlazar]

Not sure about label/value, both seem equally good to me. But I like it that you devote a lot of thinking into names. Many developers don't realize how important good names are.

I'll look into other popular completers to see what name they use.

[vlazar]

This PR is becoming a little big.

Maybe I should open a separate PR for default data converters explained in my comment above #16851 (comment)
We can also add initialization from datalist element with <option value="a">b</option> items after that.

[LeaVerou]

The size of this PR is ok.
Why the documentation change though? If the change is non-breaking, the API should work the same way. I'd be more in favor of adding new examples instead of changing the old ones, especially in ways that are not immediately understandable or elegant. There should be no example with the new API where title and value are the same, this just paints our API in a negative light.

[vlazar]

Email example was originally tricky. It needs to be changed, because replace() was receiving text from selected LI element. Actual email address was constructed in item() method from the original list of email domains only.

In this PR the replace() method receives data from the list of suggestions in this.suggestions, not from LI element itself. Since original list is just the list of domains, replace() in unchanged example would just insert domain name like @aol.com instead of full constructed email address like [email protected].

So only this example should be changed. It's a breaking change for this particular case, but I think it's a good one. We now get data from array of suggestions, not from DOM. There were requests to make it possible to display one data and insert another in completer. Like add prefix to each item in completer but still insert data from original list. This becomes possible even with old API. Suggestions are prepared with data() and you can change how it's displayed in item().

Also, changed example is now simpler. It only needs the data generation part, which is now separate from LI item construction in item() - it's data() method responsibility. Since this.suggestions now contains real email addresses, we don't need custom filter implementation and we just use Awesomplete.FILTER_STARTSWITH.

I'd be more in favor of adding new examples instead of changing the old ones, especially in ways that are not immediately understandable or elegant.

If you mean the removal of @ sign in original list, I can revert that. It's just that we don't need them anymore in the original list.

[vlazar]

Not sure about label/value, both seem equally good to me. But I like it that you devote a lot of thinking into names. Many developers don't realize how important good names are.

I looked into some popular autocompleters. jQuery UI autocomplete widget and some others are using label, not tiltle. Also I checked <datalist> docs:

https://html.spec.whatwg.org/multipage/forms.html#the-datalist-element

Each option element that is a descendant of the datalist element, that is not disabled, and whose value is a string that isn't the empty string, represents a suggestion. Each suggestion has a value and a label.

https://html.spec.whatwg.org/multipage/forms.html#concept-option-label

The label attribute provides a label for element. The label of an option element is the value of the label content attribute, if there is one and its value is not the empty string, or, otherwise, the value of the element's text IDL attribute.
The value attribute provides a value for element. The value of an option element is the value of the value content attribute, if there is one, or, if there is not, the value of the element's text IDL attribute.

So I think we should also go with label/value instead of current title/value. What do you think?

[LeaVerou]

Totally agree! External consistency with other APIs is always good. :)
Thanks so much for looking into this and not going with the first idea we thought of! It's great to see someone else who also cares about naming :)

[vlazar]

Totally agree! External consistency with other APIs is always good. :)

In case you missed it - I've already changed title -> label :)

And what about your concern with email example change answered by me in this comment?
#16851 (comment)

[LeaVerou]

Ah sorry, I had missed it.
I agree it's a good change for the most part. However, people should never, ever have to specify both label and value if they are the same. If the Suggestion class won't handle this, we should special case it. Forcing people to duplicate content is a usability mortal sin.
Being able to just return {value: newText} in those cases would be an improvement, but being able to return newText is even better.

[vlazar]

The data method converts from input format (string, array, object, ...) to output format, which is { label: ..., value: ... } object. Simple public API. In case the label and the value needs to be the same, it's a duplication, yes. But it's an accidental duplication.

If we need to change like you described, then this public method kind of loses it's purpose. Why have data conversion method, if we can't rely on it to provide required object?

The data method now kind of serves 2 purposes: converts data and modifies data (the email example). I've described possible next steps in #16851 (comment)

I think data conversion methods should be probably public, to be able to override them for default String/Array/Object/... if we support them out of the box. Or we can make default cases unchangeable and provide additional method to plug in data conversion for whatever item format we don't support out off the box. And again, it would take data in some format and provide guaranteed { label: ..., value: ... } object. Simple contract to follow.

We can have separate data for changing data (email example generates emails from domain and input text). But with all requests like handling <option value="a">b</option> cases it looks like data conversion should happen before this data changer method is called. So it will then receive { label: ..., value: ... } object.

So we still have this object either in data conversion method or data changer method. Unless we make data conversion private and non extendable.

I can try to address your concern, but that would be a lot of changes to support defaults for String/Object/Array and initialization from HTML. This may become even harder to agree on.

[vlazar]

Being able to just return {value: newText} in those cases would be an improvement, but being able to return newText is even better.

To make {value: newText} work would be a very easy and cheap addition. But if we go with this improvement, then the newText case would be probably also expected from data generation method.

If we split type conversion and data generation methods (which I think we should do anyway)...
This is what steps would be required:

1. convert each item from initial this._list to { label, value } object
2. wrap result with Suggestion to support old string based API
3. get result from data() (data generation method)
4. convert result to { label, value } object once again
5. wrap result with Suggestion to support old string based API

So we convert data twice and wrap it with Suggestion twice. I feel like we are trying to optimize to much for the minor use case (dynamic data generation like in email example). Data conversion can just work by default and not require custom data method anyway.

[vlazar]

Being able to just return {value: newText} in those cases would be an improvement, but being able to return newText is even better.

Possible way to remove content duplication (please also my comments above): bb9ce87

To me it looks like too much hassle just to optimize for this minor use case. We convert items twice, we wrap with Suggestion twice. Of course, we can make data optional and insert some if statements, but it's already far from elegant.

What do you think?

[vlazar]

We only need to wrap with Suggestion here for old string based API inside data method.

[LeaVerou]

I see. No, this new private convert() method seems like a much more inelegant solution. I kinda feel bad shooting down proposals you put time in, maybe we can discuss solutions first before you get to coding? Unless you don't mind.

Idea (might be a bad one, I’m just brainstorming here): What if the output of data() was passed to the Suggestion class constructor regardless of return value? The constructor could accept either an object or a string and then it could take care of having the proper label and value properties. Then it could be used instead of the {label, value} object everywhere, and it would be converted to a string when used as a string as it does now and work like an object otherwise. Internally, we would never use it as a string, but third party developers using the API can use it as whatever they want. Caveat is of course that then we have to expose it and can't remove it. But we design it well, we won't want to remove it. :)

Again, sorry for being a prick here, I imagine it must be pretty frustrating to put time and effort on stuff and have it take ages to be merged. I'm just trying to do what's best for the project, which is not always obvious (even to me) and often only emerges in discussion.

[vlazar]

I see. No, this new private convert() method seems like a much more inelegant solution.

We need something for data conversion. I suggested initially to split data conversion / data generation. Doesn't matter if it's a method or part of Suggestion class constructor. But if it's a private thing, we can't change it. That's the problem.

I kinda feel bad shooting down proposals you put time in, maybe we can discuss solutions first before you get to coding? Unless you don't mind.

No worries. Maybe it's even easier to discuss when you at least see the code.

I agree, that having the same API everywhere with Suggestion({ label, value }) would be cool. However I visioned this Suggestion wrapper to be removed in 2.0 and same API everywhere became just { label, value }.

Again, sorry for being a prick here, I imagine it must be pretty frustrating to put time and effort on stuff and have it take ages to be merged. I'm just trying to do what's best for the project, which is not always obvious (even to me) and often only emerges in discussion.

No worries Lea, I just feeling like maybe I'm out of ideas to support all your cases in a reasonable way. I'll try to explain.

What if the output of data() was passed to the Suggestion class constructor regardless of return value? The constructor could accept either an object or a string and then it could take care of having the proper label and value properties.

We have almost the same code now with convert method:

var data = me.data(suggestion, value);
return new Suggestion(convert(data));

If I understood you correctly, the only change would be that we move current convert logic to Suggestion constructor.

Then it could be used instead of the {label, value} object everywhere, and it would be converted to a string when used as a string as it does now and work like an object otherwise. Internally, we would never use it as a string, but third party developers using the API can use it as whatever they want.

With my latest commit this already works.

Caveat is of course that then we have to expose it and can't remove it. But we design it well, we won't want to remove it. :)

Why do we need to expose it? Isn't it better if it's hidden and users of API don't care and don't even know we have some wrapper at all?

I've planed to split and expose a data conversion logic itself as public API in next PR if this PR merged. One of my sketches was something like:

_.CONVERT = {
    // for Strings and as a default case
    String: function(data) {
        return { label: data, value: data };
    },
    Object: function(data) {
        return data;
    },
    Array: function(data) {
        return { label: data[0], value: data[1] };
    }
};

// then, given that item is initial item from list in whatever data format
var convert = _.CONVERT[Array.isArray(item) ? "Array" : typeof item === "object" ? "Object" : "String"];

// and then just call
convert(item);

// to support new data format like Date we can then just add:
Awesomplete.CONVERT.Date = function(data) {
    return { label: data.toString(), value: data.valueOf() };
}

Then the original data method can receive always the {label,value} object (possibly wrapped into Suggestion).

[vlazar]

The thing that adds inelegance is the fact that data method, if we use it for data generation only, needs to be called in evaluate() since it needs this.input.value for email example and other similar use cases.

So since we need same API we have to convert data to { label, value } object before that and wrap it with Suggestion if we want old string based API.

And if this custom data method was provided we have to do the same after we call it to make sure we have { label, value } object after the call and wrap it with Suggestion if we want old string based API after that call.

[vlazar]

BTW, since we need to support the <option value="a">b</option> as list initialization, the outcome of this is that at least in this case this._list would be a { label, value } object. In other cases it can be Array of strings or other objects or arrays. I tried it and it feels messy. So looks like it's better if we move data conversion logic to list setter and warp with Suggestion in here. Maybe we should retire this._list and have just this.suggestions, as it's already public.

The good news is it's not a lot of code to add this support. Maybe we need to deal with the whole data conversion before, but maybe we can add it too, since it already tells that it's better to move the first data conversion to list setter.

[LeaVerou]

I think we should keep the Suggestion class and expose it.
It's not just a hack for backwards compatibility, but also provides a lot of shortcuts for our API and allows for elegant short functions for filter, replace etc. It's important to be able to have elegant short functions for those, because they might be provided by our users.
If someone doesn't care about separate key/value pairs, they should be able to work with the API as if key/value pairs didn't exist. Otherwise the complexity of the API is getting in the way.
The convert() method should be in the Suggestion constructor. There is no reason for a separate private method, is there? That way we don't have to do ugly things like new Suggestion(convert(foo)).

Thoughts?

[vlazar]

It's not just a hack for backwards compatibility, but also provides a lot of shortcuts for our API and allows for elegant short functions for filter, replace etc. It's important to be able to have elegant short functions for those, because they might be provided by our users.

Yes, it allows elegant short functions for filter, replace etc. even now when we don't expose it. Do you mean the user of API will provide data method, which will receive data in original format, then wrap with (possibly extended version of Suggestion) manually? Will this be any better than dealing with it ourselves internally?

There is no reason for a separate private method, is there? That way we don't have to do ugly things like new Suggestion(convert(foo)).

I'll remove new Suggestion(convert(foo)) and do some other changes to see if this can be better.

[vlazar]

No more ugly things like new Suggestion(convert(foo)) :)

[vlazar]

One more idea.

We can make data to receive original items. Without conversion to { label, value } object and wrapping to Suggestion. The result of data method would be then converted with default converters and wrapped with Suggestion.

Our default conversion will handle Array/String/Object cases, so data method have to deal with original data format, but it can return any of Array/String/Object. So in email example, its data would be a string, the output would be a string too. And if you need a separate label/value, you can do it too. Just return a { label, value } object instead of string.

So data method responsibility would be to change data, and also to convert data to one of Array/String/Object default conversion cases. This way there is no need to expose _.CONVERT with methods as I suggested before, or expose Suggestion class like you are suggesting.

[vlazar]

Lea, can you please read the recent email notifications for this PR?

I tried to make more readable comments about 2 latest commits and lost the big one 😠.

The comment about the last commit where data method receives now original item data (string in email example) and 2 examples of using Date objects for date completer.

First commit vlazar@fb48e7c
Second commit vlazar@a732234

[vlazar]

Last idea as a separate and as minimal as possible PR #16852

[LeaVerou]

I’m a bit confused with all the comments and PRs so I might have missed something.

Why do we still need a convert method? I thought we would just put it in the Suggestion constructor.
We can wrap things with Suggestion multiple times, as long as the constructor accepts Suggestion objects too (which seems to the the case). It's a common pattern (e.g. jQuery objects are re-wrapped tons of times in their lifetime).

If you're not still using a convert method, ignore the comment above. I saw it in one of the commits but not in the PR and I'm not sure if you're proposing a union of these changes or not.

[vlazar]

First commit - with convert method it was cleaner to wrap in multiple places with both convert(smth) or smth.map(convert). With just Suggestion it would have been smh.map(i) { return new Suggestion(i) }.

Second commit - I've moved data conversion to Suggestion constructor as you suggested.

Finally, I've created a separate PR #16852 where I don't use multiple wrapping at all. Do you like it that way? The data method in email example is the way you wanted it. It converts data from original format to one of the supported by Suggestion Array/String/Object types. All other examples work as before. All tests are passing.

I haven't changed the argument names yet for PR to be easier to review. And the Object case in Suggestion constructor should be more specific, since we are looking for Object with label/value keys, otherwise we should skip to the most generic String case.

[vlazar]

Closing in favor of #16866