alsoscotland / react-super-select

MIT License
95 stars 33 forks source link

API Search #100

Closed saulfrance closed 7 years ago

saulfrance commented 7 years ago

Hi,

Is it possible to use the search bar to trigger an API call instead of using hasMoreData? For example, by default the select will have the first 10 results from ajaxDataFetch and then will call an API to get more results passing the search input?

Should be able to do this with an "onSearch" type function but can't find anything similar in the docs.

Thanks

alsoscotland commented 7 years ago

@Saulf1 There is not a way to do this currently.
The control is not designed to work as a typeahead.

arnthorsnaer commented 7 years ago

@alsoscotland Hey hey, I realize your component does not handle API search. I am still wondering if you can recommend a way I could listen for the input changes in my own code and update the datasource which would cause your component to re-render the list?

alsoscotland commented 7 years ago

@arnthorsnaer At the moment the only approach you could really take is to delegate a listener on the search field's onChange event and change the dataSource accordingly. I do not see a reason why I could not broadcast changes to the search input value though. I will investigate

arnthors commented 7 years ago

Awesome, I am also been trying out react-select to solve my issue but frankly I preferred working with react-super-select

alsoscotland commented 7 years ago

@arnthors Thanks! Nice to hear I just got back from vacation. I will take a look at this soon

alsoscotland commented 7 years ago

@arnthors just an update... I have been looking into to it but it is not super simple and I have not had much time. Still working though

arnthorsnaer commented 7 years ago

Ok, thanks a bunch for spending cycles on this. As I mentioned earlier I've got a workaround so not a big deal.

alsoscotland commented 7 years ago

@arnthorsnaer I was able to accomplish something similar with this branch, it creates a onSearchInputChange callback prop which gets called with the search term. If you have some time, please let me know if that works for your use case https://github.com/alsoscotland/react-super-select/tree/ss/issue_100_comment_external_search_event_handler

noahkconley commented 7 years ago

I'm trying to accomplish this, but I'm unclear on what the return value for my onSearchInputChange prop function should look like. Can you give an example or explain how it ties into ajaxFetch?

alsoscotland commented 7 years ago

@gallardo1226 It is a bit atypical for the control as it was not originally intended to serve as a ajax search field for resetting the results. The onSearchInputChange would need to manage the fetch and then reset the props (dataSource/initialValue) of the control once the data returns.

noahkconley commented 7 years ago

Thanks for your quick response! That should work for me, but maybe there is a way I can make it work more along the lines of the intended use. I'm working with dynamic lists that can be as long as 10k options. Returning all that data at once isn't much of an issue, but putting all of it into a select at once isn't performant or user friendly. Can I use the paging and searchable options in tandem to search over a large existing data set while limiting the number of results returned from any search?

TimNZ commented 6 years ago

I need searchable+pageable ajax, so will implement this.

If the control is in ajax mode, we want search input changes to force a refresh, and no longer just filter existing items

Proposed changes:

When search input values changes:

Seems like the minimum needed, without any breaking changes unless explicitly enabled.

Look good?

TimNZ commented 6 years ago

Works great.

All search input change logic moved to new func called by 3 places that currently just call this.props.onSearchInputChange

Need to add debounce support.

Will wait for feedback before merging into fork and a PR.

Centralised search input val change handler:

  _searchInputChange(val,cb)
  {
    if (isEqual(val,this.state.searchString))
      return cb(false);

    this.props.onSearchInputChange(val);
    if (this.props.ajaxSearch && this.props.ajaxDataFetch)
      this.doneAjaxFetch = false

    this.setState({
      searchString: val
    }, () => cb(true))
  }
  // clear the searchString value
  // for **searchable** controls
  _clearSearchString() {
    this._searchInputChange('',() => {
      this._setFocusIdToSearch();
    });
  }
  // handler for searchInput's onChange event
  _handleSearch(event) {
    this._arrestEvent(event);
    this._searchInputChange(event.target.value)
  }

Will have to investigate if any issue with nested setState calls, but appears to works fine. Likely need to refactor this code to optimise and prevent unnecessary multiple renders

_selectItemByValues(value, keepControlOpen) {
   ....
    this.setState(newState, () => {
      if (!remainOpenAfterSelection) {
        this._closeOnKeypress();
      } else {
        this._updateFocusedId(parseInt(this.lastUserSelectedOption.getAttribute('data-option-index'), 10));
      }
      if (this.props.searchable && this.props.clearSearchOnSelection) {
        this._searchInputChange('')
      }
      this._broadcastChange();
    });

Modified calls to ajaxDataFetch and pageDataFetch handlers to pass searchString

  _fetchDataViaAjax() {
    const self = this;
    this.doneAjaxFetch = true
    this.props.ajaxDataFetch(this.state.rawDataSource,this.state.searchString).then((dataSourceFromAjax) => {
  ...
TimNZ commented 6 years ago

I've removed the 'ajaxSearchDelay' prop.

Makes sense to leave throttling to the ajaxDataFetch/pageDataFetch handlers and not be opinionated about it in ReactSuperSelect.

alsoscotland commented 6 years ago

@TimNZ thanks for working on this. changes look good. Have you verified nothing fails in the existing test suite? If not, please make a PR and I can check some things out.

TimNZ commented 6 years ago

Should do by end of week.

TimNZ commented 6 years ago

This test already fails on unmodified RSS 'trigger value display will show placeholder if provided'

If you want to have a play: https://github.com/TimNZ/react-super-select/tree/ajax-search

Still got to add tests.

TimNZ commented 6 years ago

Any issue with changing the lodash imports to direct versions?

e.g.

import {bindAll} from 'lodash'

->

import bindAll from 'lodash/bindAll'

Saves 100k in production build as lodash is a big dist