benbowes / react-responsive-select

A customisable, touchable, React select / multi-select form control. Built with keyboard and screen reader accessibility in mind
https://benbowes.github.io/react-responsive-select/
MIT License
108 stars 17 forks source link

TypeError when `selectedValues` is set to empty array #76

Closed robdmoore closed 6 years ago

robdmoore commented 6 years ago

Hi,

When setting selectedValues to an empty array for a multi-select control there is a type error (attempt to access property text on undefined).

I was trying to bind it to a property that will sometimes have elements and sometimes not.

robdmoore commented 6 years ago

Here is a test that I think would work, I'm struggling to run the tests because I'm getting thousands of line ending failures (I'm running on Windows):

    it('should allow empty selection on multi select', () => {
      const props = {
        multiselect: true,
        name: 'make',
        selectedValues: [],
        options: [{ text: 'Any', value: 'null' }, { text: 'Fiat', value: 'fiat' }],
      };
      selectBox = setup(undefined, props);
      expect(selectBox.find('.rrs__options .rrs__option.rrs__option--selected')).to.have.length(0);
    });
robdmoore commented 6 years ago

The fix is to change the first line of getInitialMultiSelectSelectedOptions to:

  if (selectedValues && selectedValues.length > 0) {
robdmoore commented 6 years ago

Alternatively, you could do this test instead:

  it('should return first item for multiSelectSelectOptions when selectedValues is empty array', () => {
    const selectedValues = [];
    const name = 'Make 1';
    const result = getInitialMultiSelectSelectedOptions(
      state.options,
      selectedValues,
      name,
    );

    expect(result).to.eql([{
      name: 'Make 1',
      value: 'null',
      text: 'Any',
    }]);
  });
robdmoore commented 6 years ago

In the meantime of this fix the workaround is:

selectedValues={myValuesArray.length > 0 ? myValuesArray : undefined}
benbowes commented 6 years ago

@robdmoore great detail in your issue. I appreciate it.

I reckon that this issue relates to this https://github.com/benbowes/react-responsive-select/issues/77

The dropdown currently assumes that there will always be a first option to select - the "Any" option. Allowing emptiness would be an improvement as per your issue link. Will do.

Turns out that I already have this in local branch. Will finish it.

robdmoore commented 6 years ago

Sweet!

benbowes commented 6 years ago

I have this marked for end of July

benbowes commented 6 years ago

I have tackled this as part of https://github.com/benbowes/react-responsive-select/pull/84