Fixes #1381 - Improvements clay multi select

[matuzalemsteles]

No description provided.

[matuzalemsteles]

hey @jbalsas, I do not know if I understood your intention of the filterField, but I'm using it for the context of setting the field when the items are filtered.

https://github.com/liferay/clay/pull/1382/files#diff-21f70f10741963ea56df2ba593022af3R64

[jbalsas]

Hey @matuzalemsteles, yeah, I think this does what's expected, but the initial value is odd. It should likely be value. I'd expect the itemSelected event from AutoComplete to emit the full item that was selected. This should be more than just a string, it should be the whole object. For example:

const items = [{
    foo: 1,
    bar: 2
}, {
    foo: 3,
    bar: 4
}];

You'd then set filterField = 'foo', and the value to add should then be 1 or 3 depending on which item got selected.

Additionally, we should have a counterpart for the label. We can't simply derive it from the value, as these things might not always have a clear mapping. Imagine the following:

const categories = [{
    categoryId: 10032,
    name: 'Food',
}, {
    categoryId: 203214,
    name: 'Animals',
}];

In this case, you'd likely want to set the field value to map to the categoryId field, and the label one to map the name field.

With this in mind, I'd propose something like:

const categories = [{
    categoryId: 10032,
    name: 'Food',
}, {
    categoryId: 203214,
    name: 'Animals',
}];

new MultiSelect({
    [...]
    // Both could be string|function
    labelLocator: 'name', // could be (item) => item.name
    valueLocator: 'categoryId', // could be (item) => item.categoryId
    [...]
});

[matuzalemsteles]

Thanks @jbalsas for the clarifications, I'll make the modifications.

[matuzalemsteles]

hey @jbalsas just clarifying something related to the filterField API, we are only using to get the field of the filtered item (See https://github.com/liferay/clay/blob/master/packages/clay-data-provider/src/ClayDataProvider.js#L152). I'm assuming a dev can make its own filter system and disable the default.

[matuzalemsteles]

On the use of labelLocator and valueLocator of accepting functions is a bad thing because we need to pass to Soy and it does not deal well with this. I like the idea of ​​accepting only string, what do you think?

[jbalsas]

Got it, let's stick with strings for now, then 👌

[matuzalemsteles]

hey @jbalsas, yesterday I was looking at this and I did the following to be able to accept the string and function, I kept your idea and I'm just not taking into consideration to modify the selectedItems API schema, that way I just do the mapping of labelLocator and valueLocator... What do you think?

[jbalsas]

What did you do? 😂

[matuzalemsteles]

haha 😅, I'm going to send what I had done, it would have been clearer.

[jbalsas]

😂 👍

[jbalsas]

I think this looks much better, @matuzalemsteles. I'll merge ahead so we can put it through some extensive testing directly on the use case.

One thing I noticed is that we create one input tag per label or selected value, rather than just one. That should work as a default Array on the server implementation, so it might be great, but we'll need to double-check.

Thanks @matuzalemsteles ! 👌