Skip to content

Commit

Permalink
Make sure selected value is an option after option changed and react …
Browse files Browse the repository at this point in the history
…to value property changes even if tracking value internally (#914)

* make sure selected tracked value is an option if possible

Before this case did not work correctly:

- Select was rendered with *no* options, but *with* a saved value
- Options were fetched by ajax and options prop was updated
- Reduce function if passed

What happens without this commit is that the selected tracked value
simply was the raw reduced value (previously saved). Which means that
displaying a label for example does not work if the label comes from the
unreduced option.

This commit makes sure that the internal tracked value is checked
against all options not only once the select is created but additionally
when options change.

* remove useless keys

- first key was always undefined
- second key was always the index which is not usefull at all since it
  changes based on the order

* add test for setting value after option changed

* correctly react to value property changes if tracking internally

fixes #855, #842

* add getOptionKey prop

* yarn upgrade doc

* add getOptionKey api doc and fix links

* yarn upgrade

* do not use key on slot

* fix label spec
  • Loading branch information
doits authored and sagalbot committed Sep 13, 2019
1 parent 8ef15a1 commit ebcdcc5
Show file tree
Hide file tree
Showing 7 changed files with 3,829 additions and 4,052 deletions.
34 changes: 33 additions & 1 deletion docs/api/props.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ getOptionLabel: {
return console.warn(
`[vue-select warn]: Label key "option.${this.label}" does not` +
` exist in options object ${JSON.stringify(option)}.\n` +
'http://sagalbot.github.io/vue-select/#ex-labels'
'https://vue-select.org/api/props.html#getoptionlabel'
)
}
return option[this.label]
Expand All @@ -188,6 +188,38 @@ getOptionLabel: {
},
```

## getOptionKey

Callback to get an option key. If `option`
is an object and has an `id`, returns `option.id`
by default, otherwise tries to serialize `option`
to JSON.

The key must be unique for an option.

```js
getOptionKey: {
type: Function,
default(option) {
if (typeof option === 'object' && option.id) {
return option.id
} else {
try {
return JSON.stringify(option)
} catch(e) {
return console.warn(
`[vue-select warn]: Could not stringify option ` +
`to generate unique key. Please provide 'getOptionKey' prop ` +
`to return a unique key for each option.\n` +
'https://vue-select.org/api/props.html#getoptionkey'
)
return null
}
}
}
},
```

## onTab

Select the current value if `selectOnTab` is enabled
Expand Down
3,485 changes: 1,511 additions & 1,974 deletions docs/yarn.lock

Large diffs are not rendered by default.

78 changes: 68 additions & 10 deletions src/components/Select.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
:deselect="deselect"
:multiple="multiple"
:disabled="disabled">
<span class="vs__selected" v-bind:key="option.index">
<span :key="getOptionKey(option)" class="vs__selected">
<slot name="selected-option" v-bind="normalizeOptionForSlot(option)">
{{ getOptionLabel(option) }}
</slot>
Expand Down Expand Up @@ -55,7 +55,7 @@
<li
role="option"
v-for="(option, index) in filteredOptions"
:key="index"
:key="getOptionKey(option)"
class="vs__dropdown-option"
:class="{ 'vs__dropdown-option--selected': isOptionSelected(option), 'vs__dropdown-option--highlight': index === typeAheadPointer }"
@mouseover="typeAheadPointer = index"
Expand Down Expand Up @@ -245,7 +245,7 @@
return console.warn(
`[vue-select warn]: Label key "option.${this.label}" does not` +
` exist in options object ${JSON.stringify(option)}.\n` +
'http://sagalbot.github.io/vue-select/#ex-labels'
'https://vue-select.org/api/props.html#getoptionlabel'
)
}
return option[this.label]
Expand All @@ -254,6 +254,39 @@
}
},
/**
* Callback to get an option key. If {option}
* is an object and has an {id}, returns {option.id}
* by default, otherwise tries to serialize {option}
* to JSON.
*
* The key must be unique for an option.
*
* @type {Function}
* @param {Object || String} option
* @return {String}
*/
getOptionKey: {
type: Function,
default(option) {
if (typeof option === 'object' && option.id) {
return option.id
} else {
try {
return JSON.stringify(option)
} catch(e) {
return console.warn(
`[vue-select warn]: Could not stringify option ` +
`to generate unique key. Please provide'getOptionKey' prop ` +
`to return a unique key for each option.\n` +
'https://vue-select.org/api/props.html#getoptionkey'
)
return null
}
}
}
},
/**
* Select the current value if selectOnTab is enabled
*/
Expand Down Expand Up @@ -437,12 +470,28 @@
/**
* Maybe reset the value
* when options change.
* Make sure selected option
* is correct.
* @return {[type]} [description]
*/
options(val) {
if (!this.taggable && this.resetOnOptionsChange) {
this.clearSelection()
}
if (this.value && this.isTrackingValues) {
this.setInternalValueFromOptions(this.value)
}
},
/**
* Make sure to update internal
* value if prop changes outside
*/
value(val) {
if (this.isTrackingValues) {
this.setInternalValueFromOptions(val)
}
},
/**
Expand All @@ -453,24 +502,33 @@
*/
multiple() {
this.clearSelection()
},
}
},
created() {
this.mutableLoading = this.loading;
if (this.$options.propsData.hasOwnProperty('reduce') && this.value) {
if (Array.isArray(this.value)) {
this.$data._value = this.value.map(value => this.findOptionFromReducedValue(value));
} else {
this.$data._value = this.findOptionFromReducedValue(this.value);
}
if (this.value && this.isTrackingValues) {
this.setInternalValueFromOptions(this.value)
}
this.$on('option:created', this.maybePushTag)
},
methods: {
/**
* Make sure tracked value is
* one option if possible.
* @param {Object|String} value
* @return {void}
*/
setInternalValueFromOptions(value) {
if (Array.isArray(value)) {
this.$data._value = value.map(val => this.findOptionFromReducedValue(val));
} else {
this.$data._value = this.findOptionFromReducedValue(value);
}
},
/**
* Select a given option.
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/Labels.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe("Labels", () => {
Select.vm.open = true;
expect(spy).toHaveBeenCalledWith(
'[vue-select warn]: Label key "option.label" does not exist in options object {}.' +
"\nhttp://sagalbot.github.io/vue-select/#ex-labels"
"\nhttps://vue-select.org/api/props.html#getoptionlabel"
);
});

Expand Down
9 changes: 9 additions & 0 deletions tests/unit/ReactiveOptions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,13 @@ describe("Reset on options change", () => {
Select.setProps({options: ["four", "five", "six"]});
expect(Select.vm.selectedValue).toEqual([]);
});

it("should return correct selected value when the options property changes and a new option matches", () => {
const Select = shallowMount(VueSelect, {
propsData: { value: "one", options: [], reduce(option) { return option.value } }
});

Select.setProps({options: [{ label: "oneLabel", value: "one" }]});
expect(Select.vm.selectedValue).toEqual([{ label: "oneLabel", value: "one" }]);
});
});
15 changes: 15 additions & 0 deletions tests/unit/Reduce.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,19 @@ describe("When reduce prop is defined", () => {
});

});

it("reacts correctly when value propery changes", () => {
const optionToChangeTo = { id: 1, label: "Foo" };
const Select = shallowMount(VueSelect, {
propsData: {
value: 2,
reduce: option => option.id,
options: [optionToChangeTo, { id: 2, label: "Bar" }]
}
});

Select.setProps({ value: optionToChangeTo.id });

expect(Select.vm.selectedValue).toEqual([optionToChangeTo]);
});
});
Loading

0 comments on commit ebcdcc5

Please sign in to comment.