Skip to content

Conversation

jcjp
Copy link
Contributor

@jcjp jcjp commented Sep 6, 2025

Description

  • Check the model value if it's an empty string and treat it as a not dirty value
  • In this way we could show the placeholder from props

fixes #21972

Markup:

<template>
  <v-app>
    <v-container>
      <v-combobox
        v-model="d"
        placeholder="placeholder"
        persistent-placeholder
        :items="['a','b','c']"
      />
    </v-container>
  </v-app>
</template>

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

  const d = ref('')
</script>

@jcjp jcjp requested a review from a team September 6, 2025 15:32
@jcjp jcjp self-assigned this Sep 6, 2025
@jcjp jcjp added T: bug Functionality that does not work as intended/expected C: VCombobox labels Sep 6, 2025
@jcjp jcjp marked this pull request as ready for review September 6, 2025 15:34
J-Sek
J-Sek previously requested changes Sep 7, 2025
@KaelWD KaelWD force-pushed the master branch 4 times, most recently from 142b234 to bb54746 Compare September 12, 2025 09:01
@jcjp jcjp marked this pull request as draft September 12, 2025 10:15
@jcjp jcjp force-pushed the fix(VCombobox#21972)/placeholder-not-shown-on-initial-empty-string branch from 3c9b2e3 to ebdc1c8 Compare September 13, 2025 16:57
@jcjp jcjp changed the title fix: set is dirty false if empty string to show the placeholder (#21972) fix: vcombobox placeholder not shown on empty string value (#21972) Sep 13, 2025
@jcjp jcjp force-pushed the fix(VCombobox#21972)/placeholder-not-shown-on-initial-empty-string branch from 8ed72c6 to a19aa97 Compare September 13, 2025 18:07
@jcjp jcjp force-pushed the fix(VCombobox#21972)/placeholder-not-shown-on-initial-empty-string branch from a19aa97 to e62bf0b Compare September 13, 2025 18:11
@jcjp jcjp marked this pull request as ready for review September 13, 2025 18:15
@jcjp jcjp dismissed J-Sek’s stale review September 13, 2025 18:15

Done with the update as per your review Jacek let me know if you still have review points thanks!

@jcjp jcjp requested a review from J-Sek September 13, 2025 18:16
@J-Sek J-Sek changed the title fix: vcombobox placeholder not shown on empty string value (#21972) fix(VCombobox): placeholder visibility for empty string value Sep 13, 2025
J-Sek
J-Sek previously approved these changes Sep 13, 2025
Copy link
Contributor

@J-Sek J-Sek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Green light from me, but I wish others could have a day or two to voice their opinions. @vuetifyjs/core-team

@J-Sek J-Sek requested a review from a team September 13, 2025 21:57
@johnleider
Copy link
Member

Is there anywhere else in the framework where we parse or otherwise try to do something special when dealing with "empty" values? Are there any performance implication for building 2 more computed arrays?

props,
'modelValue',
[],
v => transformIn(wrapInArray(v)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change necessary? Combobox only works with returnObject so modelValue=null will never match any item unless they're doing something weird with the slots.

Copy link
Contributor Author

@jcjp jcjp Sep 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, after careful investigation, it seems to be not needed. During my investigation it seems that the emptyValues are not properly set on final render thus the placeholder not being shown properly will further investigate this thanks! EDIT: I updated the code and the fix seems to be working fine.

@KaelWD
Copy link
Member

KaelWD commented Sep 16, 2025

closes #20742

this doesn't affect any other components, is that still a problem elsewhere?

@J-Sek
Copy link
Contributor

J-Sek commented Sep 16, 2025

closes #20742
this doesn't affect any other components, is that still a problem elsewhere?

They are overlaping, but could go together. The old #20742 is stuck a bit because, VSelect and VAutocomplete require more testing and I have dropped the ball. My concern is about transformIn not being called (?) after items are changed to include a value with '', so model/selection could be missing an item.

VCombobox is simplere and could be fixed separately/first with this PR - it would be a step forward. #20742 can stay open.

@KaelWD
Copy link
Member

KaelWD commented Sep 16, 2025

transformIn not being called (?) after items are changed

It should be, it's called in the computed getter in proxiedModel

@J-Sek
Copy link
Contributor

J-Sek commented Sep 16, 2025

It should be, it's called in the computed getter in proxiedModel

👍🏼 confirmed as working, shows Empty when the item is pushed with delay

setTimeout(() => {
  options1.value.push({ title: 'Empty', value: '' })
}, 2000)

That PR still needs some good testing. Assuming it gets done, we can consolidate both PRs. @jcjp , is there any major difference between both implementation changes in list-items.ts? Nevermind, it's Kael's performance improvement.

@jcjp jcjp marked this pull request as draft September 21, 2025 12:54
@jcjp jcjp marked this pull request as ready for review September 24, 2025 09:42
@jcjp jcjp requested review from J-Sek and KaelWD September 24, 2025 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VCombobox T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report][3.9.6] v-combobox placeholder not showing initially if the model value is empty string
4 participants