From 972cc829584cc6203012738e09f842da5a492c01 Mon Sep 17 00:00:00 2001
From: Sanne de Vries <65487235+sanne-san@users.noreply.github.com>
Date: Mon, 9 Dec 2024 14:42:24 +0100
Subject: [PATCH] Prevented top-level comment input from closing when it has
 text (#21795)

REF https://linear.app/ghost/issue/PLG-298/

- When you're typing a comment, and exit the input field, it collapses into a non-editable state; you first have to click on it again to "open" the form. This means you can't select the text or instantly start typing again. When the input has a value, we should stop it from closing.
- added custom `useEditor` hook that wraps TipTap and exposes both the `editor` and `hasContent` props keeping logic out of the consuming components

---------

Co-authored-by: Kevin Ansfield <kevin@lookingsideways.co.uk>
---
 .../src/components/content/forms/EditForm.tsx | 13 ++---
 .../src/components/content/forms/Form.tsx     | 31 +++++-------
 .../src/components/content/forms/MainForm.tsx | 27 +++++------
 .../components/content/forms/ReplyForm.tsx    | 13 ++---
 apps/comments-ui/src/utils/editor.ts          |  7 ++-
 apps/comments-ui/src/utils/hooks.ts           | 38 ++++++++++++++-
 apps/comments-ui/test/e2e/content.test.ts     |  7 +--
 apps/comments-ui/test/e2e/editor.test.ts      |  1 +
 apps/comments-ui/test/e2e/main-form.test.ts   | 48 +++++++++++++++++++
 9 files changed, 130 insertions(+), 55 deletions(-)
 create mode 100644 apps/comments-ui/test/e2e/main-form.test.ts

diff --git a/apps/comments-ui/src/components/content/forms/EditForm.tsx b/apps/comments-ui/src/components/content/forms/EditForm.tsx
index 970a3f7207..11e36c2345 100644
--- a/apps/comments-ui/src/components/content/forms/EditForm.tsx
+++ b/apps/comments-ui/src/components/content/forms/EditForm.tsx
@@ -1,9 +1,8 @@
 import {Comment, OpenCommentForm, useAppContext} from '../../../AppContext';
 import {Form} from './Form';
-import {getEditorConfig} from '../../../utils/editor';
 import {isMobile} from '../../../utils/helpers';
-import {useCallback, useEffect} from 'react';
-import {useEditor} from '@tiptap/react';
+import {useCallback, useEffect, useMemo} from 'react';
+import {useEditor} from '../../../utils/hooks';
 
 type Props = {
     openForm: OpenCommentForm;
@@ -14,17 +13,15 @@ type Props = {
 const EditForm: React.FC<Props> = ({comment, openForm, parent}) => {
     const {dispatchAction, t} = useAppContext();
 
-    const config = {
+    const editorConfig = useMemo(() => ({
         placeholder: t('Edit this comment'),
         // warning: we cannot use autofocus on the edit field, because that sets
         // the cursor position at the beginning of the text field instead of the end
         autofocus: false,
         content: comment.html
-    };
+    }), [comment]);
 
-    const editor = useEditor({
-        ...getEditorConfig(config)
-    });
+    const {editor} = useEditor(editorConfig);
 
     // Instead of autofocusing, we focus and jump to end manually
     useEffect(() => {
diff --git a/apps/comments-ui/src/components/content/forms/Form.tsx b/apps/comments-ui/src/components/content/forms/Form.tsx
index 3f81a20eef..15c3c32abd 100644
--- a/apps/comments-ui/src/components/content/forms/Form.tsx
+++ b/apps/comments-ui/src/components/content/forms/Form.tsx
@@ -22,30 +22,26 @@ export type FormEditorProps = {
     submitText: React.ReactNode;
     submitSize: SubmitSize;
     openForm?: OpenCommentForm;
-    initialHasContent?: boolean;
 };
 
-export const FormEditor: React.FC<FormEditorProps> = ({comment, submit, progress, setProgress, close, isOpen, editor, submitText, submitSize, openForm, initialHasContent}) => {
+export const FormEditor: React.FC<FormEditorProps> = ({comment, submit, progress, setProgress, close, isOpen, editor, submitText, submitSize, openForm}) => {
     const labs = useLabs();
     const {dispatchAction, t} = useAppContext();
     let buttonIcon = null;
-    const [hasContent, setHasContent] = useState(initialHasContent || false);
 
     useEffect(() => {
-        if (editor) {
+        if (editor && openForm) {
             const checkContent = () => {
-                const editorHasContent = !editor.isEmpty;
-                setHasContent(editorHasContent);
+                const hasUnsavedChanges = comment && openForm.type === 'edit' ?
+                    editor.getHTML() !== comment.html :
+                    !editor.isEmpty;
 
-                if (openForm) {
-                    const hasUnsavedChanges = comment && openForm.type === 'edit' ? editor.getHTML() !== comment.html : editorHasContent;
-
-                    // avoid unnecessary state updates to prevent infinite loops
-                    if (openForm.hasUnsavedChanges !== hasUnsavedChanges) {
-                        dispatchAction('setCommentFormHasUnsavedChanges', {id: openForm.id, hasUnsavedChanges});
-                    }
+                // avoid unnecessary state updates to prevent infinite loops
+                if (openForm.hasUnsavedChanges !== hasUnsavedChanges) {
+                    dispatchAction('setCommentFormHasUnsavedChanges', {id: openForm.id, hasUnsavedChanges});
                 }
             };
+
             editor.on('update', checkContent);
             editor.on('transaction', checkContent);
 
@@ -152,7 +148,7 @@ export const FormEditor: React.FC<FormEditorProps> = ({comment, submit, progress
                     <button
                         className={`flex w-auto items-center justify-center ${submitSize === 'medium' && 'sm:min-w-[100px]'} ${submitSize === 'small' && 'sm:min-w-[64px]'} h-[40px] rounded-md bg-[var(--gh-accent-color)] px-3 py-2 text-center font-sans text-base font-medium text-white outline-0 transition-colors duration-200 hover:brightness-105 disabled:bg-black/5 disabled:text-neutral-900/30 sm:text-sm dark:disabled:bg-white/15 dark:disabled:text-white/35`}
                         data-testid="submit-form-button"
-                        disabled={!hasContent}
+                        disabled={!editor || editor.isEmpty}
                         type="button"
                         onClick={submitForm}
                     >
@@ -193,6 +189,7 @@ const FormHeader: React.FC<FormHeaderProps> = ({show, name, expertise, replyingT
 
     return (
         <Transition
+            data-testid="form-header"
             enter="transition duration-500 delay-100 ease-in-out"
             enterFrom="opacity-0 -translate-x-2"
             enterTo="opacity-100 translate-x-0"
@@ -258,9 +255,6 @@ const Form: React.FC<FormProps> = ({
     const [progress, setProgress] = useState<Progress>('default');
     const formEl = useRef(null);
 
-    // Initialize hasContent to true if we're editing an existing comment
-    const initialHasContent = openForm?.type === 'edit' && !!comment?.html;
-
     const memberName = member?.name ?? comment?.member?.name;
 
     if (progress === 'sending' || (memberName && isAskingDetails)) {
@@ -295,7 +289,6 @@ const Form: React.FC<FormProps> = ({
                 close={close}
                 comment={comment}
                 editor={editor}
-                initialHasContent={initialHasContent}
                 isOpen={isOpen}
                 openForm={openForm}
                 progress={progress}
@@ -328,7 +321,7 @@ const FormWrapper: React.FC<FormWrapperProps> = ({
 }) => {
     const {member, dispatchAction} = useAppContext();
     const labs = useLabs();
-    
+
     const memberName = member?.name ?? comment?.member?.name;
     const memberExpertise = member?.expertise ?? comment?.member?.expertise;
 
diff --git a/apps/comments-ui/src/components/content/forms/MainForm.tsx b/apps/comments-ui/src/components/content/forms/MainForm.tsx
index 95a130da35..038d200b45 100644
--- a/apps/comments-ui/src/components/content/forms/MainForm.tsx
+++ b/apps/comments-ui/src/components/content/forms/MainForm.tsx
@@ -1,9 +1,8 @@
-import React, {useCallback, useEffect, useRef} from 'react';
+import React, {useCallback, useEffect, useMemo, useRef} from 'react';
 import {Form, FormWrapper} from './Form';
-import {getEditorConfig} from '../../../utils/editor';
 import {scrollToElement} from '../../../utils/helpers';
 import {useAppContext} from '../../../AppContext';
-import {useEditor} from '@tiptap/react';
+import {useEditor} from '../../../utils/hooks';
 
 type Props = {
     commentsCount: number
@@ -12,14 +11,12 @@ type Props = {
 const MainForm: React.FC<Props> = ({commentsCount}) => {
     const {postId, dispatchAction, t} = useAppContext();
 
-    const config = {
+    const editorConfig = useMemo(() => ({
         placeholder: (commentsCount === 0 ? t('Start the conversation') : t('Join the discussion')),
         autofocus: false
-    };
+    }), [commentsCount]);
 
-    const editor = useEditor({
-        ...getEditorConfig(config)
-    }, [commentsCount]);
+    const {editor, hasContent} = useEditor(editorConfig);
 
     const submit = useCallback(async ({html}) => {
         // Send comment to server
@@ -28,7 +25,7 @@ const MainForm: React.FC<Props> = ({commentsCount}) => {
             status: 'published',
             html
         });
-    
+
         editor?.commands.clearContent();
     }, [postId, dispatchAction, editor]);
 
@@ -94,16 +91,16 @@ const MainForm: React.FC<Props> = ({commentsCount}) => {
         submit
     };
 
-    const isOpen = editor?.isFocused ?? false;
+    const isOpen = editor?.isFocused || hasContent;
 
     return (
         <div ref={formEl} className='px-3 pb-2 pt-3' data-testid="main-form">
             <FormWrapper editor={editor} isOpen={isOpen} reduced={false}>
-                <Form 
-                    editor={editor} 
-                    isOpen={isOpen} 
-                    reduced={false} 
-                    {...submitProps} 
+                <Form
+                    editor={editor}
+                    isOpen={isOpen}
+                    reduced={false}
+                    {...submitProps}
                 />
             </FormWrapper>
         </div>
diff --git a/apps/comments-ui/src/components/content/forms/ReplyForm.tsx b/apps/comments-ui/src/components/content/forms/ReplyForm.tsx
index a11e6156cb..90e5ae4cbc 100644
--- a/apps/comments-ui/src/components/content/forms/ReplyForm.tsx
+++ b/apps/comments-ui/src/components/content/forms/ReplyForm.tsx
@@ -1,9 +1,8 @@
 import {Comment, OpenCommentForm, useAppContext} from '../../../AppContext';
 import {Form, FormWrapper} from './Form';
-import {getEditorConfig} from '../../../utils/editor';
 import {isMobile, scrollToElement} from '../../../utils/helpers';
-import {useCallback} from 'react';
-import {useEditor} from '@tiptap/react';
+import {useCallback, useMemo} from 'react';
+import {useEditor} from '../../../utils/hooks';
 import {useRefCallback} from '../../../utils/hooks';
 
 type Props = {
@@ -15,14 +14,12 @@ const ReplyForm: React.FC<Props> = ({openForm, parent}) => {
     const {postId, dispatchAction, t} = useAppContext();
     const [, setForm] = useRefCallback<HTMLDivElement>(scrollToElement);
 
-    const config = {
+    const config = useMemo(() => ({
         placeholder: t('Reply to comment'),
         autofocus: true
-    };
+    }), []);
 
-    const editor = useEditor({
-        ...getEditorConfig(config)
-    });
+    const {editor} = useEditor(config);
 
     const submit = useCallback(async ({html}) => {
         // Send comment to server
diff --git a/apps/comments-ui/src/utils/editor.ts b/apps/comments-ui/src/utils/editor.ts
index dc403d7108..0172b6b878 100644
--- a/apps/comments-ui/src/utils/editor.ts
+++ b/apps/comments-ui/src/utils/editor.ts
@@ -7,7 +7,12 @@ import Placeholder from '@tiptap/extension-placeholder';
 import Text from '@tiptap/extension-text';
 import {EditorOptions} from '@tiptap/core';
 
-export function getEditorConfig({placeholder, autofocus = false, content = ''}: {placeholder: string; autofocus?: boolean; content?: string}): Partial<EditorOptions> {
+export type CommentsEditorConfig = {
+    placeholder: string;
+    autofocus?: boolean;
+    content?: string;
+}
+export function getEditorConfig({placeholder, autofocus = false, content = ''}: CommentsEditorConfig): Partial<EditorOptions> {
     return {
         extensions: [
             Document,
diff --git a/apps/comments-ui/src/utils/hooks.ts b/apps/comments-ui/src/utils/hooks.ts
index 264ac05feb..42aff108b7 100644
--- a/apps/comments-ui/src/utils/hooks.ts
+++ b/apps/comments-ui/src/utils/hooks.ts
@@ -1,4 +1,6 @@
-import React, {useCallback, useEffect, useMemo, useRef} from 'react';
+import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
+import {CommentsEditorConfig, getEditorConfig} from './editor';
+import {Editor, useEditor as useTiptapEditor} from '@tiptap/react';
 import {formatRelativeTime} from './helpers';
 import {useAppContext} from '../AppContext';
 
@@ -57,3 +59,37 @@ export function useRelativeTime(dateString: string) {
         return formatRelativeTime(dateString, t);
     }, [dateString]);
 }
+
+export function useEditor(editorConfig: CommentsEditorConfig, initialHasContent = false): {editor: Editor | null, hasContent: boolean} {
+    const [hasContent, setHasContent] = useState(initialHasContent);
+
+    const _editorConfig = useMemo(() => ({
+        ...getEditorConfig(editorConfig)
+    }), [editorConfig]);
+
+    const editor = useTiptapEditor(_editorConfig, [_editorConfig]);
+
+    useEffect(() => {
+        if (editor) {
+            const checkContent = () => {
+                const editorHasContent = !editor.isEmpty;
+                setHasContent(editorHasContent);
+            };
+
+            editor.on('update', checkContent);
+            editor.on('transaction', checkContent);
+
+            checkContent();
+
+            return () => {
+                editor.off('update', checkContent);
+                editor.off('transaction', checkContent);
+            };
+        }
+    }, [editor]);
+
+    return {
+        editor,
+        hasContent
+    };
+}
diff --git a/apps/comments-ui/test/e2e/content.test.ts b/apps/comments-ui/test/e2e/content.test.ts
index 659f4ff2e5..b3bce44d92 100644
--- a/apps/comments-ui/test/e2e/content.test.ts
+++ b/apps/comments-ui/test/e2e/content.test.ts
@@ -2,7 +2,8 @@ import {MockedApi, initialize} from '../utils/e2e';
 import {expect, test} from '@playwright/test';
 
 test.describe('Deleted and Hidden Content', async () => {
-    // This is actually handled by the API since it shouldn not longer return hidden or deleted comments for non-admins, but we still test the behaviour here.
+    // This is actually handled by the API since it should no longer return hidden
+    // or deleted comments for non-admins, but we still test the behaviour here.
     test('hides hidden and deleted comments for non admins', async ({page}) => {
         const mockedApi = new MockedApi({});
         mockedApi.addComment({
@@ -41,7 +42,7 @@ test.describe('Deleted and Hidden Content', async () => {
         await expect(comments).toHaveCount(3);
     });
 
-    test('hide and deleted comment shows with removed text when it has replies', async ({page}) => {
+    test('hidden and deleted comment shows with removed text when it has replies', async ({page}) => {
         const mockedApi = new MockedApi({});
         mockedApi.addComment({
             html: '<p>This is comment 1</p>'
@@ -106,7 +107,7 @@ test.describe('Deleted and Hidden Content', async () => {
         await expect (frame.getByText('This comment has been removed')).toBeVisible();
     });
 
-    test('hides replies thats hidden and deleted', async ({page}) => {
+    test('hides replies that are hidden or deleted', async ({page}) => {
         const mockedApi = new MockedApi({});
         mockedApi.addComment({
             html: '<p>This is comment 2</p>',
diff --git a/apps/comments-ui/test/e2e/editor.test.ts b/apps/comments-ui/test/e2e/editor.test.ts
index 38dfcc3825..205390c4ea 100644
--- a/apps/comments-ui/test/e2e/editor.test.ts
+++ b/apps/comments-ui/test/e2e/editor.test.ts
@@ -17,6 +17,7 @@ test.describe('Editor', async () => {
         const placeholderElement = editor.locator('[data-placeholder="Start the conversation"]');
         await expect(placeholderElement).toBeVisible();
     });
+
     test('Can comment on a post', async ({page}) => {
         const mockedApi = new MockedApi({});
         mockedApi.setMember({});
diff --git a/apps/comments-ui/test/e2e/main-form.test.ts b/apps/comments-ui/test/e2e/main-form.test.ts
new file mode 100644
index 0000000000..073b9e0810
--- /dev/null
+++ b/apps/comments-ui/test/e2e/main-form.test.ts
@@ -0,0 +1,48 @@
+import {MockedApi, initialize} from '../utils/e2e';
+import {buildMember} from '../utils/fixtures';
+import {expect, test} from '@playwright/test';
+
+test.describe('Main form', async () => {
+    let mockedApi: MockedApi;
+
+    async function initializeTest(page, options = {}) {
+        mockedApi = new MockedApi({});
+        mockedApi.setMember(buildMember({}));
+
+        return await initialize({
+            mockedApi,
+            page,
+            publication: 'Publisher Weekly',
+            ...options
+        });
+    }
+
+    test('hides header by default', async ({page}) => {
+        const {frame} = await initializeTest(page);
+        await expect(frame.locator('[data-testid="main-form"]')).toBeVisible();
+        await expect(frame.locator('[data-testid="main-form"] [data-testid="form-header"]')).not.toBeVisible();
+    });
+
+    test('shows header when focused', async ({page}) => {
+        const {frame} = await initializeTest(page);
+        await frame.locator('[data-testid="main-form"] [data-testid="form-editor"]').click();
+        await expect(frame.locator('[data-testid="main-form"] [data-testid="form-header"]')).toBeVisible();
+    });
+
+    test('hides header when blurred', async ({page}) => {
+        const {frame} = await initializeTest(page);
+        await frame.locator('[data-testid="main-form"] [data-testid="form-editor"]').click();
+        await expect(frame.locator('[data-testid="main-form"] [data-testid="form-header"]')).toBeVisible();
+        await page.locator('body').click();
+        await expect(frame.locator('[data-testid="main-form"] [data-testid="form-header"]')).not.toBeVisible();
+    });
+
+    test('keeps showing header when blurred with unpublished changes', async ({page}) => {
+        const {frame} = await initializeTest(page);
+        await frame.locator('[data-testid="main-form"] [data-testid="form-editor"]').click();
+        await expect(frame.locator('[data-testid="main-form"] [data-testid="form-header"]')).toBeVisible();
+        await frame.locator('[data-testid="main-form"] [data-testid="form-editor"]').pressSequentially('Some text');
+        await page.locator('body').click();
+        await expect(frame.locator('[data-testid="main-form"] [data-testid="form-header"]')).toBeVisible();
+    });
+});