Sharlaan / material-ui-superselectfield

multiselection autocomplete dropdown component for Material-UI
https://sharlaan.github.io/material-ui-superselectfield
MIT License
266 stars 92 forks source link

Selected options deselects itself, on selecting multiple options #154

Closed gauravagarwal2704 closed 6 years ago

gauravagarwal2704 commented 6 years ago

I have around 100 options fetched asynchronously in SuperSelectField, on scrolling and selecting options, previous options de-selects itself. See attached gif

selection error

I am not sure whether this is a bug or some issue on my side. Can you help ?

Sharlaan commented 6 years ago

Ok can you show your handlers passed to onChange (or onSelect if you use it ?) ?

gauravagarwal2704 commented 6 years ago

Sorry for the long code, I am making changes to existing code, so wan't sure what's important and what's not.

SuperSelectField :

<SuperSelectField
          name={this.props.name}
          multiple={this.props.multiple}
          disabled={this.props.status}
          checkPosition="left"
          showAutocompleteThreshold="always"
          hintTextAutocomplete={"Search.."}
          nb2show={8}
          hintText={Placeholder}
          onChange={this.handleTypeaheadChangeLocal}
          onSelect={this.handleDeselectAll}
          value={selected}
          underlineStyle={{ display: "none" }}
          style={{
            minWidth: 150,
            marginTop: 0,
            textOverflow: "ellipsis",
            overflow: "hidden",
            whiteSpace: "nowrap",
            borderRadius: 4,
            border: "solid 1px #dfe3e9",
            paddingLeft: 5,
            paddingRight: 5,
            paddingTop: 5
          }}
          innerDivStyle={{
            fontFamily: "GothamRoundedLight",
            fontWeight: "bold"
          }}
          menuStyle={{ width: "100%" }}
          menuFooterStyle={{ width: "100%" }}
          menuCloseButton={
            <FlatButton
              label="close"
              hoverColor={"#eee"}
              style={{ width: "100%" }}
            />
          }
        >
          {this.props.options
            ? this.props.options.map((data, index) => {
                return (
                  <div key={index} value={data.value} label={data.name}>
                    {data.name}
                  </div>
                );
              })
            : ""}
        </SuperSelectField>

handleDeselectAll :

  handleDeselectAll = (selectedValues, name) => {
     if (selectedValues) {
      var length = selectedValues.length;
      var optionLength = this.props.options.length;

      if (length !== optionLength && a == true) {
        selectedValues.map(o => {
          if (o.label === "All") {
            this.props.PopulateAll(name, (status = 1));
            a = false;
          }
        });
      } else {
        a = true;
        let Allcount = length;
        selectedValues.map(o => {
          if (o.label === "All") {
            Allcount--;
          }
        });
        if (Allcount !== length) {
          this.props.PopulateAll(name, (status = 3));
        }
      }
      var count = 0;
      if (length === optionLength - 1) {
        selectedValues.map(o => {
          if (o.label !== "All") {
            count++;
          }
        });
        if (count === length) {
          this.props.PopulateAll(name, (status = 2));
        }
      }
    }
  };

handleTypeaheadChange:

