From 70ab283686e6629b7f87c10c79aab207d566a0df Mon Sep 17 00:00:00 2001 From: Herman Wikner Date: Mon, 6 May 2024 13:58:45 +0200 Subject: [PATCH] fix(core): collapsed range decorations (#6568) * fix(portable-text-editor): fix loop issue with collapsed decorator * fix(portable-text-editor): remove unused hook dep * test(core): presence cursors * doc(portable-text-editor): add comment --------- Co-authored-by: Per-Kristian Nordnes --- .../src/editor/Editable.tsx | 6 +- .../PortableText/PresenceCursors.spec.tsx | 112 ++++++++++++++++++ .../PortableText/PresenceCursorsStory.tsx | 39 ++++++ .../tests/formBuilder/utils/TestForm.tsx | 34 ++++-- .../tests/formBuilder/utils/TestWrapper.tsx | 20 ++-- .../presence-cursors/UserPresenceCursor.tsx | 8 +- 6 files changed, 199 insertions(+), 20 deletions(-) create mode 100644 packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/PresenceCursors.spec.tsx create mode 100644 packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/PresenceCursorsStory.tsx diff --git a/packages/@sanity/portable-text-editor/src/editor/Editable.tsx b/packages/@sanity/portable-text-editor/src/editor/Editable.tsx index de2ddee1229..ca44394ce09 100644 --- a/packages/@sanity/portable-text-editor/src/editor/Editable.tsx +++ b/packages/@sanity/portable-text-editor/src/editor/Editable.tsx @@ -315,7 +315,7 @@ export const PortableTextEditable = forwardRef(function PortableTextEditable( // debug('Unsubscribing to changes$') sub.unsubscribe() } - }, [change$, restoreSelectionFromProps, syncRangeDecorations]) + }, [change$, restoreSelectionFromProps]) // Restore selection from props when it changes useEffect(() => { @@ -591,6 +591,10 @@ export const PortableTextEditable = forwardRef(function PortableTextEditable( const result = rangeDecorationState.filter((item) => { // Special case in order to only return one decoration for collapsed ranges if (SlateRange.isCollapsed(item)) { + // Collapsed ranges should only be decorated if they are on a block child level (length 2) + if (path.length !== 2) { + return false + } return Path.equals(item.focus.path, path) && Path.equals(item.anchor.path, path) } // Include decorations that either include or intersects with this path diff --git a/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/PresenceCursors.spec.tsx b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/PresenceCursors.spec.tsx new file mode 100644 index 00000000000..43601922fb8 --- /dev/null +++ b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/PresenceCursors.spec.tsx @@ -0,0 +1,112 @@ +/* eslint-disable max-nested-callbacks */ +import {expect, test} from '@playwright/experimental-ct-react' +import {type Page} from '@playwright/test' +import {type SanityDocument} from '@sanity/client' +import {type FormNodePresence} from 'sanity' + +import {testHelpers} from '../../../utils/testHelpers' +import {PresenceCursorsStory} from './PresenceCursorsStory' + +const TEXT = 'Hello, this is some text in the editor.' + +const DOCUMENT: SanityDocument = { + _id: '123', + _type: 'test', + _createdAt: new Date().toISOString(), + _updatedAt: new Date().toISOString(), + _rev: '123', + body: [ + { + _type: 'block', + _key: 'a', + children: [{_type: 'span', _key: 'a1', text: TEXT}], + markDefs: [], + }, + ], +} + +const offset1 = TEXT.indexOf('this is') +const offset2 = TEXT.indexOf('some text') + +const PRESENCE: FormNodePresence[] = [ + { + path: ['body', 'text'], + lastActiveAt: new Date().toISOString(), + sessionId: 'session-A', + selection: { + anchor: {offset: offset1, path: [{_key: 'a'}, 'children', {_key: 'a1'}]}, + focus: {offset: offset1, path: [{_key: 'a'}, 'children', {_key: 'a1'}]}, + backward: false, + }, + user: { + id: 'user-A', + displayName: 'User A', + }, + }, + { + path: ['body', 'text'], + lastActiveAt: new Date().toISOString(), + sessionId: 'session-B', + selection: { + anchor: {offset: offset2, path: [{_key: 'a'}, 'children', {_key: 'a1'}]}, + focus: {offset: offset2, path: [{_key: 'a'}, 'children', {_key: 'a1'}]}, + backward: false, + }, + user: { + id: 'user-B', + displayName: 'User B', + }, + }, +] + +async function getSiblingTextContent(page: Page) { + return await page.evaluate(() => { + const cursorA = document.querySelector('[data-testid="presence-cursor-User-A"]') + const cursorB = document.querySelector('[data-testid="presence-cursor-User-B"]') + + return { + cursorA: cursorA?.nextElementSibling?.textContent, + cursorB: cursorB?.nextElementSibling?.textContent, + } + }) +} + +test.describe('Portable Text Input', () => { + test.describe('Presence Cursors', () => { + test('should keep position when inserting text in the editor', async ({mount, page}) => { + const {getFocusedPortableTextEditor, insertPortableText} = testHelpers({page}) + + await mount() + + const editor$ = await getFocusedPortableTextEditor('field-body') + const $cursorA = editor$.getByTestId('presence-cursor-User-A') + const $cursorB = editor$.getByTestId('presence-cursor-User-B') + + await expect($cursorA).toBeVisible() + await expect($cursorB).toBeVisible() + + const siblingContentA = await getSiblingTextContent(page) + expect(siblingContentA.cursorA).toBe('this is ') + expect(siblingContentA.cursorB).toBe('some text in the editor.') + + await insertPortableText('INSERTED TEXT. ', editor$) + + // Make sure that the cursors keep their position after inserting text + const siblingContentB = await getSiblingTextContent(page) + expect(siblingContentB.cursorA).toBe('this is ') + expect(siblingContentB.cursorB).toBe('some text in the editor.') + }) + + test.skip('should keep position when deleting text in the editor', async () => { + // todo + }) + + test.skip('should keep position when pasting text i the editor', async () => { + // todo + }) + + test.skip('should change position when updating the selection in the editor', async () => { + // todo + }) + }) +}) diff --git a/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/PresenceCursorsStory.tsx b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/PresenceCursorsStory.tsx new file mode 100644 index 00000000000..20ffa35f4ed --- /dev/null +++ b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/PresenceCursorsStory.tsx @@ -0,0 +1,39 @@ +import {defineArrayMember, defineField, defineType, type SanityDocument} from '@sanity/types' +import {type FormNodePresence} from 'sanity' + +import {TestForm} from '../../utils/TestForm' +import {TestWrapper} from '../../utils/TestWrapper' + +const schemaTypes = [ + defineType({ + type: 'document', + name: 'test', + title: 'Test', + fields: [ + defineField({ + type: 'array', + name: 'body', + of: [ + defineArrayMember({ + type: 'block', + }), + ], + }), + ], + }), +] + +interface PresenceCursorsStoryProps { + presence: FormNodePresence[] + document: SanityDocument +} + +export function PresenceCursorsStory(props: PresenceCursorsStoryProps) { + const {document, presence} = props + + return ( + + + + ) +} diff --git a/packages/sanity/playwright-ct/tests/formBuilder/utils/TestForm.tsx b/packages/sanity/playwright-ct/tests/formBuilder/utils/TestForm.tsx index 523065c5919..872012599b6 100644 --- a/packages/sanity/playwright-ct/tests/formBuilder/utils/TestForm.tsx +++ b/packages/sanity/playwright-ct/tests/formBuilder/utils/TestForm.tsx @@ -10,6 +10,7 @@ import { EMPTY_ARRAY, FormBuilder, type FormBuilderProps, + type FormNodePresence, getExpandOperations, type PatchEvent, setAtPath, @@ -21,6 +22,7 @@ import { } from 'sanity' import {applyAll} from '../../../../src/core/form/patch/applyPatch' +import {PresenceProvider} from '../../../../src/core/form/studio/contexts/Presence' import {type FormDocumentValue} from '../../../../src/core/form/types' import {createMockSanityClient} from '../../mocks/createMockSanityClient' @@ -32,17 +34,23 @@ declare global { } } -export function TestForm({ - focusPath: focusPathFromProps, - onPathFocus: onPathFocusFromProps, - document: documentFromProps, - id: idFromProps = 'root', -}: { +interface TestFormProps { focusPath?: Path onPathFocus?: (path: Path) => void document?: SanityDocument id?: string -}) { + presence?: FormNodePresence[] +} + +export function TestForm(props: TestFormProps) { + const { + document: documentFromProps, + focusPath: focusPathFromProps, + id: idFromProps = 'root', + onPathFocus: onPathFocusFromProps, + presence: presenceFromProps = EMPTY_ARRAY, + } = props + const [validation, setValidation] = useState([]) const [openPath, onSetOpenPath] = useState([]) const [fieldGroupState, onSetFieldGroupState] = useState>() @@ -106,7 +114,7 @@ export function TestForm({ comparisonValue: null, fieldGroupState, openPath, - presence: EMPTY_ARRAY, + presence: presenceFromProps, validation, value: document, }) @@ -199,7 +207,7 @@ export function TestForm({ onSetFieldSetCollapsed: handleOnSetCollapsedFieldSet, onSetPathCollapsed: handleOnSetCollapsedPath, path: EMPTY_ARRAY, - presence: EMPTY_ARRAY, + presence: presenceFromProps, schemaType: formState?.schemaType || schemaType, validation, value: formState?.value as FormDocumentValue, @@ -220,13 +228,17 @@ export function TestForm({ handleSetActiveFieldGroup, idFromProps, patchChannel, + presenceFromProps, schemaType, setOpenPath, validation, ], ) - - return + return ( + + + + ) } async function validateStaticDocument( diff --git a/packages/sanity/playwright-ct/tests/formBuilder/utils/TestWrapper.tsx b/packages/sanity/playwright-ct/tests/formBuilder/utils/TestWrapper.tsx index 873cda0b82f..0e3054d6e74 100644 --- a/packages/sanity/playwright-ct/tests/formBuilder/utils/TestWrapper.tsx +++ b/packages/sanity/playwright-ct/tests/formBuilder/utils/TestWrapper.tsx @@ -2,9 +2,11 @@ import {type SanityClient} from '@sanity/client' import {Card, LayerProvider, studioTheme, ThemeProvider, ToastProvider} from '@sanity/ui' import {type ReactNode, Suspense, useEffect, useState} from 'react' import { + ColorSchemeProvider, ResourceCacheProvider, type SchemaTypeDefinition, SourceProvider, + UserColorManagerProvider, type Workspace, WorkspaceProvider, } from 'sanity' @@ -57,13 +59,17 @@ export const TestWrapper = ({ - - - - {children} - - - + + + + + + {children} + + + + + diff --git a/packages/sanity/src/core/form/inputs/PortableText/presence-cursors/UserPresenceCursor.tsx b/packages/sanity/src/core/form/inputs/PortableText/presence-cursors/UserPresenceCursor.tsx index 7edff7e64d1..d7a0b50a090 100644 --- a/packages/sanity/src/core/form/inputs/PortableText/presence-cursors/UserPresenceCursor.tsx +++ b/packages/sanity/src/core/form/inputs/PortableText/presence-cursors/UserPresenceCursor.tsx @@ -6,7 +6,7 @@ import { getTheme_v2, } from '@sanity/ui/theme' import {AnimatePresence, motion, type Transition, type Variants} from 'framer-motion' -import {useCallback, useState} from 'react' +import {useCallback, useMemo, useState} from 'react' import {css, styled} from 'styled-components' import {useUserColor} from '../../../../user-color/hooks' @@ -119,10 +119,16 @@ export function UserPresenceCursor(props: UserPresenceCursorProps): JSX.Element const handleMouseEnter = useCallback(() => setHovered(true), []) const handleMouseLeave = useCallback(() => setHovered(false), []) + const testId = useMemo( + () => `presence-cursor-${user.displayName?.split(' ').join('-')}`, + [user.displayName], + ) + return (