appbaseio / reactivesearch

Search UI components for React and Vue
https://opensource.appbase.io/reactivesearch
Apache License 2.0
4.9k stars 467 forks source link

Enter key in DataSearch should ignore debounce value #994

Closed joepio closed 4 years ago

joepio commented 5 years ago

Issue Type: I think this classifies as a bug

Platform: Web

Description: The default debounce value of 0 sends a lot of requests to my Elastic backend, which makes my app laggy, so I turned up the Debounce value to 500ms. However, this delays every single search request - also if the user presses enter.

I believe the correct behavior should be: only debounce for onChange events.

Reactivesearch version: 3.0.0-rc.6

Browser: all

joepio commented 5 years ago

Perhaps related: it would be nice to have a prop that determines that search is only triggered when the enter key is pressed. Especially for big, slow elastic instances this is really useful.

jyash97 commented 5 years ago

Hey @joepio You can use controlled behavior of DataSearch and with triggerQuery you can control the triggering of search. Here is the example: https://codesandbox.io/s/datasearch-v56r3

Hope this helps!

joepio commented 5 years ago

Thanks, @jyash97!

However, this seems to require that the value prop is managed by the external component state. If I add onKeyPress={handleKey}, the query is triggered but is set to an empty string.

This also causes the autosearch (which uses debounce) not to work at all.

import * as React from "react";
import { DataSearch } from "@appbaseio/reactivesearch";
import { ids } from "../helpers";
const fields = ["text", "title", "description", "name"];

const SearchBar: React.FunctionComponent = () => {
  const [query, setQuery] = React.useState<string>("");

  const handleKey = (e:KeyboardEvent, triggerQuery: Function) => {
    if (e.key === "Enter") {
      triggerQuery();
    }
  };

  return (
    <DataSearch
      autoFocus
      className="SearchBar"
      componentId={ids.searchbox}
      debounce={1200}
      onKeyPress={handleKey}
      onChange={setQuery}
      showFilter={false}
      dataField={fields}
      autosuggest={false}
      placeholder="Zoeken..."
      value={query}
      URLParams={true}
    />
  );
};

export default SearchBar;

I think the expected default behavior is that the DataSearch component executes triggerQuery when the enter key is pressed.

jyash97 commented 5 years ago

@joepio I didn't get the issue you are facing. So is debounce affecting even when you use triggerQuery?

Also on other note, don't pass setQuery directly to onChange of DataSearch it will give console errors as we pass triggerQuery as a second argument to onChange.

Here is the updated code sandbox using hooks: https://codesandbox.io/s/datasearch-v56r3

joepio commented 5 years ago

Hi @jyash97,

The problem in my previous comment, is that search does not trigger at all, unless I use Enter. Debounce has no effect.

Regarding onChange: triggerQuery is not passed to onChange:

<Input
  className={getClassName(this.props.innerClass, 'input') || null}
  placeholder={this.props.placeholder}
  value={this.state.currentValue ? this.state.currentValue : ''}
  onChange={this.onInputChange}
  onBlur={this.withTriggerQuery(this.props.onBlur)}
  onFocus={this.withTriggerQuery(this.props.onFocus)}
  onKeyPress={this.withTriggerQuery(this.props.onKeyPress)}
  onKeyDown={this.withTriggerQuery(this.props.onKeyDown)}
  onKeyUp={this.withTriggerQuery(this.props.onKeyUp)}
  autoFocus={this.props.autoFocus}
  iconPosition={this.props.iconPosition}
  showIcon={this.props.showIcon}
  showClear={this.props.showClear}
  themePreset={themePreset}
/>

Perhaps if triggerQuery is passed to onChange, I can work around the issue by manually calling triggerQuery.

jyash97 commented 5 years ago

triggerQuery is passed as second argument you can check the docs for onChange prop in DataSearch component.

joepio commented 5 years ago

It's in the docs, but not in the sourcecode

jyash97 commented 5 years ago

Check this https://github.com/appbaseio/reactivesearch/blob/7b8205fb05b66ceb07bf9bedd2a0ca6a261525e4/packages/web/src/components/search/DataSearch.js#L508

joepio commented 5 years ago

You're right, @jyash97 , sorry.

Now I can call triggerQuery using a custom onChange prop. I can set a timer and use the useState hook to pass the timer instance to the onKeyDown handler, so it does not trigger when the user presses enter.

import * as React from "react";
import { DataSearch } from "@appbaseio/reactivesearch";
import { ids } from "../helpers";
import { keyMap } from "../helpers/keyMap";

const fields = ["text", "title", "description", "name"];

const SearchBar: React.FunctionComponent = () => {
  const [query, setQuery] = React.useState<string>("");
  const [timer, setTimer] = React.useState<number>();

  React.useEffect(
    () => () => {
      // When the component unmounts, remove the timer.
      clearTimeout(timer)
    },
    []
  );

  const handleChange = (value: string, triggerQuery: Function) => {
    setQuery(value);
    // Set a timer for debouncing, if it's passed, call triggerQuery.
    setTimer(setTimeout(triggerQuery, 1000));
  };

  const handleKey = (e: KeyboardEvent, triggerQuery: Function) => {
    if (e.key === "Enter") {
      triggerQuery();
      // Reset the timer for debouncing.
      clearTimeout(timer);
    }
  };

  return (
    <DataSearch
      className="SearchBar"
      componentId={ids.searchbox}
      // Debounce is no longer functional
      // debounce={3000}
      onKeyPress={handleKey}
      onChange={handleChange}
      showFilter={false}
      dataField={fields}
      autosuggest={false}
      // This is required to manually set, because otherwise triggerQuery sets an empty string. 
      value={query}
      URLParams={true}
    />
  );
};

export default SearchBar;

I think this is the behavior that most users would expect: the enter key triggers the search engine and ignores the debounce value. Unfortunately, this behavior currently requires quite a bit of boilerplate code - the handleChange / handleKey functions, managing state of the value prop, managing the setTimeout.

Anyway, thanks for the help and thanks for this awesome library. My project could not have been realized without you guys.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

andymed-jlp commented 3 years ago

Thanks for the example @joepio - worked a treat. To limit the boilderplate I packaged it up as a component so I can simply use it as a drop in replacement for DataSearch:

import { useState, useEffect } from 'react';
import { DataSearch } from '@appbaseio/reactivesearch';

const DataSearchWithEnter = ({ debounce = 1000, ...props }) => {
  const [query, setQuery] = useState('');
  const [timer, setTimer] = useState();

  useEffect(
    () => () => {
      clearTimeout(timer);
    },
    [timer],
  );

  const handleChange = (value, triggerQuery) => {
    setQuery(value);
    setTimer(setTimeout(triggerQuery, debounce));
  };

  const handleKey = (e, triggerQuery) => {
    if (e.key === 'Enter') {
      triggerQuery();
      clearTimeout(timer);
    }
  };

  return <DataSearch {...props} value={query} onKeyPress={handleKey} onChange={handleChange} />;
};

export default DataSearchWithEnter;

I put a default of 1000 for the debounce - but that's not required, just was to make my life easier! :-)