handleTypeaheadChange = (list, name) => {

    if (list) {
      this.setState(
        {
          selected: {
            ...this.state.selected,
            [name]: list
          }
        },
        () => {
          if (this.state.selected[name]) {
            if (name === "BrandFilter") {
              this.setState(
                {
                  Brand: this.state.selected[name].map(function(o) {
                    return o.value;
                  })
                },
                () => {
                  this.functionCall(name);
                }
              );
            }
          }

          if (name === "SubCatFilter") {
            this.setState(
              {
                SubCategory: this.state.selected[name].map(function(o) {
                  return o.value;
                })
              },
              () => {
                this.functionCall(name);
              }
            );
          }

          if (name === "ProductFilter") {
            this.setState(
              {
                Product: this.state.selected[name].map(function(o) {
                  return o.value;
                })
              },
              () => {
                this.functionCall(name);
              }
            );
          }

          if (name === "CityFilter") {
            this.setState(
              {
                City: this.state.selected[name].map(function(o) {
                  return o.label;
                })
              },
              () => {
                this.functionCall(name);
              }
            );
          }

          if (name === "PartnerFilter") {
            this.setState(
              {
                Partner: this.state.selected[name].map(function(o) {
                  return o.value;
                })
              },
              () => {
                this.functionCall(name);
              }
            );
          }

          if (name === "ServiceCentre") {
            this.setState(
              {
                ServiceCentre: this.state.selected[name].map(function(o) {
                  return o.value;
                })
              },
              () => {
                this.functionCall(name);
              }
            );
          }
        }
      );
    }
Sharlaan commented 6 years ago

mmm i admit i didnot covered this usecase: using BOTH onSelect and onChange ....

Originally, they were supposed to be mutually exclusive, like explained in documentation.

onChange: stores selections in internal state, and update your handler only once on menu closing.

onSelect: use handler to update your external state directly (no storing in internal state)

From what i can see, you want :

Feel free to correct me.

i'll check this more in detail this week-end, and eventually make a release to support the deselectAll feature.

gauravagarwal2704 commented 6 years ago

I am not sure how to handle this :

  1. On selecting multiple options, back to back, previous options gets de-selected.
  2. After closing and re-opening the dropdown, and again selecting the options, it works fine and options get selected.
Sharlaan commented 6 years ago

Ok i have to go, i'll check that more in depth this afternoon.

gauravagarwal2704 commented 6 years ago

Sure, no problem

Sharlaan commented 6 years ago

for 1. here the explanation : you passed handleDeselectAll to onSelect, which from documentation triggers directly on each selection => you are simply "deselecting all" at each selection ...

for 2. here the explanation : you passed handleTypeaheadChange to onChange, which from documentation triggers only on menu closing, and returns the whole selectedValues array stored internally.

"Read the docs, Luke !" ;)

Now i refactored a bit to make these 2 functions bit more readable :

// onSelect={this.handleDeselectAll}
handleDeselectAll = (selectedValues, name) => {
  if (selectedValues) {
    const length = selectedValues.length
    const optionLength = this.props.options.length

    if (length !== optionLength && a) {
      for (const { label } of selectedValues) {
        if (label === 'All') {
          this.props.PopulateAll(name, (status = 1))
          a = false
        }
      }
    } else {
      a = true
      const Allcount = selectedValues.reduce((total, { label }) => label === 'All' ? total-- : total, length)
      if (Allcount !== length) this.props.PopulateAll(name, (status = 3))
    }

    if (length === optionLength - 1) {
      const count = selectedValues.reduce((total, { label }) => label !== 'All' ? total++ : total, 0)
      if (count === length) this.props.PopulateAll(name, (status = 2))
    }
  }
}

// onChange={this.handleTypeaheadChangeLocal}
handleTypeaheadChange = (list, name) => {
  if (list) {
    this.setState(
      { selected: { ...this.state.selected, [name]: list } },
      () => {
        if (!this.state.selected[name]) return

        let prop2update
        switch (name) {
          case 'BrandFilter': prop2update = 'Brand'; break
          case 'SubCatFilter': prop2update = 'SubCategory'; break
          case 'ProductFilter': prop2update = 'Product'; break
          case 'CityFilter': prop2update = 'City'; break
          case 'PartnerFilter': prop2update = 'Partner'; break
          case 'ServiceCentre': prop2update = 'ServiceCentre'; break
          default: prop2update = false
        }

        if (prop2update) {
          this.setState(
            { [prop2update]: this.state.selected[name].map((o) => o.value) },
            () => this.functionCall(name)
          )
        }
      }
    )
  }
}

Now i'm not sure to understand what you want to achieve exactly with each of these functions ? Also what the globals a and status are for ? (pretty bad practice)

gauravagarwal2704 commented 6 years ago

Thanks for the effort, now from the code above you might have figured out that there is one custom label : All, on which all the data in the list gets selected. But on unchecking All, the list should gets deselected, that's what status is for.

Also, onSelect is used to show the following options in dropdown are selected as soon as All is selected.

Can you tell me if there is existing way of selecting all the options in the list and deselecting it ?

Sharlaan commented 6 years ago

Yes i have currently a build adding a "menu header" containing 2 buttons : select all and reset. Thing is im not sure if i should let these buttons present by default, or make them available through a prop ?

