Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SelectableList not working #2347

Closed
prashaantt opened this issue Dec 2, 2015 · 20 comments
Closed

SelectableList not working #2347

prashaantt opened this issue Dec 2, 2015 · 20 comments
Labels
component: list This is the name of the generic UI component, not the React module! component: select This is the name of the generic UI component, not the React module!

Comments

@prashaantt
Copy link

I can't get selectable lists to work. After failing to work it out in my own project, I've tried the example straight out of the manual -- the one that looks like this:

getInitialState() {
  return { selectedIndex: 1 };
},

handleUpdateSelectedIndex(e,index) {
  this.setState({
    selectedIndex: index,
  });
}
...

let valueLink = {
  value: this.state.selectedIndex,
  requestChange: this.handleUpdateSelectedIndex
}

...

<SelectableList valueLink={valueLink}>
  <ListItem
    value={...}
    primaryText="..."/>
  ...
</SelectableList>

A couple observations:

  1. The handleUpdateSelectedIndex isn't getting fired no matter what. So no highlights.
  2. The ListItems must be the direct children of the SelectableList for it to even highlight the first line upon first render. That means that I can't work out any complex logic inside a map to, let's say, include ListDividers for all but the last ListItem — which needs my map to return the ListItem and ListDivider inside a div per iteration.

Am I missing something?

@prashaantt
Copy link
Author

Rookie mistake, I wasn't including the react-tap-event-plugin which was causing the click events to also not register.

But my issue no. 2 is still valid.

@JasonWeiseUnreal
Copy link

I can confirm this.. SelectableList doesn't even fire the handleUpdateSelectedIndex event if you use a map and want ListDividers.
Example on what doesn't work inside SelectableList

<SelectableList>
{this.data.sites.map(site =>
        <div key={site._id} >
               <ListItem  value={site.index}
                                leftAvatar={<img src="images/icon_bp.png" />}
                                rightIconButton={rightIconMenu}
                                primaryText={site.name}
                                secondaryText={<p>
                                        <span style={{color: Colors.darkBlack}}>{site.phone}</span><br/>
                                        {site.address}
                                      </p>}
                                secondaryTextLines={2}
                  />
                 <ListDivider inset={true} />
       </div>
</SelectableList>

Can anyone confirm how to have "dynamic" list items in a selectable list with dividers.

@alitaheri
Copy link
Member

Well this issue isn't really related to this component alone. It's By Design. The SelectableList interacts with it's immediate children. I believe this, along many other components must be revised to support nesting of their expected children. This can be achieved with the use of context. SelectableList can provide a callback with it's context and then expected children (or any other custom ListItem) can use these callbacks to interact with their parent, the same case is with Table and it's Rows and Columns.
@oliviertassinari What do you think about the approach I proposed?

@oliviertassinari
Copy link
Member

I have another remarque. I think that we should drop the support of valueLink accordingly to facebook/react#2302.

I'm not sure that we should use the context in this situation.
I'm not sure to follow the 2/.
It seems to me that you can use an ListDivider if I refer to https://github.com/callemall/material-ui/blob/master/src/hoc/selectable-enhance.js#L96.

@alitaheri
Copy link
Member

@oliviertassinari Actually the issue here is that in order for SelectableContainerEnhance to understand ListItems they should be it's immediate children, not nested within a div or any other components. we should either recursively search the sub tree (very inefficient) or have ListItem register itself within the SelectableContainerEnhance using the context it provides. Or do you have other solutions in mind? This is the best I can come up with -_-

@oliviertassinari
Copy link
Member

@subjectix Ohh, I see, this is not the first time I see this issue. I think that we shouldn't support nested component. That's just to complicate and often doesn't provide any value.
But I might be wrong.

That means that I can't work out any complex logic inside a map to, let's say, include ListDividers for all but the last ListItem

I think that you can. How are you doing it?

@alitaheri
Copy link
Member

@oliviertassinari That's one way to put it, I can also think of another way to somehow support this. what if we have a HOC that handles all the logic and can be easily customized. If we don't support nested components, then we have to make ListItem highly customizeable.

@prashaantt
Copy link
Author

I think that you can. How are you doing it?

@CoveTechnologyAus was almost there:

<SelectableList>
  {this.data.sites.map((site, index) => {
    return (
      <div key={...} >
        <ListItem value={...} />
        {index < this.data.sites.length - 1 ? <ListDivider /> : ''}
      </div> 
    )
  })}
</SelectableList>

@oliviertassinari
Copy link
Member

Can you try without the div and with React.Children.toArray?

@alitaheri
Copy link
Member

@prashaantt Ummm... calling map isn't the only way to do this. you can program a little.

const length = this.data.sites.length;
const items = [];
for (let i = 0; i < length; i++) {
  // Don't push a ListDivider for the first item
  if(i !== 0) items.push(<ListDivider key={i} />);
  items.push(<ListItem value={this.data.sites[i]} key={i + length} />);
}
return (
    <SelectableList>
      {items}
    </SelectableList>
);

Edit:

You can also use Array.prototype.reduce. But that's way too hacky!

@prashaantt
Copy link
Author

@subjectix Well yes, that's a good work around but a map looks more elegant to me, personally. I have since anyhow programmed around this issue by writing my own onClick handlers on the ListItem, but a proper native support for nested components would be ideal.

@oliviertassinari
Copy link
Member

but a map looks more elegant to me

That's what React.Children.toArray is for.

@alitaheri
Copy link
Member

@oliviertassinari Is right, you can use that method to do what you want rather easily. I didn't know about it myself 😅

@prashaantt
Copy link
Author

I didn't know of React.children.toArray. This works perfectly:

<SelectableList>
  {this.items.map(item => {
    return (
      React.Children.toArray([
        <ListDivider />,
        <ListItem value={...} />
      ])
    )
  })}
</SelectableList>

Thank you @oliviertassinari!

@oliviertassinari
Copy link
Member

@prashaantt Can we close the issue then? :)

@prashaantt
Copy link
Author

Yes, but I think this is a frequent use case and should be documented. I'll try sending a PR in a couple of days.

@oliviertassinari
Copy link
Member

Alright, feel free to open an PR to add an example.

@diwyanshu
Copy link

diwyanshu commented Apr 23, 2016

Thank You Call-Em-All team for providing the lovely platform of Material Design on top of React

I have a question to ask:

By any means is it possible to do multiple selection from the list ??
Like in the wrapstate

selectedIndex prop, can be provide an array of index rather than a single id,

Or will it be better to make a component by own for multiple select purpose. as i guess the Selectable List Component enhance list code is not meant for multiple option

I don't want to edit the npm module in my project directory, Can we have a work around for this

Looking for some positive suggestion on it .

Thanks !! :)

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 23, 2016

@diwyanshu The feature you are looking for isn't implemented.
Still, we have a HOC that is close: MakeSelectable. You could start from here and do few changes.

@lakshmantgld
Copy link

@diwyanshu any improvements on multi-selectable list?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: list This is the name of the generic UI component, not the React module! component: select This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

7 participants