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

Suggest component performance #3281

Open
synapse opened this issue Jan 14, 2019 · 19 comments
Open

Suggest component performance #3281

synapse opened this issue Jan 14, 2019 · 19 comments

Comments

@synapse
Copy link

synapse commented Jan 14, 2019

Is there a way to add/set one of the following options in order to make big list (1000+ items) suggestion component more performant:

  • add a debounce or star filtering after a certain amount of characters
  • limit the number of shown items after the filter is applied in order to make the render faster

I'm having a list of 1000+ items and apart the fact that on mount it hangs a little but on usage is kinda blocking all the UI (on every keystroke)

@giladgray
Copy link
Contributor

@synapse the select components were deliberately designed for much lower scale (~100s items max). you can use itemListRenderer to compose a virtualization library to speed up rendering.
https://blueprintjs.com/docs/#select/select-component.custom-menu

@LoiKos
Copy link

LoiKos commented Jan 15, 2019

I get the same issue with large list (around 4000 items ) and i use a virtualization library to render the list.

Virtualization isn't enough when you need to apply some expensive filtering on the list because the function that render the list (idk why exactly but it seems that's happen only when query managed by the state) a change on the query will start 2 render instead of 1 intended.

I don't know exactly how is managed the suggest inside but when user add the query props the intended behaviour is that the suggest update when the props is updated and not when query is changing.

Then each developer should choose if he need to delay query change to avoid unnecessary rendering.

@synapse
Copy link
Author

synapse commented Jan 15, 2019

thanks @giladgray the virtualised list could work ok to improve the render of a big list but is there a way to delay the search after for example 3 characters? This could further improve the search speed.

@synapse
Copy link
Author

synapse commented Jan 17, 2019

Hey @LoiKos can you show me how you used react-virtualized in the suggest componente. I'm experiencing some flickering when scrolling, also the items disappear when scrolling faster

@LoiKos
Copy link

LoiKos commented Jan 17, 2019

Sure !

function itemListRenderer(itemListProps) {
    
/*few checks  here*/
    
   // find active Item index
    let active = null
    if (itemListProps.activeItem)
      active = itemListProps.filteredItems.findIndex(
        item => item.id === itemListProps.activeItem.id
      )
    // Blueprint function to apply RenderItem on each filteredItems
    const renderItems = renderFilteredItems(itemListProps, noResult())
    // If result is array i used react virtualized
    if (Array.isArray(renderItems)) return renderList(renderItems, active)
    // this is used when i don't find any result to render Suggest noResult 
    return renderItems
}

function noResult() {
  return (
    // your component when no result
  )
}

function itemRenderer(item, itemProps) {
  return ( 
  // Your item component  
)
}

// This is how i render List using react virtualized
function renderList(items, active) {
  let props = {
    width: 400,
    height: 300,
    rowCount: items.length,
    rowHeight: 45,
    rowRenderer: props => {
      return (
        <div key={props.key} style={props.style}>
          {items[props.index]}
        </div>
      )
    },
  }

  if (active) {
    props.scrollToIndex = active
  }

  return <List {...props} />
}

@LoiKos
Copy link

LoiKos commented Jan 17, 2019

you should also look around the overscanRowCount properties for List component : https://github.com/bvaughn/react-virtualized/blob/master/docs/overscanUsage.md

@emadabdulrahim
Copy link

emadabdulrahim commented Nov 7, 2019

@LoiKos Would love if you can provide a small example with full code using any virtual list.

I'm having an issue with that because the virtual list library—I'm using react-window https://github.com/bvaughn/react-window —wants to take over rendering the items but itemRenderer prop should be doing that. Sorry the example you provided above isn't obvious to me

@emadabdulrahim
Copy link

I think I got it.

@LoiKos
Copy link

LoiKos commented Nov 8, 2019

Hello @emadabdulrahim !

create a little example with codeSandbox

https://codesandbox.io/s/kind-mclean-t4lv9?fontsize=14

tell me if that match/solve your problem !

@emadabdulrahim
Copy link

Hey @LoiKos, Thank you so much for the quick reply and the code example. Much appreciated! 🧡

@BackstromForsberg
Copy link

IMO, there should definitely be an optional debounce prop on this component to keep onQueryChange from getting fired on every keystroke. While this may have been designed with small sets of data in mind, the reality is, we get tasked with using a library and a component for a certain task, and we just kind of have to make it work. Debouncing any input is usually a good idea given the right circumstance and, the reality is, it's just incredibly time-consuming and difficult to do it as-is.