Le lun. 19 mars 2018 08:33, Gaurav Agarwal notifications@github.com a écrit :

Thanks for the effort, now from the code above you might have figured out that there is one custom label : All, on which all the data in the list gets selected. But on unchecking All, the list should gets deselected, that's what status is for.

Can you tell me if there is existing way of selecting all the options in the list and deselecting it ?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Sharlaan/material-ui-superselectfield/issues/154#issuecomment-374123486, or mute the thread https://github.com/notifications/unsubscribe-auth/ACkTu5wetuTCNbW_Y5iM2DhFHCySISWKks5tf19GgaJpZM4Stdhe .

gauravagarwal2704 commented 6 years ago

In my opinion it should be available through a prop.

Sharlaan commented 6 years ago

Ok i just pushed a first implementation in dev branch, can you try it ?

New props :

gauravagarwal2704 commented 6 years ago

I have installed material-ui-superselectfield using npm, and I have no idea how to pull dev branch from github in my project ? I tried npm install git://github.com/Sharlaan/material-ui-superselectfield.git#dev --save, but it says Error: Cannot find module "material-ui-superselectfield"

Sorry for the trouble, but can you help me with it ?

gauravagarwal2704 commented 6 years ago

Tried to update it following way :

// package.json
...
"dependencies": {
"material-ui-superselectfield": "github:Sharlaan/material-ui-superselectfield#dev",
}
....

on removing previous folder from node_modules and performingnpm install only files in the folder are, screen shot 2018-03-20 at 3 23 59 pm

Sharlaan commented 6 years ago

Can you try without github: ?

"dependencies": {
...
"material-ui-superselectfield": "Sharlaan/material-ui-superselectfield#dev",
...
}

PS: i doubt it would work since you would need the built files (es/ or lib/ dirs), while there is only sources in repo ...

Sharlaan commented 6 years ago

Another method :

https://help.github.com/articles/syncing-a-fork/
important tip at the end of the article

gauravagarwal2704 commented 6 years ago

I cloned your dev branch today and tried withResetSelectAllButtons, and it works fine. But resetButton and selectAllButton doesn't seems to be working.

Although I have few doubts before I proceed with the steps you provided above:

  1. What will happen, when I have to deploy my app and the local linked version of SSF is not available ?
  2. Since withResetSelectAllButtons is working and that is what I needed, is it possible for you to push it on master, so I can just npm install and the latest version will be updated ?
gauravagarwal2704 commented 6 years ago

Performed all the steps you told, came up with this error :

screen shot 2018-03-20 at 5 25 13 pm

and it's pointing to my local forked copy of SSF.

Sharlaan commented 6 years ago

Yes of course don't deploy with a linked dep ^^

Yes i will publish when i finalize a few CSS bugs corrections. I wanted to be sure everything is working.

How did you test this : But resetButton and selectAllButton doesn't seems to be working. ?

gauravagarwal2704 commented 6 years ago

resetButton = {<FlatButton label='reset' hoverColor='rgba(69, 90, 100, 0.1)' fullWidth />}

selectAllButton = {<FlatButton label='select all' hoverColor='rgba(69, 90, 100, 0.1)' fullWidth labelStyle={{ whiteSpace: 'nowrap' }} />}

Sharlaan commented 6 years ago

These are the already used as default props (look in documentation) => useless

These 2 props are there to allow you to customize. You can use any node you want, styled at your leisure. What i'm interested in is if there are any error when you try to input some custom component ?

gauravagarwal2704 commented 6 years ago

If this was used as default props, why am I not able to see respective buttons ? What am I doing wrong ?

Sharlaan commented 6 years ago

Indeed i forgot to include a note for these 2 props:
the prop withResetSelectAllButtons is required, as per suggestion

In my opinion it should be available through a prop.

ohyjek commented 6 years ago

@Sharlaan I believe this new prop will solve https://github.com/Sharlaan/material-ui-superselectfield/issues/111 as well 👍

Sharlaan commented 6 years ago

Hi all !

I just pushed on dev branch some fixes regarding width management.

Can you test and report me any weirdness please ? hopefully documentation should be clear.

Sharlaan commented 6 years ago

Please check release 1.9.0