0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-12-30 22:34:01 -05:00

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>
This commit is contained in:
Sanne de Vries 2024-12-09 14:42:24 +01:00 committed by GitHub
parent b25f42fec6
commit 972cc82958
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 130 additions and 55 deletions

View file

@ -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(() => {

View file

@ -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;

View file

@ -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>

View file

@ -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

View file

@ -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,

View file

@ -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
};
}

View file

@ -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>',

View file

@ -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({});

View file

@ -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();
});
});