kaivi / ReactInlineEdit

Simple React component for in-place text editing and validation
MIT License
170 stars 90 forks source link

Fix rerender bug #11

Closed masad-frost closed 8 years ago

masad-frost commented 8 years ago

Fix bug introduced in #6.

Reproduce:

1- Set prop text="sometext" 2- Click on component to edit, type text and change it to "othertext". 3- Update any other prop, e.g. className, or any other reason componentWillReceiveProps is called 4- The text will return to being "sometext"

The component always updates state.text to be equal to prop.text whenever any prop is changed and the state.text was changed at any point. We should only update the text when we change the text prop itself.

cc @ccnokes

masad-frost commented 8 years ago

I should've noticed that when I saw the code, but it didn't do anything to my component initially because I my code looked something like this, and I think this is how most people (should?) use this component:

// react stuff
...
changeText = ({newText}) => {
  this.setState({ text: newText });
};
...
render() {
  return (
    <InlineEdit
      text={this.state.text}
      paramName="newText"
      change={this.changeText}/>
  );
}
...

In which case the component is fine, but in cases where your change function doesn't affect the text prop directly (i.e text="static string", just use it for initial/defaultText), and you leave the component itself manage the text on its own, the component is broken.

edit: here's a quick example

// react stuff
...
changeText = ({newText}) => {
  console.log(`new text is ${newText}`);
};
...
render() {
  return (
    <InlineEdit
      text="This is my default text, and i don't like placeholders"
      paramName="newText"
      change={this.changeText}/>
  );
}
...

Another use case is changing the text using redux or something directly without mutating the parent's state.

Sorry for the wall of text for changing 4 characters in code :(. Just wanted to make sure I get my point across

ccnokes commented 8 years ago

@masad-frost Thanks for fixing my bug :smile: Looks good to me. +1

kaivi commented 8 years ago

Thanks a lot, this is now merged.