From e0bc6174a78592f7dbd38420b9c0f65b9848d194 Mon Sep 17 00:00:00 2001
From: Joseph Garrone <joseph.garrone.gj@gmail.com>
Date: Wed, 29 May 2024 16:25:54 +0200
Subject: [PATCH] Fix edditing pushing to the api at every stroke

---
 src/components/TodoApp/Todo.tsx    | 34 ++++++++++++++++++++++++------
 src/components/TodoApp/TodoApp.tsx |  7 +++---
 src/tools/useConstCallback.ts      | 17 +++++++++++++++
 3 files changed, 48 insertions(+), 10 deletions(-)
 create mode 100644 src/tools/useConstCallback.ts

diff --git a/src/components/TodoApp/Todo.tsx b/src/components/TodoApp/Todo.tsx
index 82a4bae..09f768e 100644
--- a/src/components/TodoApp/Todo.tsx
+++ b/src/components/TodoApp/Todo.tsx
@@ -1,8 +1,9 @@
-import { memo, useState } from "react";
+import { memo, useState, useEffect, useReducer } from "react";
 import { tss } from "tss-react";
 import { fr } from "@codegouvfr/react-dsfr";
 import { Button } from "@codegouvfr/react-dsfr/Button";
 import Checkbox from "@mui/material/Checkbox";
+import { useConstCallback } from "tools/useConstCallback";
 
 export type Todo = {
     id: string;
@@ -19,12 +20,33 @@ type TodoProps = {
 };
 
 export const Todo = memo((props: TodoProps) => {
-    const { className, todo, onUpdateTodoText, onToggleTodo, onDeleteTodo } = props;
+    const { className, todo, onToggleTodo, onDeleteTodo } = props;
 
-    const [isEditing, setIsEditing] = useState(false);
+    // NOTE: Make sure it's not stale for when used in the reducer.
+    // We know it's constant because we also used useListCallbacks() in the parent component
+    // but this component is not supposed to be aware of that.
+    const onUpdateTodoText = useConstCallback(props.onUpdateTodoText);
+
+    const [isEditing, setIsEditing] = useReducer((isEditing: boolean, isEditing_new: boolean) => {
+        if (isEditing_new === isEditing) {
+            return isEditing;
+        }
+
+        if (!isEditing_new) {
+            onUpdateTodoText(text);
+        }
+
+        return isEditing_new;
+    }, false);
 
     const { classes, cx } = useStyles({ isEditing });
 
+    const [text, setText] = useState(todo.text);
+
+    useEffect(() => {
+        setText(todo.text);
+    }, [todo.text]);
+
     return (
         <div className={cx(classes.root, className)}>
             <Checkbox checked={todo.isDone} onChange={() => onToggleTodo()} />
@@ -33,8 +55,8 @@ export const Todo = memo((props: TodoProps) => {
                 {isEditing ? (
                     <input
                         className={cx(fr.cx("fr-input"), classes.input)}
-                        value={todo.text}
-                        onChange={e => onUpdateTodoText(e.target.value)}
+                        value={text}
+                        onChange={e => setText(e.target.value)}
                         onBlur={() => setIsEditing(false)}
                     />
                 ) : (
@@ -44,7 +66,7 @@ export const Todo = memo((props: TodoProps) => {
 
             <div className={classes.buttonsWrapper}>
                 <Button
-                    iconId="ri-pencil-line"
+                    iconId={isEditing ? "ri-check-line" : "ri-pencil-line"}
                     onClick={() => setIsEditing(!isEditing)}
                     priority="secondary"
                     title="Edit"
diff --git a/src/components/TodoApp/TodoApp.tsx b/src/components/TodoApp/TodoApp.tsx
index eb91335..4efbd53 100644
--- a/src/components/TodoApp/TodoApp.tsx
+++ b/src/components/TodoApp/TodoApp.tsx
@@ -17,10 +17,6 @@ export function TodoApp(props: Props) {
 
     const { classes, cx } = useStyles();
 
-    const getOnUpdateTodoText = useListCallbacks(([todoId]: [string], [text]: [string]) =>
-        onUpdateTodoText(todoId, text)
-    );
-
     /*
     If we use this custom hook instead of just doing:  
       onToggleTodo={()=> onToggleTodo(todo.id)}
@@ -34,6 +30,9 @@ export function TodoApp(props: Props) {
 
     Note: This is the state of the art for React 18. React 19 shuffles the deck with it's pre-compiler.  
     */
+    const getOnUpdateTodoText = useListCallbacks(([todoId]: [string], [text]: [string]) =>
+        onUpdateTodoText(todoId, text)
+    );
     const getOnToggleTodo = useListCallbacks(([todoId]: [string]) => onToggleTodo(todoId));
     const getOnDeleteTodo = useListCallbacks(([todoId]: [string]) => onDeleteTodo(todoId));
 
diff --git a/src/tools/useConstCallback.ts b/src/tools/useConstCallback.ts
new file mode 100644
index 0000000..da58134
--- /dev/null
+++ b/src/tools/useConstCallback.ts
@@ -0,0 +1,17 @@
+import { useRef, useState } from "react";
+import { Parameters } from "tsafe/Parameters";
+
+/** https://stackoverflow.com/questions/65890278/why-cant-usecallback-always-return-the-same-ref */
+export function useConstCallback<T extends ((...args: any[]) => unknown) | undefined | null>(
+    callback: NonNullable<T>
+): T {
+    const callbackRef = useRef<typeof callback>(null as any);
+
+    callbackRef.current = callback;
+
+    return useState(
+        () =>
+            (...args: Parameters<T>) =>
+                callbackRef.current(...args)
+    )[0] as T;
+}