Sp-dev-fx-controls-react: Add resolveDelay Property to PeoplePicker

Created on 27 Sep 2018  路  10Comments  路  Source: pnp/sp-dev-fx-controls-react

_Originally posted by @Arknev in https://github.com/SharePoint/sp-dev-fx-controls-react/issues/117#issuecomment-422910161_

The UI freezes when searching for users. In our tenant we have millions of users. It would be great if the "resolveDelay" property (IBasePickerProps) could be set when calling the PeoplePicker. I don't see a way to delay search until a minimum number of characters are entered, which would be best, but this time delay would likely meet our needs.

Most helpful comment

@tsekityam , seems like above code is working because it is filtering/searching limiting to first 5 users. However this doesn't make the filter work properly as it should filter all users not 5. So, may be the above is not a right approach.

The code in previous comment is for referencing only. I have created a pull request #145 for it.

Looking at the technical resolution, I suppose there are two other approaches we could try and see if resolveDelay property is not a fix. @tsekityam , could you give these a try. I don't have an environment to test so cannot validate.

1. Set `limitResults`property in `_onPersonFilterChanged`method to see if that fixes. I am thinking ResolveSuggestions method causing the bottleneck.

2. Set `resultsMaximumNumber` property in `BaseSuggestionprops` to see if that limits the hang on the suggestion resolve.

I will try them all later today.

All 10 comments

Set resolveDelay to 1000 reduce the chance of no responding, but it doesn't resolve the issue.

The page may have no responding if I search for one character only. Looks like we need to find more way to improve the proformance

By the way, we can limit the length of text to be used in filtering by

  private _filterPersons(filterText: string): IPersonaProps[] {
    if (filterText.length < this.props.minFilterTextLength) {
        return [];
    }
    return this.state.peoplePersonaMenu.filter(item =>
    this._doesTextStartWith(item.text as string, filterText)
    || this._doesTextContains(item.text as string, filterText)
    || this._doesTextStartWith(item.secondaryText as string, filterText)
    || this._doesTextContains(item.secondaryText as string, filterText));
  }

But I think that it resolve the issue completely, because we still need to compare thousands of user profiles with the filter text.

I will try limiting the number of the result, see if it helps.

Looks like limiting the number of suggestions helps.

Perhaps the people picker could not handle too many suggestions.

Here is the code I tried:

  /**
   * Filter persons based on Name and Email (starting with and contains)
   *
   * @param filterText
   */
  private _filterPersons(filterText: string): IPersonaProps[] {
    let result: IPersonaProps[] = [];
    let suggestionLimit = this.props.suggestionLimit ? this.props.suggestionLimit : 5;

    for (let i: number = 0; i < this.state.peoplePersonaMenu.length; i++) {
      let item = this.state.peoplePersonaMenu[i];

      if (this._doesTextStartWith(item.text as string, filterText)
      || this._doesTextContains(item.text as string, filterText)
      || this._doesTextStartWith(item.secondaryText as string, filterText)
      || this._doesTextContains(item.secondaryText as string, filterText))
      {
        result.push(item);
        if (result.length === suggestionLimit) {
          break;
        }
      }
    }

    return result;
  }

@tsekityam , seems like above code is working because it is filtering/searching limiting to first 5 users. However this doesn't make the filter work properly as it should filter all users not 5. So, may be the above is not a right approach.

Looking at the technical resolution, I suppose there are two other approaches we could try and see if resolveDelay property is not a fix. @tsekityam , could you give these a try. I don't have an environment to test so cannot validate.

  1. Set limitResultsproperty in _onPersonFilterChangedmethod to see if that fixes. I am thinking ResolveSuggestions method causing the bottleneck.
  2. Set resultsMaximumNumber property in BaseSuggestionprops to see if that limits the hang on the suggestion resolve.

@tsekityam , seems like above code is working because it is filtering/searching limiting to first 5 users. However this doesn't make the filter work properly as it should filter all users not 5. So, may be the above is not a right approach.

The code in previous comment is for referencing only. I have created a pull request #145 for it.

Looking at the technical resolution, I suppose there are two other approaches we could try and see if resolveDelay property is not a fix. @tsekityam , could you give these a try. I don't have an environment to test so cannot validate.

1. Set `limitResults`property in `_onPersonFilterChanged`method to see if that fixes. I am thinking ResolveSuggestions method causing the bottleneck.

2. Set `resultsMaximumNumber` property in `BaseSuggestionprops` to see if that limits the hang on the suggestion resolve.

I will try them all later today.

  1. Set limitResultsproperty in _onPersonFilterChangedmethod to see if that fixes. I am thinking ResolveSuggestions method causing the bottleneck.
  2. Set resultsMaximumNumber property in BaseSuggestionprops to see if that limits the hang on the suggestion resolve.

Both of them fix the freezing, seems that the limitation is on rending too many suggestions.

Anyway, we still need to add a new props resultsMaximumNumber to let user to set the number of results show.

By the way, limitResults is not a parameter of onResolveSuggestions callback, so it is always undefined. We have to change _onPersonFilterChanged to

  private _onPersonFilterChanged = (filterText: string, currentPersonas: IPersonaProps[]): IPersonaProps[] => {
    let limitResults = this.props.limitResults;
    if (filterText) {
      let filteredPersonas: IPersonaProps[] = this._filterPersons(filterText);
      filteredPersonas = this._removeDuplicates(filteredPersonas, currentPersonas);
      filteredPersonas = limitResults ? filteredPersonas.splice(0, limitResults) : filteredPersonas;
      return filteredPersonas;
    } else {
      return [];
    }
  }

@tsekityam , great, thanks for checking it. Yes, adding the property resultsMaximumNumber should be fine but lets call the property 'maximumSuggestions' so it makes a closer sense to the implementation. Would you like to take a stab at this change and implement the changes, since you already have a pull request for it?

Yeah, _onPersonFilterChanged is a event method, so thought the value of limitResultsshould be picked automatically from suggestionProps but your implementation looks fine too. But lets' go with resultsMaximumNumber as we would't need to change any methods that way. Thoughts?

@AsishP , agree. Let me revise my PR...

Thanks @tsekityam and @AsishP. I have merged the PR + updated the property to suggestionsLimit to make more sense of its intention.

A new beta has been released, if you want to test it out @Arknev to verify the fix. You can follow these steps in order to install a beta version: https://sharepoint.github.io/sp-dev-fx-controls-react/beta/

This is now available in version 1.10.0.

Yes, this works great. One 'npm update', added new property (suggestionsLimit), bundled, and pushed to our site collection. Everything is working smoothly. Thanks!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

pavan2920 picture pavan2920  路  8Comments

AdiuTankT72 picture AdiuTankT72  路  4Comments

estruyf picture estruyf  路  6Comments

thespooler picture thespooler  路  6Comments

AriGunawan picture AriGunawan  路  8Comments