WickyNilliams / react-simple-colorpicker

Simple, composable, lightweight colorpicker for react
http://wicky.nillia.ms/react-simple-colorpicker
MIT License
46 stars 19 forks source link

Add optional isFinished handler #11

Closed joerter closed 8 years ago

joerter commented 8 years ago

Hi, thanks for your hard work on this component! I've been using it with great success, and have recently had a need to fire color changes when the user has finished choosing the color in addition to during dragging.

I accomplished this by updating the position in the stopUpdates function of DraggableMixin.js, and passing a boolean flag up the chain.

I don't really like the boolean flag method, so if you have an idea for a more elegant solution, that would be awesome. Or if you don't like this feature at all, that's fine too. I just thought I would share and see what your thoughts are.

WickyNilliams commented 8 years ago

Hey thanks for submitting the PR, and glad to hear that it's been working well for you.

I can see how this feature might be useful! I share your concern about the boolean flag, propagating it up the call stack is a little awkward. I think this feature would be better exposed as a callback, say onFinish. Users can then subscribe to onChange or onFinish or both, to suit their needs, and you wouldn't need to propagate a flag.

However, at the moment I don't want to add this feature directly to the component. Why? My ethos with this component has been to provide the absolute minimal set of functionality for a color picker, and that extension be achieved via composition rather than modification.

I think this PR shows that the color picker needs some drag start/end hooks, which would allow for this feature to be added easily. In lieu of those hooks, would this suffice for your situation: If no changes have happened in some timeout period, then invoke an onFinish callback with the latest color?

We create a new component that wraps the basic ColorPicker and bolt on that feature:

const MyColorPicker = React.createClass({

  getDefaultProps: function() {
    return {
      onFinish : function() {},
      timeout  : 250
    };
  },

  handleChange(color) {
    this.props.onChange(color);

    clearTimeout(this.timer);
    this.timer = setTimeout(this.props.onFinish, this.props.timeout, color);
  },

  render() {
    return (
      <ColorPicker {...this.props} onChange={this.handleChange} />
    );
  }
});

Super simple, plus configurable :) It also handles the case where someone has momentarily stopped dragging but hasn't lifted mouse/touch - it might be useful to know that color.

Hopefully this is helpful. I'll get around to adding some extra hooks in the near future.