mui-downshift icon indicating copy to clipboard operation
mui-downshift copied to clipboard

Updated multi select example

Open jozsi opened this issue 5 years ago • 12 comments

Based on the multi-select branch, combined with the material-ui and downshift examples

TODO

  • [ ] prop to prevent menu from automatically opening after a selection?
  • [ ] clear adornment? (needs a customizable handleClearSelection)
  • [ ] ...

jozsi avatar Sep 04 '18 18:09 jozsi

@jozsi Thanks for taking the initiative to get this going.

  • Regarding the prop to prevent menu from automatically opening, could this be accomplished using downshift's stateReducer?

Along with chips, I've also wanted to support showing a "# selected" similar to

Like the bootstrap example, it would be nice to show some amount long form, and then change to "# selected", but instead of adding props, I'd like to be able to control this using a render prop/etc (see valueRenderer)

techniq avatar Sep 04 '18 19:09 techniq

Hey @techniq,

I'm explicitly calling the openMenu method of downshift on change, otherwise the user needs to blur then re-focus to be able to select again.

Since the chips are added as a left adornment, it always keeps less space for the user input. Should probably set a limit on that and either wrap the chips on multiple rows (I don't know if it's a good idea), or just use the # selected. I guess that would also require checkboxes in the ListItem.

jozsi avatar Sep 04 '18 20:09 jozsi

@jozsi Thanks for doing this. 👍

AirborneEagle avatar Sep 04 '18 21:09 AirborneEagle

@jozsi regarding using left adornment and input, you can also set a minWidth on the input to also make sure there is enough room

image

techniq avatar Sep 05 '18 13:09 techniq

Before getting deeper into this, I've started developing tests to make sure things work out as expected. My experience with testing React components is still amateurish, so please bear with strange stuff below (any suggestion is more than welcome!):

import createTestContext from 'react-cosmos-test/enzyme';
import fixture from './base.fixture';

const { mount, getWrapper } = createTestContext({ fixture });

beforeAll(mount);

describe('demonstrate integration vs. unit tests', () => {
  let select;
  const item = {
    label: '4-LOM',
    value: 0,
  };

  it('mounts the Select component', () => {
    // Select is a wrapped inside a Higher Order Component
    // so it is not the root component, but it's children
    //
    // getWrapper(selector) is an alias of getWrapper().find(selector)
    select = getWrapper('Select').instance();

    // We will need to spy on these calls for later tests
    jest.spyOn(select, 'getListItem');
    jest.spyOn(select, 'handleDelete');
  });

  it('[integration] accepts user input', () => {
    // For the integration test, we interact with the native input element
    const input = getWrapper('input');
    // We search for the first character of our desired item (4)
    // and make the assumption it's the single result
    input.simulate('change', { target: { value: item.label[0] } });
    expect(input.instance().value).toBe(item.label[0]);
  });

  it('[unit] accepts user input', () => {
    // The input is controlled by Select, so we verify to see it's state value
    expect(select.state.inputValue).toBe(item.label[0]);
  });

  it('[integration] filters list based on input', () => {
    // Each result in the autocomplete is a ListItem
    const listItems = getWrapper('ListItem');
    // Based on our search criteria, there's a single result
    expect(listItems).toHaveLength(1);
  });

  it('[unit] filters list based on input', () => {
    // The component state has of list of the results matching the filter
    expect(select.state.filteredItems).toEqual([item]);
  });

  it('[integration] should display list correctly', () => {
    // The actual text/label is inside a ListItemText component
    const listItemText = getWrapper('ListItemText');
    expect(listItemText.text()).toBe(item.label);
  });

  it('[unit] should display list correctly', () => {
    // It might not be the best approach, but we check that in our last call,
    // getListItem is requested to render our expected result
    expect(select.getListItem).lastCalledWith(
      expect.objectContaining({ item }),
    );
  });

  it('[integration] should select item', () => {
    // We need to simulate a click on the ListItem for it to be selected
    const listItem = getWrapper('ListItem');
    listItem.simulate('click');
    // A Chip will then be added with the corresponding label
    expect(getWrapper(`Chip[label="${item.label}"]`).exists()).toBeTruthy();
  });

  it('[unit] should select item', () => {
    // The component state has of list of selected items
    expect(select.state.selectedItems).toEqual([item]);
  });

  it('[integration] resets input after selection', () => {
    const input = getWrapper('input');
    expect(input.instance().value).toBe('');
  });

  it('[unit] resets input after selection', () => {
    expect(select.state.inputValue).toBe('');
  });

  it('[integration] re-opens menu after selection', () => {
    const menu = getWrapper('Menu');
    // Menu always exists, but is empty when it's not open
    expect(menu.children().exists()).toBeTruthy();
  });

  it('[unit] re-opens menu after selection', () => {
    // We dig down into the Downshift instance and verify it's state
    const downshift = getWrapper('Downshift');
    expect(downshift.instance().state.isOpen).toBeTruthy();
  });

  it('[integration] deletes the selection', () => {
    // The Chip has an icon that removes the selection on click
    const icon = getWrapper('SvgIcon');
    icon.simulate('click');
    expect(getWrapper('SvgIcon').exists()).toBeFalsy();
  });

  it('[unit] deletes the selection', () => {
    // The correct selection should've been sent for deletion
    expect(select.handleDelete).lastCalledWith(item);
  });
});

PS: I am using react-cosmos and it's headless testing, imagine a Cosmos fixture as a more advanced story that - besides the visual playground capabilities (that could supersede the current Storybook implementation) - can also be mounted in a test.

jozsi avatar Sep 09 '18 16:09 jozsi

Hey @jozsi and @techniq , whats the status on this? I am getting closer to putting this functionality in my code. If I get to that point and this is not completed yet, I would be happy to jump on it and help.

AirborneEagle avatar Sep 12 '18 16:09 AirborneEagle

@AirborneEagle - I am still experimenting with this in a project of mine, will have enough feedback from users in a couple of weeks based on their feedback. Meanwhile feel free to contribute to it :) the current shape of it is very well usable.

jozsi avatar Sep 12 '18 16:09 jozsi

@jozsi I've not used react-cosmos myself, and I've mostly been using react-testing-library over enzyme nowadays (see react-fetch-component's tests for an example).

With that said, if you're willing to put in the work to add some tests, I'm all for it. I've been considering moving from Storybook to docz (but mostly for interactive examples / documentation), but as long as react-cosmos adds the proper value, I won't mind. I would rather use react-testing-library over enzyme if that works for you though. Kent has always been great to work with (he wrote downshift and also react-testing-library) and in my opinion writing tests with it feels "less magicky", although async isn't as as straight forward in some cases.

techniq avatar Sep 17 '18 02:09 techniq

Whats the status here? I am now needing a multi-select option. I feel lame, since I am not really contributing, but I am pretty new to react and would almost certainly dirty the code should I touch it. 🤕

But I would be happy to try using or testing something. Is there something I can do to help move this feature along?

AirborneEagle avatar Oct 05 '18 22:10 AirborneEagle

I am still making changes every few days to it based on the user feedback, however, it might've become a bit too opinionated. The code also needs a little refactoring, but it works. Send me an email if you are in need of it right away - I'll try to update the PR asap, though.

jozsi avatar Oct 05 '18 22:10 jozsi

I will stay tuned. I have just copied the code from your last commit, and I was going to try and use that locally until the PR is complete. but if you have made a lot of changes since then, perhaps I should get your updates. I'll send you a message with my email address.

AirborneEagle avatar Oct 05 '18 22:10 AirborneEagle

Any progress on this PR?

andrewmclagan avatar Mar 06 '19 03:03 andrewmclagan