Merge pull request #6102 from kl-nevermore/6067

Fix #6067: Editor: onTextChange handler does not reflect updated props
primefaces · Mar 18, 2024 · f9d08c3 · f9d08c3
2 parents 98077b2 + d36dbde
commit f9d08c3
Show file tree

Hide file tree

Showing 2 changed files with 163 additions and 68 deletions.
diff --git a/components/lib/editor/Editor.js b/components/lib/editor/Editor.js
@@ -29,6 +29,8 @@ export const Editor = React.memo(
         const quill = React.useRef(null);
         const isQuillLoaded = React.useRef(false);
 
+        const [quillCreated, setQuillCreated] = React.useState(false);
+
         useMountEffect(() => {
             if (!isQuillLoaded.current) {
                 const configuration = {
@@ -44,94 +46,109 @@ export const Editor = React.memo(
 
                 if (QuillJS) {
                     // GitHub #3097 loaded by script only
-                    quill.current = new Quill(contentRef.current, configuration);
-                    initQuill();
-
-                    if (quill.current && quill.current.getModule('toolbar')) {
-                        props.onLoad && props.onLoad(quill.current);
-                    }
+                    initQuill(new Quill(contentRef.current, configuration));
                 } else {
-                    import('quill')
-                        .then((module) => {
-                            if (module && DomHandler.isExist(contentRef.current)) {
-                                if (module.default) {
-                                    // webpack
-                                    quill.current = new module.default(contentRef.current, configuration);
-                                } else {
-                                    // parceljs
-                                    quill.current = new module(contentRef.current, configuration);
-                                }
-
-                                initQuill();
-                            }
-                        })
-                        .then(() => {
-                            if (quill.current && quill.current.getModule('toolbar')) {
-                                props.onLoad && props.onLoad(quill.current);
+                    import('quill').then((module) => {
+                        if (module && DomHandler.isExist(contentRef.current)) {
+                            let quillInstance;
+
+                            if (module.default) {
+                                // webpack
+                                quillInstance = new module.default(contentRef.current, configuration);
+                            } else {
+                                // parceljs
+                                quillInstance = new module(contentRef.current, configuration);
                             }
-                        });
+
+                            initQuill(quillInstance);
+                        }
+                    });
                 }
 
                 isQuillLoaded.current = true;
             }
         });
 
-        const initQuill = () => {
-            if (props.value) {
-                quill.current.setContents(quill.current.clipboard.convert(props.value));
+        const onTextChange = (delta, oldContents, source) => {
+            let firstChild = contentRef.current.children[0];
+            let html = firstChild ? firstChild.innerHTML : null;
+            let text = quill.current.getText();
+
+            if (html === '<p><br></p>') {
+                html = null;
             }
 
-            quill.current.on('text-change', (delta, oldContents, source) => {
-                let firstChild = contentRef.current.children[0];
-                let html = firstChild ? firstChild.innerHTML : null;
-                let text = quill.current.getText();
+            // GitHub #2271 prevent infinite loop on clipboard paste of HTML
+            if (source === 'api') {
+                const htmlValue = contentRef.current.children[0];
+                const editorValue = document.createElement('div');
 
-                if (html === '<p><br></p>') {
-                    html = null;
-                }
+                editorValue.innerHTML = props.value || '';
 
-                // GitHub #2271 prevent infinite loop on clipboard paste of HTML
-                if (source === 'api') {
-                    const htmlValue = contentRef.current.children[0];
-                    const editorValue = document.createElement('div');
+                // this is necessary because Quill rearranged style elements
+                if (DomHandler.isEqualElement(htmlValue, editorValue)) {
+                    return;
+                }
+            }
 
-                    editorValue.innerHTML = props.value || '';
+            if (props.maxLength) {
+                const length = quill.current.getLength();
 
-                    // this is necessary because Quill rearranged style elements
-                    if (DomHandler.isEqualElement(htmlValue, editorValue)) {
-                        return;
-                    }
+                if (length > props.maxLength) {
+                    quill.current.deleteText(props.maxLength, length);
                 }
+            }
 
-                if (props.maxLength) {
-                    const length = quill.current.getLength();
+            if (props.onTextChange) {
+                props.onTextChange({
+                    htmlValue: html,
+                    textValue: text,
+                    delta: delta,
+                    source: source
+                });
+            }
+        };
 
-                    if (length > props.maxLength) {
-                        quill.current.deleteText(props.maxLength, length);
-                    }
-                }
+        const onSelectionChange = (range, oldRange, source) => {
+            if (props.onSelectionChange) {
+                props.onSelectionChange({
+                    range: range,
+                    oldRange: oldRange,
+                    source: source
+                });
+            }
+        };
 
-                if (props.onTextChange) {
-                    props.onTextChange({
-                        htmlValue: html,
-                        textValue: text,
-                        delta: delta,
-                        source: source
-                    });
-                }
-            });
-
-            quill.current.on('selection-change', (range, oldRange, source) => {
-                if (props.onSelectionChange) {
-                    props.onSelectionChange({
-                        range: range,
-                        oldRange: oldRange,
-                        source: source
-                    });
-                }
-            });
+        const initQuill = (quillInstance) => {
+            quill.current = quillInstance;
+
+            if (props.value) {
+                quill.current.setContents(quill.current.clipboard.convert(props.value));
+            }
+
+            setQuillCreated(true);
         };
 
+        useUpdateEffect(() => {
+            if (quillCreated) {
+                quill.current.on('text-change', onTextChange);
+                quill.current.on('selection-change', onSelectionChange);
+
+                return () => {
+                    quill.current.off('text-change', onTextChange);
+                    quill.current.off('selection-change', onSelectionChange);
+                };
+            }
+        });
+
+        useUpdateEffect(() => {
+            if (quillCreated) {
+                if (quill.current && quill.current.getModule('toolbar')) {
+                    props.onLoad && props.onLoad(quill.current);
+                }
+            }
+        }, [quillCreated]);
+
         useUpdateEffect(() => {
             if (quill.current && !quill.current.hasFocus()) {
                 props.value ? quill.current.setContents(quill.current.clipboard.convert(props.value)) : quill.current.setText('');

diff --git a/components/lib/editor/Editor.spec.js b/components/lib/editor/Editor.spec.js
@@ -0,0 +1,78 @@
+import { useState, useRef, StrictMode } from 'react';
+import '@testing-library/jest-dom';
+import { render, fireEvent, cleanup, waitFor } from '@testing-library/react';
+import { Editor } from './Editor';
+
+afterEach(cleanup);
+
+describe('Editor', () => {
+    // https://github.com/primefaces/primereact/issues/6067
+    test('onTextChange handler does not reflect updated props', async () => {
+        const consoleSpy = jest.spyOn(console, 'log').mockImplementation();
+
+        function BasicDemo() {
+            const [state, setState] = useState(1);
+
+            return (
+                <>
+                    <button
+                        data-testid="update-state"
+                        onClick={() => {
+                            setState((p) => ++p);
+                        }}
+                    >
+                        add
+                    </button>
+                    state is: <span data-testid="state">{state}</span>
+                    <BasicDemo1 state={state} />
+                </>
+            );
+        }
+
+        function BasicDemo1({ state }) {
+            const [text, setText] = useState('');
+
+            const ref = useRef();
+
+            return (
+                <div className="card">
+                    <button
+                        data-testid="update-editor"
+                        onClick={() => {
+                            ref.current.getQuill().setText('789');
+                        }}
+                    ></button>
+                    <Editor
+                        ref={ref}
+                        value={text}
+                        onTextChange={(e) => {
+                            // eslint-disable-next-line no-console
+                            console.log(`Editor. state is:${state}`);
+                            setText(e.htmlValue);
+                        }}
+                        onLoad={(q) => {
+                            q.setText('hi');
+                        }}
+                    />
+                </div>
+            );
+        }
+
+        const { getByTestId } = render(<BasicDemo />);
+
+        fireEvent.click(getByTestId('update-state'));
+        expect(getByTestId('state').textContent).toBe('2');
+
+        await waitFor(() => {
+            expect(consoleSpy).toHaveBeenCalledWith('Editor. state is:2');
+        });
+
+        fireEvent.click(getByTestId('update-state'));
+        fireEvent.click(getByTestId('update-editor'));
+        expect(getByTestId('state').textContent).toBe('3');
+        await waitFor(() => {
+            expect(consoleSpy).toHaveBeenCalledWith('Editor. state is:3');
+        });
+        consoleSpy.mockRestore();
+    });
+});