react-contenteditable
react-contenteditable copied to clipboard
Issue with hooks and onBlur
Hey, very useful component, thanks!
I ran into an issue when trying to use this component in a functional component combined with useState. The issue is that not all prop changes lead to an update of the component, therefore an onBlur like in my example would return the old initial content
value.
The issue resides in the current implementation of shouldComponentUpdate
.
Please look at this example Codesandbox. I copied this component's current source code over there and just return true in the shouldComponentUpdate
and everything works fine. To see the issue, comment the return true and uncomment the original code. If you type something and look in the console, you'll see the following logs:
render log:
render log: a
render log: as
render log: asd
onBlur log:
To fix this, I'd suggest going with a return true or make it a PureComponent to make updates based on prop changes.
Maintainer edit
Short answer
Do this
const text = useRef('');
const handleChange = evt => {
text.current = evt.target.value;
};
const handleBlur = () => {
console.log(text.current);
};
return <ContentEditable
html={text.current}
onBlur={handleBlur}
onChange={handleChange} />
NOT THAT
const [text, setText] = useState('');
const handleChange = evt => {
setText(evt.target.value);
};
const handleBlur = () => {
console.log(text); // incorrect value
};
return <ContentEditable
html={text}
onBlur={handleBlur}
onChange={handleChange} />
Explanation
react-contenteditable has to prevent rendering (using shouldComponentUpdate
) very frequently. Otherwise, the caret would jump to the end of the editable element on every key stroke. With useState, you create a new onBlur event handler on every keystroke, but since the ContentEditable is preventing rendering, your event handlers are not taken into account on every keystroke, and it's the handler function that was creating the last time the component was rendered that gets called.
Added a PR in case you decide to go this way. I can't really see any downsides to this.
The deep equal check of styles might have some historic reasons that might make this a breaking change. So any input on why that check was part of the shouldComponentUpdate
function would be good.
I had a look at your codesandbox, and your change breaks the expected behavior of the component. You can't type anywhere in the editable component except at the end.
You're right. Didn't catch that.
I'll have another focused look at it.
+1 Inside onBlur my state seems to be reset to it's original value. My workaround is to use innerRef to read the value directly off the domElement
const [text, setText] = useState('');
const contentEditable = useRef();
const handleChange = evt => {
setText(evt.target.value);
};
const handleBlur = () => {
console.log(contentEditableRef.current.innerHTML); // Correct value
console.log(text); // Incorrect value
};
<ContentEditable
innerRef={contentEditableRef}
html={text}
onBlur={handleBlur}
onChange={handleChange} />
Same issue here, can't use your workaround though since I'm using component state.
The same thing happens with onKeyUp
and onKeyDown
(though not with onChange
).
Our ugly workaround is to make sure that something that shouldComponentUpdate
cares about changes on each render, in this case className
.
A pull request would be welcome :)
Dropping in to say that I just encountered this issue too.
If no fix is coming, perhaps an update to the README with a warning? I lost a lot of time on this issue before realizing that this component is (as far as I can tell) completely incompatible with react hooks.
Done.
https://github.com/lovasoa/react-contenteditable/pull/195
I updated the top post to give more context. What could be done to avoid every new user getting bitten by this is to detect the situation and print a message like updating event handlers after the element creation is not supported, see #161
@embiem
Explanation
react-contenteditable has to prevent rendering (using
shouldComponentUpdate
) very frequently. Otherwise, the caret would jump to the end of the editable element on every key stroke. With useState, you create a new onBlur event handler on every keystroke, but since the ContentEditable is preventing rendering, your event handlers are not taken into account on every keystroke, and it's the handler function that was creating the last time the component was rendered that gets called.
your explanation sounds like you should use the useCallback hook to wrap the handleChange function. correct me if I am wrong.
like this
const handleChange = useCallback(evt => { setText(evt.target.value); }, []);
The quoted text is by me, not embiem. The problem is not with the handleChange
function, that is correctly called on every change, but with handleBlur
. Every time you render your component, you create a new handleBur
instance (which closes over the current value of text
). But since the contenteditable cancels rendering, the event is not rebound to the new function.
We could implement callback function comparison in shouldComponentUpdate
, and then rerender on every callback change. Then, users could use useCallback
together with useState
without having to use useRef
. However, this would break the component for all users who currently rely on the component not rerendering (this would make the caret jump to the end of the element while editing).
Maybe the caret position can be saved and restored?
https://gist.github.com/timdown/244ae2ea7302e26ba932a43cb0ca3908 https://github.com/timdown/rangy
Hey all;
So I also hit this issue. It appears to be less about the "hooks" feature of React, and more about the paradigm that "hooks" operate within - i.e. the need to create new callback functions containing new scope closures.
The current design of this component, along with the usage of shouldComponentUpdate
does not play nicely with this paradigm. I haven't gone into the source in depth, and understand the introduction of shouldComponentUpdate
was to prevent the caret position from changing. However, the current design doesn't allow new instances of callback components to be passed down, therefore you can get into a case where an old callback instance is called with a stale closure scope, leading to difficult to debug issues and strange behaviour.
To try and alleviate this I created my own wrapping component which manages refs of each callback and then passes down the callbacks down.
I've had good results from this as a temporary measure. My longer term hope is that we can revisit the internal design of this component as a community and move it forward.
Here is my wrapping component (click to view)
// wrapped-content-editable.js
import React from 'react';
import ReactContentEditable from 'react-contenteditable';
export default function ContentEditable({
onChange,
onInput,
onBlur,
onKeyPress,
onKeyDown,
...props
}) {
const onChangeRef = React.useRef(onChange);
const onInputRef = React.useRef(onInput);
const onBlurRef = React.useRef(onBlur);
const onKeyPressRef = React.useRef(onKeyPress);
const onKeyDownRef = React.useRef(onKeyDown);
React.useEffect(() => {
onChangeRef.current = onChange;
}, [onChange]);
React.useEffect(() => {
onInputRef.current = onInput;
}, [onInput]);
React.useEffect(() => {
onBlurRef.current = onBlur;
}, [onBlur]);
React.useEffect(() => {
onKeyPressRef.current = onKeyPress;
}, [onKeyPress]);
React.useEffect(() => {
onKeyDownRef.current = onKeyDown;
}, [onKeyDown]);
return (
<ReactContentEditable
{...props}
onChange={
onChange
? (...args) => {
if (onChangeRef.current) {
onChangeRef.current(...args);
}
}
: undefined
}
onInput={
onInput
? (...args) => {
if (onInputRef.current) {
onInputRef.current(...args);
}
}
: undefined
}
onBlur={
onBlur
? (...args) => {
if (onBlurRef.current) {
onBlurRef.current(...args);
}
}
: undefined
}
onKeyPress={
onKeyPress
? (...args) => {
if (onKeyPressRef.current) {
onKeyPressRef.current(...args);
}
}
: undefined
}
onKeyDown={
onKeyDown
? (...args) => {
if (onKeyDownRef.current) {
onKeyDownRef.current(...args);
}
}
: undefined
}
/>
);
}
Yeah, I know that looks a bit verbose and repetitive, but I prefer the explicit simplicity. 😊
You can now write your code as described how not to do it in within the OP.
i.e. This is fine now:
function Demo() {
const [text, setText] = React.useState('Woot! Hooks working');
const handleChange = evt => {
setText(evt.target.value);
};
const handleBlur = () => {
console.log(text); // 👍 correct value
};
return (
<ContentEditable html={text} onBlur={handleBlur} onChange={handleChange} />
);
}
@ctrlplusb with your solution, trying to use innerRef
to programmatically give the focus to the content editable div element, totally break the behaviour, as you can see in the following demo : https://codesandbox.io/s/bold-surf-kilyp
Any tips to fix that ? :/
When I get the text inside the div, using the onChange event, I get the html formatted text, but not the original text entered by the user. Is there a way to get what the user has typed but not the html formatted text.
I know I can parse the text and remove the html tags, but I don't want to do that as users may type the html itself as the content
Hey all;
So I also hit this issue. It appears to be less about the "hooks" feature of React, and more about the paradigm that "hooks" operate within - i.e. the need to create new callback functions containing new scope closures.
The current design of this component, along with the usage of
shouldComponentUpdate
does not play nicely with this paradigm. I haven't gone into the source in depth, and understand the introduction ofshouldComponentUpdate
was to prevent the caret position from changing. However, the current design doesn't allow new instances of callback components to be passed down, therefore you can get into a case where an old callback instance is called with a stale closure scope, leading to difficult to debug issues and strange behaviour.To try and alleviate this I created my own wrapping component which manages refs of each callback and then passes down the callbacks down.
I've had good results from this as a temporary measure. My longer term hope is that we can revisit the internal design of this component as a community and move it forward.
Here is my wrapping component (click to view) You can now write your code as described how not to do it in within the OP.
i.e. This is fine now:
function Demo() { const [text, setText] = React.useState('Woot! Hooks working'); const handleChange = evt => { setText(evt.target.value); }; const handleBlur = () => { console.log(text); // 👍 correct value }; return ( <ContentEditable html={text} onBlur={handleBlur} onChange={handleChange} /> ); }
thank you for your help. i just had some problem with ref. passing ref like that doesn't work perfectly. accourding to react website forwadRef documentation, i used bellow and it solved my problem with ref:
import React, { forwardRef } from "react";
import ReactContentEditable from "react-contenteditable";
export default forwardRef(
({ onChange, onInput, onBlur, onKeyPress, onKeyDown, ...props }, ref) => {
const onChangeRef = React.useRef(onChange);
const onInputRef = React.useRef(onInput);
const onBlurRef = React.useRef(onBlur);
const onKeyPressRef = React.useRef(onKeyPress);
const onKeyDownRef = React.useRef(onKeyDown);
React.useEffect(() => {
onChangeRef.current = onChange;
}, [onChange]);
React.useEffect(() => {
onInputRef.current = onInput;
}, [onInput]);
React.useEffect(() => {
onBlurRef.current = onBlur;
}, [onBlur]);
React.useEffect(() => {
onKeyPressRef.current = onKeyPress;
}, [onKeyPress]);
React.useEffect(() => {
onKeyDownRef.current = onKeyDown;
}, [onKeyDown]);
return (
<ReactContentEditable
{...props}
innerRef={ref}
onChange={
onChange
? (...args) => {
if (onChangeRef.current) {
onChangeRef.current(...args);
}
}
: undefined
}
onInput={
onInput
? (...args) => {
if (onInputRef.current) {
onInputRef.current(...args);
}
}
: undefined
}
onBlur={
onBlur
? (...args) => {
if (onBlurRef.current) {
onBlurRef.current(...args);
}
}
: undefined
}
onKeyPress={
onKeyPress
? (...args) => {
if (onKeyPressRef.current) {
onKeyPressRef.current(...args);
}
}
: undefined
}
onKeyDown={
onKeyDown
? (...args) => {
if (onKeyDownRef.current) {
onKeyDownRef.current(...args);
}
}
: undefined
}
/>
);
}
);
now sending ref is like this :
function Demo() {
const contentRef = React.useRef(null);
const [text, setText] = React.useState('Woot! Hooks working');
const handleChange = evt => {
setText(evt.target.value);
};
const handleBlur = () => {
console.log(text); // 👍 correct value
};
return (
<ContentEditable ref={contentRef} html={text} onBlur={handleBlur} onChange={handleChange} />
);
}
You can use the following hook to reduce boilerplate
const useRefCallback = <T extends any[]>(
value: ((...args: T) => void) | undefined,
deps?: React.DependencyList
): ((...args: T) => void) => {
const ref = React.useRef(value);
React.useEffect(() => {
ref.current = value;
}, deps ?? [value]);
const result = React.useCallback((...args: T) => {
ref.current?.(...args);
}, []);
return result;
};
Usage in the wrapping component
import React from 'react';
import ReactContentEditable, { Props } from 'react-contenteditable';
const useRefCallback = <T extends any[]>(
value: ((...args: T) => void) | undefined,
deps?: React.DependencyList
): ((...args: T) => void) => {
const ref = React.useRef(value);
React.useEffect(() => {
ref.current = value;
}, deps ?? [value]);
const result = React.useCallback((...args: T) => {
ref.current?.(...args);
}, []);
return result;
};
export default function ContentEditable({
ref,
onChange,
onInput,
onBlur,
onKeyPress,
onKeyDown,
...props
}: Props) {
const onChangeRef = useRefCallback(onChange);
const onInputRef = useRefCallback(onInput);
const onBlurRef = useRefCallback(onBlur);
const onKeyPressRef = useRefCallback(onKeyPress);
const onKeyDownRef = useRefCallback(onKeyDown);
return (
<ReactContentEditable
{...props}
onChange={onChangeRef}
onInput={onInputRef}
onBlur={onBlurRef}
onKeyPress={onKeyPressRef}
onKeyDown={onKeyDownRef}
/>
);
}
If you don't want to copy the wrapping component, you can use useRefCallback
as a drop-in replacement for useCallback
:
function Demo() {
const [text, setText] = React.useState('Woot! Hooks working');
const handleChange = useRefCallback((evt) => {
setText(evt.target.value);
}, []);
const handleBlur = useRefCallback(() => {
console.log(text); // 👍 correct value
}, [text]);
return (
<ContentEditable html={text} onBlur={handleBlur} onChange={handleChange} />
);
}
Hey all;
So I also hit this issue. It appears to be less about the "hooks" feature of React, and more about the paradigm that "hooks" operate within - i.e. the need to create new callback functions containing new scope closures.
The current design of this component, along with the usage of
shouldComponentUpdate
does not play nicely with this paradigm. I haven't gone into the source in depth, and understand the introduction ofshouldComponentUpdate
was to prevent the caret position from changing. However, the current design doesn't allow new instances of callback components to be passed down, therefore you can get into a case where an old callback instance is called with a stale closure scope, leading to difficult to debug issues and strange behaviour.To try and alleviate this I created my own wrapping component which manages refs of each callback and then passes down the callbacks down.
I've had good results from this as a temporary measure. My longer term hope is that we can revisit the internal design of this component as a community and move it forward.
Here is my wrapping component (click to view) You can now write your code as described how not to do it in within the OP.
i.e. This is fine now:
function Demo() { const [text, setText] = React.useState('Woot! Hooks working'); const handleChange = evt => { setText(evt.target.value); }; const handleBlur = () => { console.log(text); // 👍 correct value }; return ( <ContentEditable html={text} onBlur={handleBlur} onChange={handleChange} /> ); }
typescript version of the wrapping component:
import React, {useEffect, useRef} from 'react';
import ReactContentEditable, {
ContentEditableEvent,
} from 'react-contenteditable';
interface ContentEditableProps {
onChange?: (event: ContentEditableEvent) => void;
onBlur?: (event: React.FormEvent<HTMLDivElement>) => void;
onInput?: (event: React.FormEvent<HTMLDivElement>) => void;
onKeyPress?: (event: React.KeyboardEvent<HTMLDivElement>) => void;
onKeyDown?: (event: React.KeyboardEvent<HTMLDivElement>) => void;
html: string;
}
export const ContentEditable: React.FC<ContentEditableProps> = ({
onChange,
onInput,
onBlur,
onKeyPress,
onKeyDown,
...props
}) => {
const onChangeRef = useRef(onChange);
const onInputRef = useRef(onInput);
const onBlurRef = useRef(onBlur);
const onKeyPressRef = useRef(onKeyPress);
const onKeyDownRef = useRef(onKeyDown);
useEffect(() => {
onChangeRef.current = onChange;
}, [onChange]);
useEffect(() => {
onInputRef.current = onInput;
}, [onInput]);
useEffect(() => {
onBlurRef.current = onBlur;
}, [onBlur]);
useEffect(() => {
onKeyPressRef.current = onKeyPress;
}, [onKeyPress]);
useEffect(() => {
onKeyDownRef.current = onKeyDown;
}, [onKeyDown]);
return (
<ReactContentEditable
{...props}
onChange={
onChange
? (...args) => {
if (onChangeRef.current) {
onChangeRef.current(...args);
}
}
: () => {}
}
onInput={
onInput
? (...args) => {
if (onInputRef.current) {
onInputRef.current(...args);
}
}
: undefined
}
onBlur={
onBlur
? (...args) => {
if (onBlurRef.current) {
onBlurRef.current(...args);
}
}
: undefined
}
onKeyPress={
onKeyPress
? (...args) => {
if (onKeyPressRef.current) {
onKeyPressRef.current(...args);
}
}
: undefined
}
onKeyDown={
onKeyDown
? (...args) => {
if (onKeyDownRef.current) {
onKeyDownRef.current(...args);
}
}
: undefined
}
/>
);
};
Hi, the documentation proposition for hook and class isn't working as I expected:
const text = useRef('');
const handleChange = evt => {
text.current = evt.target.value;
// /!\ Won't be colored !
// text.current = `<span style="color: red;"> ${evt.target.value}</span>`
// /!\ Completly ignored
// text.current = `override`
};
const handleBlur = () => {
console.log(text.current);
};
return <ContentEditable html={text.current} onBlur={handleBlur} onChange={handleChange} />
Using setText with useState work will color the text. But the caret will jump at the end of the text at each edit. Same caret issue when using the class component.
Seems to work fine with Hooks, am I missing something? https://codesandbox.io/s/shy-snow-rc1md6?file=/src/App.js
Seems to work fine with Hooks, am I missing something? https://codesandbox.io/s/shy-snow-rc1md6?file=/src/App.js
I think the issue lies in the number of renders that occur with each state change, and this behavior is normal. By using useRef, we can eliminate the 'extra' renders. I've added useRef to your example so you can visually see how it works: https://codesandbox.io/s/fragrant-water-7pg4zw?file=/src/App.js
I don't think the comment in the README is the best workaround. With useRef
, when you set text.current
, a rerender is not triggered. This is fine if you only want to read the current value, but if you do postprocessing/sanitization in onChange
, your new value is not immediately reflected.
I think @KevinSheedy's solution is right:
- keep using
useState
- store a
ref
to the ContentEditable - read the current text using
ref.current.innerHTML
I don't think the comment in the README is the best workaround. With
useRef
, when you settext.current
, a rerender is not triggered. This is fine if you only want to read the current value, but if you do postprocessing/sanitization inonChange
, your new value is not immediately reflected.I think @KevinSheedy's solution is right:
* keep using `useState` * store a `ref` to the ContentEditable * read the current text using `ref.current.innerHTML`
Yea! Alternatively:
- Get the value of the current input value from your onChange handler
- Pass that value around if needed.
const [text, setText] = useState();
const updateTextFn = (text) => {
// will use the current text value passed :)
}
const handleChange = evt => {
const currentVal= evt.target.value;
setText(currentVal);
updateTextFn(currentVal)
};