Don't use stale readOnly prop. (Fix bug ianstormtaylor#3321)

kena0ki · Dec 25, 2019 · 566432e · 566432e
1 parent 22d9095
commit 566432e
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 39 deletions.
diff --git a/packages/slate-react/src/components/editable.tsx b/packages/slate-react/src/components/editable.tsx
@@ -125,40 +125,6 @@ export const Editable = (props: EditableProps) => {
     }
   })
 
-  // Attach a native DOM event handler for `selectionchange`, because React's
-  // built-in `onSelect` handler doesn't fire for all selection changes. It's a
-  // leaky polyfill that only fires on keypresses or clicks. Instead, we want to
-  // fire for any change to the selection inside the editor. (2019/11/04)
-  // https://github.com/facebook/react/issues/5785
-  useIsomorphicLayoutEffect(() => {
-    window.document.addEventListener('selectionchange', onDOMSelectionChange)
-
-    return () => {
-      window.document.removeEventListener(
-        'selectionchange',
-        onDOMSelectionChange
-      )
-    }
-  }, [])
-
-  // Attach a native DOM event handler for `beforeinput` events, because React's
-  // built-in `onBeforeInput` is actually a leaky polyfill that doesn't expose
-  // real `beforeinput` events sadly... (2019/11/04)
-  // https://github.com/facebook/react/issues/11211
-  useIsomorphicLayoutEffect(() => {
-    if (ref.current) {
-      // @ts-ignore The `beforeinput` event isn't recognized.
-      ref.current.addEventListener('beforeinput', onDOMBeforeInput)
-    }
-
-    return () => {
-      if (ref.current) {
-        // @ts-ignore The `beforeinput` event isn't recognized.
-        ref.current.removeEventListener('beforeinput', onDOMBeforeInput)
-      }
-    }
-  }, [])
-
   // Whenever the editor updates, make sure the DOM selection state is in sync.
   useIsomorphicLayoutEffect(() => {
     const { selection } = editor
@@ -354,9 +320,27 @@ export const Editable = (props: EditableProps) => {
         }
       }
     },
-    []
+    [readOnly]
   )
 
+  // Attach a native DOM event handler for `beforeinput` events, because React's
+  // built-in `onBeforeInput` is actually a leaky polyfill that doesn't expose
+  // real `beforeinput` events sadly... (2019/11/04)
+  // https://github.com/facebook/react/issues/11211
+  useIsomorphicLayoutEffect(() => {
+    if (ref.current) {
+      // @ts-ignore The `beforeinput` event isn't recognized.
+      ref.current.addEventListener('beforeinput', onDOMBeforeInput)
+    }
+
+    return () => {
+      if (ref.current) {
+        // @ts-ignore The `beforeinput` event isn't recognized.
+        ref.current.removeEventListener('beforeinput', onDOMBeforeInput)
+      }
+    }
+  }, [onDOMBeforeInput])
+
   // Listen on the native `selectionchange` event to be able to update any time
   // the selection changes. This is required because React's `onSelect` is leaky
   // and non-standard so it doesn't fire until after a selection has been
@@ -392,9 +376,25 @@ export const Editable = (props: EditableProps) => {
         }
       }
     }, 100),
-    []
+    [readOnly]
   )
 
+  // Attach a native DOM event handler for `selectionchange`, because React's
+  // built-in `onSelect` handler doesn't fire for all selection changes. It's a
+  // leaky polyfill that only fires on keypresses or clicks. Instead, we want to
+  // fire for any change to the selection inside the editor. (2019/11/04)
+  // https://github.com/facebook/react/issues/5785
+  useIsomorphicLayoutEffect(() => {
+    window.document.addEventListener('selectionchange', onDOMSelectionChange)
+
+    return () => {
+      window.document.removeEventListener(
+        'selectionchange',
+        onDOMSelectionChange
+      )
+    }
+  }, [onDOMSelectionChange])
+
   const decorations = decorate([editor, []])
 
   if (

diff --git a/site/examples/read-only.js b/site/examples/read-only.js
@@ -5,10 +5,29 @@ import { Slate, Editable, withReact } from 'slate-react'
 const ReadOnlyExample = () => {
   const [value, setValue] = useState(initialValue)
   const editor = useMemo(() => withReact(createEditor()), [])
+  const [readOnly, setReadOnly] = useState(true)
+  const [cnt, setCnt] = useState(0)
   return (
-    <Slate editor={editor} value={value} onChange={value => setValue(value)}>
-      <Editable readOnly placeholder="Enter some plain text..." />
-    </Slate>
+    <div>
+      <div>
+        Current readOnly is
+        <button onClick={() => setReadOnly(!readOnly)}>
+          {readOnly.toString()}
+        </button>
+        . onChange is called {cnt} times.
+        <hr />
+      </div>
+      <Slate
+        editor={editor}
+        value={value}
+        onChange={value => {
+          setValue(value)
+          setCnt(preCnt => preCnt + 1)
+        }}
+      >
+        <Editable readOnly={readOnly} placeholder="Enter some plain text..." />
+      </Slate>
+    </div>
   )
 }