react-contenteditable icon indicating copy to clipboard operation
react-contenteditable copied to clipboard

more explicit onChange

Open WendellLiu opened this issue 9 years ago • 3 comments

  1. onChange will take the innerHTML as argument
  2. modification was updated in README

WendellLiu avatar Sep 02 '16 01:09 WendellLiu

Personally, I think this would be a nicer API, though I understand if others disagree. The current function signature onChange({target: {value}}) puts the value at the same place as the original onChange event would, with the advantage of making it easier to use existing handler code. However, it is not an event in any other way, which makes it slightly confusing and inelegant.

It may be better to change the function name, e.g. to valueChanged or onContentChange or so, to not sound like the standard event. Also, thinking ahead, if at some point we do want to support reading the plain text (#35) rather than the html, we should think of a logical way to access either of them (onHtmlChange / onTextChange? onContentChange({html, text})?).

In any case, because it is an API-breaking change, it may not be worth changing unless we really know what we want, or if some other developments will require a breaking change at some moment anyway.

Treora avatar Sep 02 '16 09:09 Treora

  1. This is truely an API-breaking change. So this PR could be pending. It's fine.
  2. In my opinion, onChange should take event value or event itself but not a {target: value}. User get {target: value} can't use other event feature and can't use it explicitly. That's why I send this PR. But I guess that this design is for mock native event ?

WendellLiu avatar Sep 02 '16 09:09 WendellLiu

Blocked by https://github.com/lovasoa/react-contenteditable/pull/47

monolithed avatar Jan 23 '17 14:01 monolithed