Unless I'm totally missing a new feature or a way to do it otherwise, this is should be pretty high priority. I work in the events industry and we need autosuggest components for a lot of different fields on our forms. We're tasked with using blueprint, and this component creates a poor user experience without the ability to debounce until after a user stops typing.

@LoiKos
Copy link

LoiKos commented Apr 1, 2020

@ScottGMallon

onQueryChange from getting fired on every keystroke.

I don't think you want that because if you don't fired onQueryChange each time an user is typing you couldn't update the input field which is really weird UX or very special use case. When i started using suggest, i was thinking like you.

But finally i think it's better that suggest let you manage this on your own. That give you more flexibility.

Quick example how to handle suggest using debounce with Hooks :

function useDebounceCallback(callback, timeout) {
  let timeoutRef = useRef(undefined);

  const cancel = function() {
    if (!!timeoutRef.current) {
      clearTimeout(timeoutRef.current);
      timeoutRef.current = null;
    }
  };

  const debounceCallback = useCallback(
    value => {
      cancel();
      timeoutRef.current = setTimeout(() => {
        timeoutRef.current = null;
        callback(value);
      }, timeout);
    },
    [callback, timeout]
  );

  return [debounceCallback, cancel];
}

function YouSuggestComponent(props) {
  let [query, setQuery] = useState();
  const [debouncedCallback, cancel] = useDebounceCallback(query => {
    /* Do whatever you want/need to do with query*/
  }, 500);

  return <Suggest query={query} onQueryChange={(query, event) => {
      setQuery(query)
      debouncedCallback(query)
  }} {...otherSuggestProps} />;
}

@BackstromForsberg
Copy link

BackstromForsberg commented Apr 2, 2020

I didn't really check the language of my initial comment and I apologize for the misunderstanding of what I was referring to. It was a very busy day. Obviously, we would want the inputs value to update considering that's the problem we faced in the first place. I was referring to the callback.

At the time of writing my previous comment, we already went this approach and debounced the callback. I guess the reason I came here was to second the issue because we thought an optional debounce feature would quite often be used in use cases with large data sets. Also, the only thing that would change from user to user is the timeout value, which made me think it was a pretty good candidate for a prop. When I write filters of other varieties, I find myself writing this type of debounce logic constantly, and the approach never really changes.

If the team decides this to be an unnecessary/weird addition, hey, I totally get it. It's a library. Nothing will ever be perfectly customized to how every dev wants to use a tool, and the example here will certainly help those in a similar situation. And yes, we wrote it once, and now we reuse it, so it's not like we have to write it again.

We just thought it would make it a touch friendlier of a component out of the box. Since we use Blueprint daily, we just wanted to give you our input. No pun intended.

Thanks for the response, and take care.

@giladgray
Copy link
Contributor

giladgray commented Apr 3, 2020

from a library perspective, providing the event callback prop is sufficient to allow users to do whatever they want. debouncing your handler is the best practice here, rather than the library doing it for you, because you have perfect control over how to debounce and I didn't have to do anything 🙌

@Amarendra2627
Copy link

can anyone explain how to write onItemSelect value in a select component from blueprint js?

@adidahiya
Copy link
Contributor

@Amarendra2627 there's an example in the code sandbox linked in the docs

@casperOne
Copy link

thanks @giladgray the virtualised list could work ok to improve the render of a big list but is there a way to delay the search after for example 3 characters? This could further improve the search speed.

I've found that observables are perfect for debouncing input. Here's what I'm using for debouncing/remote lookups using a Blueprint suggest:

https://codesandbox.io/s/ofl-blueprint-typeahead-w4yb8

@petalas
Copy link

petalas commented Apr 19, 2021

Thanks for your example @LoiKos

One problem I noticed is that by using props.items the renderItem function can potentially return null (based on the predicate/query). Then you can end up rendering 'blank' rows.

The best fix afaik is to just use props.filteredItems instead.
This also improves performance significantly, at least in my case.

Just thought I'd mention it in case anyone else runs into this issue.

My suggestion:

  const itemListRenderer: ItemListRenderer<Whatever> = (props) => {
    return (
      <Menu ulRef={props.itemsParentRef}>
        <List
          height={300}
          itemCount={props.filteredItems.length}
          itemSize={35}
          width={400}>
          {({ index, style }) => (
            <div style={style}>
              {props.renderItem(props.filteredItems[index], index)}
            </div>
          )}
        </List>
      </Menu>
    );
  };

@markgmat

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants