material-components / material-components-web-react

Material Components for React (MDC React)
MIT License
2.02k stars 227 forks source link

Feature Request: Define onChangeComplete callback property in <Input> (<TextField>) #958

Closed poying closed 5 years ago

poying commented 5 years ago

onChange will be called whenever the input value is changed and cause potential performance issue, so users need to debounce onChange function. I think it is better to do it in <Input> then let users do it by themselves, because it is pretty annoying to users.

So I propose to define a new callback property, onChangeComplete, which will be called when user stop typing. It works just like debounced onChange function.

I can implement this if you guys like the idea.

moog16 commented 5 years ago

The onChange method is supposed to work like it does with the "native" React <input /> element. I believe that is how it behaves currently.

When would you call the onChangeComplete? I think this isn't a common pattern. Have you actually noticed any performance issues?

poying commented 5 years ago

@moog16

Here are some cases:

  1. Send http request when input value is changed (like search input). In this case, if we don't debounce onChange, it will send lots of unnecessary requests and cause performance issue. What we really need is just the last request.
  2. Large form. In this case, rendering process is really expensive. If we re-render the form whenever the input value is changed, users will notice that the newest input value is delayed to display.

There are lots of articles and questions talk about this on the web.

moog16 commented 5 years ago

The issue is knowing what the the last "change" event has happened. How do you accurately determine when the user has stopped typing?

poying commented 5 years ago

@moog16

We can't accurately determine when the last change event has happened. The practical way is using setTimeout to delay callback invocation, and abort it if another event happened before the callback actually be invoked.

Here is the example code:

const debounce = (fn, timeout=300) => {
  let timer;
  return (...args) => {
    clearTimeout(timer);
    timer = setTimeout(() => fn(...args), timeout);
  };
};

class Input extends Component {
  constructor(props) {
    super(props);
    this.changeComplete = debounce(this.props.onChangeComplete);
  }

  onChange(e) {
    this.props.onChange(e);
    this.changeComplete(e.target);
  }
}
moog16 commented 5 years ago

If we cannot do it accurately then that means there will be bugs. I think there is no way to please everyone without calling onChange every time a user types something. If we were to debounce, we might only see part of word being typed, which would make a bad user experience.

Therefore the debounce must be on the app developer (you & others).