-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[SelectField][Dropdown Menu] Menu with many items performance much worse with popovers #2787
Comments
@tehleach After code quality and documentation, performances are the next point I want to take care of. That's worrying. Could we have a gif of what's happening? Or do I just need to render 400 simple items? |
I don't think I can give you a gif. I would assume rendering 400 simple items would do the trick, but I'm looking at the example on the docs site with 100 items and it seems quick. It might have to do with having long primaryText strings? I can do some more thorough testing in a bit. |
Ok, here's a quick project I threw together.. pretty bare bones. I'm just rendering a DropDownMenu component with 400 items. It's possible I'm doing something really stupid but I am still having the same performance issue. If anyone pulls it down and runs it, let me know if you run into any issues. I'd like to look into and try to improve the performance but I'm not sure when I'll have time to. |
I have same performance issue.My SelectField have about 500 items and it's very very slowly(2-3 second) when expanding and collapsing.I try to increase items number from 100 to 500 and then the SelectField's performance exponential decrease. |
I had a similar experience but also have some spots in my app with several SelectFields of 10-20 items - even thought the list of items is short the performance seems to slow exponentially depending on how many total select fields are rendered on the page at any given time. |
I have the same performance issue on all these elements :
Maybe using something like https://github.com/bvaughn/react-virtualized would be the solution ? |
react-virtualized author here. I think that implementing windowing by default within components that may potentially contain a lot of data (like selects) is a great idea. If you'd be interested in discussing a possible integration, @oliviertassinari, please reach out to me. |
@bvaughn The Material-UI team is definitely interested in this integration. We are glad you are willing to help 👍. |
Let's talk more then. Let me know how best to do that. (Email? Hangout? Discord?) |
@oliviertassinari This seems like there may be quite a bit of work/time before this is resolved. Do we need it for |
@nathanmarks That would be better to have into |
I have this performance problem with the AutoComplete with 500 records :( Is there any workaround when its not fixed yet? Thanx |
@marcelobateira We have merged a first PR that improve the rendering by 20%. I'm pretty sure we can do much better. |
same problem here with ~250 items using material-ui 0.15. any news about the fix? |
Also having performance issues with dropdown |
Still happy to lend a hand with react-virtualized integration if there's still interest and things have progressed far enough to work on it. (I think last time we talked there was some refactoring work in the queue that you guys wanted to do first.) |
We implemented this using react-virtualized, though it's been modified for our uses without regard for back compat so it's not easy to merge back upstream. It works great! mitodl/micromasters#568 |
@noisecapella: "it" being react-virtualized or material-ui? |
Material-UI from what I have seen. |
Ah, yeah. They forked If react-virtualized needed to be forked I was going to ask if I could help merge the changes back into the mainline. B'c I think getting material-ui and react-virtualized working together would be great. But since it's a fork of your guy's component... your call I guess. Still happy to help if there's anything I can help with! |
@bvaughn Yes, we didn't need to modify react-virtualized at all. |
We hit this performance issue as well. A quick profiling reveals that pure rendering does not kick-in for return React.cloneElement(child, {
desktop: desktop,
focusState: focusState,
onTouchTap: (event) => {
this.handleMenuItemTouchTap(event, child, index);
if (child.props.onTouchTap) child.props.onTouchTap(event);
},
ref: isFocused ? 'focusedMenuItem' : null,
style: mergedChildrenStyles,
}); The problem is that the |
@ligaz I tested out your idea by setting onTouchTap and style to null, and thus easily returning a false for shouldComponentUpdate. I did my tests with a DropDownMenu of 1000 MenuItems, using React's Perf. Before ---------- After Without any modifications it seems like upon closing, the MenuItems re-render 3 times unnecessarily, which is fairly stupid considering that they are just going to be unmounted anyways. By setting those objects to null, it speeds up the closing of the menu entirely, no time wasted, and near instant. Inclusive and Exclusive rendering time upon opening the menu. I think this is expected, as each of the 1000 items renders once, nothing has been wasted, as seen in the earlier two experiments. Opening the menu was extremely slow regardless of the setting-to-null experiment. So the issue for the closing the menu is definitely a shouldComponentUpdate problem, whereas the issue for opening the menu could just be the slowness of material-ui's method of rendering menus with popovers. |
I am using SelectField with about 900-1000 items and it seems very slow. I wonder if there's any update on this issue ? Thanks :) |
I'm facing this issue in the lastest version Takes around 1.5s to open and render around 300 items on a MacBook Pro. Similar when closing. The code I'm using: <FormControl fullWidth required={required} error={!!error}>
{label && (
<InputLabel shrink htmlFor="select">
{label}
</InputLabel>
)}
<Select
id={id}
native={native}
disabled={disabled}
value={value}
onChange={this.onChange}
input={<Input id="select" />}
>
{showEmpty && <MenuItem value="">--</MenuItem>}
{native
? rows.map(row => (
<option key={getId(row)} value={getId(row)}>
{getTitle(row)}
</option>
))
: rows.map(row => (
<MenuItem key={getId(row)} value={getId(row)}>
{getTitle(row)}
</MenuItem>
))}
</Select>
{helperText && <FormHelperText>{helperText}}</FormHelperText>}
</FormControl> What can we do from our side to improve this? |
@alexjoverm The component wasn't designed to display a large list of choice. Pick the native implementation if you need to display more items, like the list of all the countries. Otherwise, we will need to invest in virtualization. |
In the code above, you can see that if you pass |
@alexjoverm I'm rendering 250 countries with no performance issue with the native option. |
I found similar performance issue with a list of 300. My workaround is to combine react virtualized [List] with i.e. rowRenderer() {
const { key, index, style } = row;
const { options = [] } = this.props;
const option = options[index];
return (
<MenuItem
key={key}
style={style}
value={option}
>
<ListItemText primary={option} />
</MenuItem>
);
}
render() {
const { options = [] } = this.props;
const { selectedValues = [] } = this.state;
return (
<TextField
select
SelectProps={{
multiple: true,
}}
value={selectedValues}
>
<List
width={listWidth}
height={listHeight}
rowCount={options.length}
rowHeight={48}
rowRenderer={this.rowRenderer}
/>
</TextField>
)
} |
@senluchen2015 Sweet! Would you mind adding demo about in the documentation? However, I'm wondering. Does the keyboard navigation works? |
I made this https://codesandbox.io/s/o7zo71nk2q to demo react-virtualized. The keyboard navigation doesn't work as default, but with some workarounds it functions just fine. |
@senluchen2015 any chance we get a working example :D |
I got it working with the Autocomplete virtualized example and the following small modifications to the
The main changes are taking in the the various things I wanted to change. It works great for my 500ish item list |
I was using a SelectField before the changes related to popovers (release 0.14.0-rc2). The menu has about 400 items to choose from. Before, expanding and collapsing the menu was very fast (almost instant) but with the popover change it is quite slow (.5-1 second). I have verified that this performance change happened in 0.14.0-rc2 by switching back and forth between that and 0.14.0-rc1, and observe fast expand/collapse in rc1 but slow expand/collapse in rc2. Has anyone else noticed a similar performance hit? Was there any testing done with large lists of menuItems?
It's a bummer because the popover is giving us lots of nice features (such as actually being able to scroll through the menu items :) ) we were looking for but as is, we cannot use it.
The text was updated successfully, but these errors were encountered: