vue-select icon indicating copy to clipboard operation
vue-select copied to clipboard

The selected-option-container slot has an inconsistent deselect function API

Open lloydjatkinson opened this issue 3 years ago • 2 comments

selected-option-container
This is the root element where v-for="option in selectedValue". Most of the time you'll want to use selected-option, but this container is useful if you want to disable the deselect button, or have fine grain control over the markup.

option {Object} - Currently iterated selected option
deselect {Function} - Method used to deselect a given option when multiple is true
disabled {Boolean} - Determine if the component is disabled
multiple {Boolean} - If the component supports the selection of multiple values

It's not clear from the docs that the deselect function requires passing the current option, e.g. deselect(option).

I was going to make a PR to add a sentence explaining this but it raises the question of if option should even be passed to deselect. The reason I ask this is because this slot is being rendered in a v-for in the Select component, and it also has all of context for the current option:

  • option is the exact option

  • disabled indicates if the exact option is disabled or not

  • deselect requires passing the current option

I propose instead that this function be bound from within the v-for.

Instead of:

https://github.com/sagalbot/vue-select/blob/2f18243d0ad7225d266a17f09b5b3c43c59079e5/src/components/Select.vue#L11-L16

It would become:

        <slot v-for="option in selectedValue"
              name="selected-option-container"
              :option="normalizeOptionForSlot(option)"
              :deselect="() => deselect(option)"
              :multiple="multiple"
              :disabled="disabled">

This would then also match the existing behaviour in the existing slots default content: https://github.com/sagalbot/vue-select/blob/2f18243d0ad7225d266a17f09b5b3c43c59079e5/src/components/Select.vue#L21-L23

lloydjatkinson avatar Apr 14 '21 19:04 lloydjatkinson

+1 to this 🙂 Not understanding why my deselect call wasn't working led me to this issue.

bkuzma avatar Sep 03 '21 07:09 bkuzma

@bkuzma same here. 🙂

me-shaon avatar Jan 20 '22 18:01 me-shaon