vmware-clarity / ng-clarity

Clarity Angular is a scalable, accessible, customizable, open-source design system built for Angular.
https://clarity.design
Other
335 stars 72 forks source link

Combobox does not recognize already contained custom objects #278

Open TechScar opened 2 years ago

TechScar commented 2 years ago

To be completely honest, this can be both a bug report or feature request, but since it initially caused a bug in our use case, I have decided to file it as such.

Describe the bug

The Clarity Combobox offers a toggle-like feature in the dropdown for multiselect usage when used with a fixed input of available optionItems. In order to decide whether an option is already present in the input ("selected"), the code checks for its presence in the array via Array.prototype.includes(). This works fine for basic types, however does not work as intended with custom objects, UNLESS the objects in the selection are the same instances as the ones in the optionItems.

Elaboration on how includes() treats objects:

Note: Technically speaking, includes() uses the sameValueZero algorithm to determine whether the given element is found. (Source)

How to reproduce

Stackblitz example: https://stackblitz.com/edit/combobox-with-custom-objects?file=src%2Fapp%2Fapp.component.html

Steps to reproduce the behavior:

  1. Click on the combobox dropdown button
  2. See that none of the pre-selected items show as selected in the dropdown
  3. Try adding any of item2, item3 or item6
  4. See that instead of the already present item disappearing, the item is added again
  5. See that deselecting any of the new items by clicking on it in the dropdown again does not affect the previously already existent entries.

Expected behavior

There is more than one possible solution for this:

  1. Change nothing and expect the user to fill the FormControl with the same instances present in the available optionItems (not very user-friendly imho)
  2. Instead of using .includes(), allow the user to supply a custom equals function, or use an internal deep equals operation
  3. Allow the user to supply a "key", akin to the "field" property, by which the items are compared for equality (personally most preferred)

Versions

Clarity version:

Framework:

Framework version: Angular 12+

Device:

Jinnie commented 2 years ago

Thanks for the detailed issue, @TechScar

We do have such a function in our API but we don't use it for this comparison. It's the trackBy function of the *clrOptionItems structural directive.

It's not directly visible from the Model (and it probably shouldn't be) but I will try to see if we can use it some way.

Jinnie commented 2 years ago

I am still working on taking trackby function into account, when comparing items, as I believe this is a right direction. But I can suggest a workaround, which looks viable to me, in the meantime.

https://stackblitz.com/edit/combobox-with-custom-objects-workaround

The idea is to use the combobox the way we use <select> element. The value on the options resolves the unique id, not the whole element. Then you only need to use a resolver function for the selection template, which is used for the pill. No need to overwrite the selection objects and can still replace the selection array with new, simple type id values.

The key moments are using [clrValue]="item.id" on the option and the getById(selected) resolver function in the selection template.

Jinnie commented 2 years ago

There is one aspect we underestimated during triage. The trackby function allows creating implementations that may use the index as parameter for comparison. While using the trackby works fine when only the item is used, we can't guarantee that there will not be a track-by-index implementation. And this plays very badly with our scenario where we're comparing a selection subset against the full data set. It may be possible to fix or provide alternative solution, but this extends beyond our current prioritization scope for this issue. So far, our advice is to use the workaround provided in the comment above.