fix(VAutocomplete/VSelect/VCombobox): compare items using item value if passed

[vexleet]

fixes #16279

In order for the itemValue compare to work with objects it should also have returnObject to true. We need that because useItems returns the whole object rather than the key and when we are comparing we need to check if the returned selectedItem from useItems has the key from itemValue and if it matches with anything from passed items

I tested this creating unit tests and also using playground. I would also like to add unit tests when I got more time for this thing such as ones without having return-object and not using multiple too, and maybe some more.

<script setup>
import { ref } from 'vue'

const data = ref([
  {
    email: "[email protected]",
    id: 1,
    firstname: "John"
  },
  {
    email: "[email protected]",
    id: 2,
    firstname: "Marie"
  },
]);

const selectedData = ref([
  {
    email: "[email protected]",
    id: 1
  }
]);

</script>

<template>
  <v-app>
    <v-main>
      <v-autocomplete
        v-model="selectedData"
        :items="data"
        chips
        label="Associated user"
        item-title="email"
        item-value="id"
        multiple
        return-object
      >
      </v-autocomplete>
      <v-select
        v-model="selectedData"
        :items="data"
        chips
        label="Associated user"
        item-title="email"
        item-value="id"
        multiple
        return-object
      >
      </v-select>
      <v-combobox
        v-model="selectedData"
        :items="data"
        chips
        label="Associated user"
        item-title="email"
        item-value="id"
        multiple
        return-object
      >
      </v-combobox>
    </v-main>
  </v-app>
</template>

[ThomasCarbon]

Any update on the futur of this ? :'(

[yuwu9145]

I wonder probably we should start use a primitive-only "internalModelValue" that holds item-value when using return-object. This would make value comparison easier.

[quatrochan]

This looks like a very important feature but PR is stale. Any updates on the future of this?

[johnleider]

This looks like a very important feature but PR is stale. Any updates on the future of this?

We are waiting on the author to address feedback.

[vexleet]

Sorry for being absent, have a lot of personal and work stuff related. I will look into the comments and everything today after work.

[vexleet]

I saw that transformItem already does what I have suggested here and is the title property inside item so I guess it makes sense to use just that and all tests pass like that.

[vexleet]

@johnleider the test failing now is that since returnObject is looking for the value inside the objects it finds Item 3 and makes it the selected one rather than creating a new one. Should the test be updated to something like this:

cy.get('input').type('item3')
cy.should(() => {
  expect(model.value).to.equal('item3')
  expect(search.value).to.equal('item3')
})
cy.get('input')
  .should('have.value', 'item3')
  .blur()
cy.get('.v-combobox__selection')
-  .should('contain', 'item3')
+  .should('contain', 'Item 3')

// same test as what was before but writing item5 rather than item3

[johnleider]

What is the reason for adding the fallback? That's what the || v is for.

[vexleet]

What is the reason for adding the fallback? That's what the || v is for.

@johnleider the reason is because when we have items without value property like the example below (taken from the first unit test in VSelect.spec thats failing without fallback value), it doesnt really work since getPropertyFromItem(item.raw, props.itemValue) return undefined for both item and v, and valueComparator returns it as true so it doesnt really go to || v. I added the fallback because I saw it was made like that in vuetify 2 and also tested it with the below example too and there it was working.

Not sure how to proceed now - should we keep the fallback value or add a check in getPropertyFromItem if something is undefined to return false or do something else?

Example:

<script setup>
import { ref } from 'vue'

const items = [
  { title: 'a' },
  { title: 'b' },
  { title: 'c' },
]
let model = ref([{ title: 'b' }])

</script>

<template>
  <v-app>
    <v-main>
      <VSelect
        v-model="model"
        :items="items"
        return-object
        multiple
      />
    </v-main>
  </v-app>
</template>

[johnleider]

I think that the fallback should be removed.

[oruman]

Changes break work with modelValue, when modelValue is not Object. Before changes code below work fine, but now vModel MUST BE Object. Example:

<script setup>
import { ref } from 'vue'

const items = [
  { id: '1', title: 'a' },
  { id, '2', title: 'b' },
  { id: '3', title: 'c' },
]
let model = ref(2)

const comporator = (a, b) => {
  if (a === b) return true
  const c = (a ?? "").toString()
  const d = (b ?? "").toString()
  return c === d
}
</script>

<template>
  <v-app>
    <v-main>
      <VSelect
        v-model="model"
        :items="items"
        item-value="id"
        :value-comparator="comporator"
      />
    </v-main>
  </v-app>
</template>

[johnleider]

Changes break work with modelValue, when modelValue is not Object. Before changes code below work fine...

Create a new issue please.

[oruman]

Changes break work with modelValue, when modelValue is not Object. Before changes code below work fine...

Create a new issue please.

Done: https://github.com/vuetifyjs/vuetify/issues/17997