lovasoa / react-contenteditable

React component for a div with editable contents
http://npmjs.com/package/react-contenteditable
Apache License 2.0
1.63k stars 218 forks source link

Certain EventHandler props are not updated on rerender #164

Open jhanschoo opened 5 years ago

jhanschoo commented 5 years ago

The cause of this bug is related to #161. In the following minimal example,

import React, { Component, ChangeEvent, KeyboardEvent } from 'react';
import ContentEditable from 'react-contenteditable';

class TestContentEditable extends Component {
  public readonly state = {
    html: 'text',
  };

  public readonly render = () => {
    const { html } = this.state;
    const handleChange = (e: ChangeEvent<HTMLInputElement>) => {
      console.log(html);
      console.log(this.state.html);
      this.setState({ html: e.target.value });
    }
    const handleKeyPress = (e: KeyboardEvent<HTMLDivElement>) => {
      const { key } = e;
      if (key === 'Enter') {
        e.preventDefault();
        console.log(html);
        console.log(this.state.html);
      }
    };
    return (
      <ContentEditable
        html={html}
        onChange={handleChange}
        onKeyDown={handleKeyPress}
      />
    );
  }
}
export default TestContentEditable;

we should expect the output of console.log(html); in handleKeyPress to be equal to console.log(this.state.html);. However, it is not, and the old handleKeyPress handler is used that sees the initial html value in its context. On the other hand, onChange is correctly updated.

bravo-kernel commented 5 years ago

Could anybody confirm this is actually/still a problem? I'm having similar issues with props used within onKeyDown handler are not being updated. If this goes against intended use of this library it would be good to know.

FWIW my current workaround is setting key={Date()} but I noticed setting a key with https://github.com/dylang/shortid works as well.

jhanschoo commented 5 years ago

Well, just try the minimal example...

bravo-kernel commented 5 years ago

Maybe I should have formulated my question differently.

Is this really a bug/missing feature or are we expected to solve this using different solutions?

The reason I was asking is because I found two workarounds which made me wonder if my original approach (same as your example) was even intended to work:

  1. setting key
  2. using a ref
jhanschoo commented 5 years ago

Ah, I see. In your case your workarounds force the component to be reinstantiated every refresh? The bug is not expected behavior of React components so if this workaround fixes the issue without introducing unexpected behavior as in #161 I suppose it should be implemented in the package itself.

bravo-kernel commented 5 years ago

Basically yes, I have a prop called stateValue. Inside the keypress handler function I use that prop to determine if user-input matches state-value. If it does not I do two actions:

  1. update ContentEditable value
  2. update state

Component then gets refreshed, prop stateValue gets the updated state-value. After that, the cycle continues.

lankovova commented 4 years ago

Any progress on this one? I am facing the same problem with both onKeyUp and onKeyDown events.

lovasoa commented 4 years ago

I'd be ready to review a PR fixing this ! If you do a pull request, please include tests demonstrating the fix.

archywillhe commented 4 years ago

hmm I'm still running into this.. And gosh I'm working with a few libs took me a while to realise this is what causes the bug

dilizarov commented 2 years ago

I think I'm running into this years later...

fake364 commented 1 year ago

I've got this issue, I have initial state every time event triggered , It looked like code works against javascript usual behaviour ) I see that lib is abandoned, so I will remove it from my repo then