diff --git a/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/Annotations.spec.tsx b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/Annotations.spec.tsx index 7996495ba9c..719f11b3da9 100644 --- a/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/Annotations.spec.tsx +++ b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/Annotations.spec.tsx @@ -48,8 +48,25 @@ test.describe('Portable Text Input', () => { // Expect the editor to have focus after closing the popover await expect($pte).toBeFocused() + const $toolbarPopover = page.getByTestId('annotation-toolbar-popover') + // Assertion: the annotation toolbar popover should be visible await expect(page.getByTestId('annotation-toolbar-popover')).toBeVisible() + await expect($toolbarPopover).toBeVisible() + + // Assertion: tab works to get to the toolbar popover buttons + await page.keyboard.press('Tab') + await expect(page.getByTestId('edit-annotation-button')).toBeFocused() + await page.keyboard.press('Tab') + await expect(page.getByTestId('remove-annotation-button')).toBeFocused() + await page.keyboard.press('Escape') + await expect($pte).toBeFocused() + await expect($toolbarPopover).toBeVisible() + await page.waitForTimeout(100) + await page.keyboard.press('Escape') + await page.waitForTimeout(100) + // Assertion: escape closes the toolbar popover + await expect($toolbarPopover).not.toBeVisible() }) test('Can create, and then open the existing annotation again for editing', async ({ diff --git a/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/FocusTracking.spec.tsx b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/FocusTracking.spec.tsx index 600c8313ee3..1831c08102a 100644 --- a/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/FocusTracking.spec.tsx +++ b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/FocusTracking.spec.tsx @@ -117,6 +117,7 @@ test.describe('Portable Text Input', () => { const $portableTextInput = component.getByTestId('field-body') const $pteTextbox = $portableTextInput.getByRole('textbox') await expect($pteTextbox).not.toBeFocused() + await page.keyboard.press('Tab+Tab') const blockObjectInput = page.getByTestId('objectBlockInputField').getByRole('textbox') await expect(blockObjectInput).toBeFocused() }) diff --git a/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/ObjectBlock.spec.tsx b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/ObjectBlock.spec.tsx index c1c42a264d4..37687944204 100644 --- a/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/ObjectBlock.spec.tsx +++ b/packages/sanity/playwright-ct/tests/formBuilder/inputs/PortableText/ObjectBlock.spec.tsx @@ -30,6 +30,32 @@ test.describe('Portable Text Input', () => { await expect($pte.getByText('Custom preview block:')).toBeVisible() }) + test('Inline object toolbars works as expected', async ({mount, page}) => { + const {getFocusedPortableTextEditor} = testHelpers({page}) + await mount() + const $pte = await getFocusedPortableTextEditor('field-body') + await page.getByRole('button', {name: 'Insert Inline Object (inline)'}).click() + // Assertion: the annotation toolbar popover should not be visible + await expect(page.getByTestId('inline-object-toolbar-popover')).not.toBeVisible() + const $locatorDialog = page.getByTestId('popover-edit-dialog') + // Assertion: Object edit dialog should be visible + await expect($locatorDialog).toBeVisible() + await page.keyboard.press('Escape') + // Assertion: the annotation toolbar popover should be visible + await expect(page.getByTestId('inline-object-toolbar-popover')).toBeVisible() + // Assertion: tab works to get to the toolbar popover buttons + await page.keyboard.press('Tab') + await expect(page.getByTestId('edit-inline-object-button')).toBeFocused() + await page.keyboard.press('Tab') + await expect(page.getByTestId('remove-inline-object-button')).toBeFocused() + await page.keyboard.press('Escape') + await expect(page.getByTestId('edit-inline-object-button')).toBeVisible() + await expect($pte).toBeFocused() + await page.keyboard.press('Escape') + // Assertion: escape closes the toolbar popover + await expect(page.getByTestId('inline-object-toolbar-popover')).not.toBeVisible() + }) + test('Double-clicking opens a block', async ({mount, page}) => { const {getFocusedPortableTextEditor} = testHelpers({page}) await mount() diff --git a/packages/sanity/src/core/form/inputs/PortableText/object/Annotation.tsx b/packages/sanity/src/core/form/inputs/PortableText/object/Annotation.tsx index 89345431cb6..43feee8ad74 100644 --- a/packages/sanity/src/core/form/inputs/PortableText/object/Annotation.tsx +++ b/packages/sanity/src/core/form/inputs/PortableText/object/Annotation.tsx @@ -1,7 +1,7 @@ import {PortableTextEditor, usePortableTextEditor} from '@portabletext/editor' import {type ObjectSchemaType, type Path, type PortableTextObject} from '@sanity/types' import {isEqual} from '@sanity/util/paths' -import {type ComponentType, type ReactNode, useCallback, useMemo, useState} from 'react' +import {type ComponentType, useCallback, useMemo, useState} from 'react' import {Tooltip} from '../../../../../ui-components' import {pathToString} from '../../../../field' @@ -54,7 +54,7 @@ interface AnnotationProps { value: PortableTextObject } -export function Annotation(props: AnnotationProps): ReactNode { +export function Annotation(props: AnnotationProps): React.JSX.Element { const { children, editorNodeFocused, @@ -241,7 +241,7 @@ export function Annotation(props: AnnotationProps): ReactNode { ) } -export const DefaultAnnotationComponent = (props: BlockAnnotationProps) => { +export const DefaultAnnotationComponent = (props: BlockAnnotationProps): React.JSX.Element => { const { __unstable_floatingBoundary: floatingBoundary, __unstable_referenceBoundary: referenceBoundary, @@ -252,8 +252,8 @@ export const DefaultAnnotationComponent = (props: BlockAnnotationProps) => { onRemove, open, readOnly, - selected, schemaType, + selected, textElement, validation, } = props @@ -264,6 +264,7 @@ export const DefaultAnnotationComponent = (props: BlockAnnotationProps) => { const isReady = Boolean(children) const {t} = useTranslation() + const toneKey = useMemo(() => { if (hasError) { return 'critical' @@ -294,11 +295,11 @@ export const DefaultAnnotationComponent = (props: BlockAnnotationProps) => { void - onRemove: () => void + onOpenAnnotation: () => void + onRemoveAnnotation: () => void referenceBoundary: HTMLElement | null referenceElement: HTMLElement | null - selected: boolean title: string } -export function AnnotationToolbarPopover(props: AnnotationToolbarPopoverProps) { +export function AnnotationToolbarPopover(props: AnnotationToolbarPopoverProps): ReactNode { const { annotationOpen, + annotationTextSelected, floatingBoundary, - onOpen, - onRemove, + onOpenAnnotation, + onRemoveAnnotation, referenceBoundary, referenceElement, - selected, title, } = props - const [renderPopover, setRenderPopover] = useState(false) const [popoverOpen, setPopoverOpen] = useState(false) const [cursorRect, setCursorRect] = useState(null) const rangeRef = useRef(null) const {sanity} = useTheme() const {t} = useTranslation() - const popoverRef = useRef(null) + const editButtonRef = useRef(null) + const deleteButtonRef = useRef(null) + const focusTrappedRef = useRef(null) const popoverScheme = sanity.color.dark ? 'light' : 'dark' - - //Add separate handler for popover state - //to prevent the popover from jumping when opening - const handleOpenPopover = useCallback((open: boolean) => { - setRenderPopover(open) - if (open) { - startTransition(() => { - setPopoverOpen(open) - }) - } else { - setPopoverOpen(open) - } - }, []) + const editor = usePortableTextEditor() // This is a "virtual element" (supported by Popper.js) const cursorElement = useMemo(() => { @@ -63,6 +53,13 @@ export function AnnotationToolbarPopover(props: AnnotationToolbarPopoverProps) { } }, [cursorRect]) as HTMLElement + const handleClosePopover = useCallback(() => { + PortableTextEditor.focus(editor) + setPopoverOpen(false) + focusTrappedRef.current = null + }, [editor]) + + // Tab to edit button on tab // Close floating toolbar on Escape useGlobalKeyDown( useCallback( @@ -70,18 +67,38 @@ export function AnnotationToolbarPopover(props: AnnotationToolbarPopoverProps) { if (!popoverOpen) { return } + if (event.key === 'Tab') { + if ( + annotationTextSelected && + event.target instanceof HTMLElement && + event.target.contentEditable && + focusTrappedRef.current === null + ) { + event.preventDefault() + editButtonRef.current?.focus() + focusTrappedRef.current = editButtonRef.current + return + } + if (event.target === deleteButtonRef.current) { + event.preventDefault() + event.stopPropagation() + focusTrappedRef.current = null + PortableTextEditor.focus(editor) + return + } + } if (event.key === 'Escape') { - handleOpenPopover(false) + handleClosePopover() } }, - [handleOpenPopover, popoverOpen], + [editor, handleClosePopover, popoverOpen, annotationTextSelected], ), ) // Open popover when selection is within the annotation text const handleSelectionChange = useCallback(() => { if (annotationOpen) { - handleOpenPopover(false) + setPopoverOpen(false) setCursorRect(null) return } @@ -90,20 +107,21 @@ export function AnnotationToolbarPopover(props: AnnotationToolbarPopoverProps) { if (!sel || sel.rangeCount === 0) return + focusTrappedRef.current = null const range = sel.getRangeAt(0) const isWithinRoot = referenceElement?.contains(range.commonAncestorContainer) if (!isWithinRoot) { - handleOpenPopover(false) + setPopoverOpen(false) setCursorRect(null) return } const rect = range?.getBoundingClientRect() if (rect) { setCursorRect(rect) - handleOpenPopover(true) + setPopoverOpen(true) } - }, [annotationOpen, referenceElement, handleOpenPopover]) + }, [annotationOpen, referenceElement, setPopoverOpen]) // Detect selection changes useEffect(() => { @@ -114,24 +132,14 @@ export function AnnotationToolbarPopover(props: AnnotationToolbarPopoverProps) { }, [handleSelectionChange]) const handleEditButtonClicked = useCallback(() => { - handleOpenPopover(false) - onOpen() - }, [onOpen, handleOpenPopover]) - - // Open the popover when closing the annotation dialog - useEffect(() => { - if (!annotationOpen && selected && cursorRect) { - handleOpenPopover(true) - } - if (annotationOpen) { - handleOpenPopover(false) - } - }, [annotationOpen, selected, cursorRect, handleOpenPopover]) + setPopoverOpen(false) + onOpenAnnotation() + }, [onOpenAnnotation]) const handleRemoveButtonClicked = useCallback(() => { - handleOpenPopover(false) - onRemove() - }, [onRemove, handleOpenPopover]) + setPopoverOpen(false) + onRemoveAnnotation() + }, [onRemoveAnnotation]) const handleScroll = useCallback(() => { if (rangeRef.current) { @@ -147,26 +155,17 @@ export function AnnotationToolbarPopover(props: AnnotationToolbarPopoverProps) { }, [popoverOpen]) useEffect(() => { - //Attach and detach scroll event listener for popover to follow the current reference boundary - if (popoverOpen && referenceBoundary) { - referenceBoundary.addEventListener('scroll', handleScroll) - return () => referenceBoundary.removeEventListener('scroll', handleScroll) - } - - if (!popoverOpen) { - return undefined + // Listen for scroll events on the floating boundary and the reference boundary + // and move the popover accordingly + if (popoverOpen) { + floatingBoundary?.addEventListener('scroll', handleScroll) + referenceBoundary?.addEventListener('scroll', handleScroll) } - - referenceBoundary?.addEventListener('scroll', handleScroll) - return () => { + floatingBoundary?.removeEventListener('scroll', handleScroll) referenceBoundary?.removeEventListener('scroll', handleScroll) } - }, [popoverOpen, referenceBoundary, handleScroll]) - - if (!renderPopover) { - return null - } + }, [popoverOpen, referenceBoundary, floatingBoundary, handleScroll]) return ( @@ -196,6 +196,7 @@ export function AnnotationToolbarPopover(props: AnnotationToolbarPopoverProps) { icon={TrashIcon} mode="bleed" onClick={handleRemoveButtonClicked} + ref={deleteButtonRef} tabIndex={0} tone="critical" tooltipProps={null} @@ -207,7 +208,6 @@ export function AnnotationToolbarPopover(props: AnnotationToolbarPopoverProps) { placement="top" portal preventOverflow - ref={popoverRef} referenceBoundary={referenceBoundary} referenceElement={cursorElement} scheme={popoverScheme} diff --git a/packages/sanity/src/core/form/inputs/PortableText/object/BlockObject.tsx b/packages/sanity/src/core/form/inputs/PortableText/object/BlockObject.tsx index 1e2822854ac..7350adb4257 100644 --- a/packages/sanity/src/core/form/inputs/PortableText/object/BlockObject.tsx +++ b/packages/sanity/src/core/form/inputs/PortableText/object/BlockObject.tsx @@ -435,7 +435,7 @@ export const DefaultBlockObjectComponent = (props: BlockProps) => { floatingBoundary={__unstable_floatingBoundary} defaultType="dialog" onClose={onClose} - autoFocus={focused} + autoFocus schemaType={schemaType} referenceBoundary={__unstable_referenceBoundary} referenceElement={__unstable_referenceElement} diff --git a/packages/sanity/src/core/form/inputs/PortableText/object/InlineObject.tsx b/packages/sanity/src/core/form/inputs/PortableText/object/InlineObject.tsx index d9c194fe814..430a23ecd1f 100644 --- a/packages/sanity/src/core/form/inputs/PortableText/object/InlineObject.tsx +++ b/packages/sanity/src/core/form/inputs/PortableText/object/InlineObject.tsx @@ -6,7 +6,7 @@ import { type PortableTextChild, } from '@sanity/types' import {isEqual} from '@sanity/util/paths' -import {useCallback, useEffect, useMemo, useState} from 'react' +import {useCallback, useMemo, useState} from 'react' import {Tooltip} from '../../../../../ui-components' import {pathToString} from '../../../../field/paths' @@ -56,7 +56,7 @@ interface InlineObjectProps { value: PortableTextChild } -export const InlineObject = (props: InlineObjectProps) => { +export const InlineObject = (props: InlineObjectProps): React.JSX.Element => { const { floatingBoundary, focused, @@ -107,7 +107,7 @@ export const InlineObject = (props: InlineObjectProps) => { PortableTextEditor.blur(editor) onItemOpen(memberItem.node.path) } - }, [editor, onItemOpen, memberItem]) + }, [onItemOpen, editor, memberItem]) const onClose = useCallback(() => { onItemClose() @@ -236,7 +236,7 @@ export const InlineObject = (props: InlineObjectProps) => { ) } -export const DefaultInlineObjectComponent = (props: BlockProps) => { +export const DefaultInlineObjectComponent = (props: BlockProps): React.JSX.Element => { const { __unstable_floatingBoundary: floatingBoundary, __unstable_referenceBoundary: referenceBoundary, @@ -257,26 +257,10 @@ export const DefaultInlineObjectComponent = (props: BlockProps) => { } = props const {t} = useTranslation() const hasMarkers = markers.length > 0 - const [popoverOpen, setPopoverOpen] = useState(false) const popoverTitle = schemaType?.title || schemaType.name const hasError = validation.filter((v) => v.level === 'error').length > 0 const hasWarning = validation.filter((v) => v.level === 'warning').length > 0 - const openItem = useCallback((): void => { - setPopoverOpen(false) - onOpen() - }, [onOpen]) - - useEffect(() => { - if (open) { - setPopoverOpen(false) - } else if (focused) { - setPopoverOpen(true) - } else { - setPopoverOpen(false) - } - }, [focused, open]) - const tone = useMemo(() => { if (hasError) { return 'critical' @@ -292,10 +276,6 @@ export const DefaultInlineObjectComponent = (props: BlockProps) => { return undefined }, [focused, hasError, hasWarning, selected]) - const onClosePopover = useCallback(() => { - setPopoverOpen(false) - }, []) - return ( <> { data-selected={selected || undefined} data-warning={hasWarning || undefined} forwardedAs="span" - onClick={readOnly ? openItem : undefined} - onDoubleClick={openItem} + onClick={readOnly ? onOpen : undefined} + onDoubleClick={onOpen} tone={tone} > @@ -324,10 +304,10 @@ export const DefaultInlineObjectComponent = (props: BlockProps) => { {referenceElement && ( { )} {open && ( void - onDelete: (event: MouseEvent) => void - onEdit: (event: MouseEvent) => void + inlineObjectFocused: boolean + inlineObjectOpen: boolean + onOpenInlineObject: () => void + onRemoveInlineObject: () => void referenceBoundary: HTMLElement | null referenceElement: HTMLElement | null title: string } -export function InlineObjectToolbarPopover(props: InlineObjectToolbarPopoverProps) { +export function InlineObjectToolbarPopover(props: InlineObjectToolbarPopoverProps): ReactNode { const { floatingBoundary, - onClosePopover, - onEdit, - onDelete, + inlineObjectFocused, + inlineObjectOpen, + onOpenInlineObject, + onRemoveInlineObject, referenceBoundary, referenceElement, title, - open, } = props + const [popoverOpen, setPopoverOpen] = useState(false) const {sanity} = useTheme() const {t} = useTranslation() const editButtonRef = useRef(null) const deleteButtonRef = useRef(null) + const focusTrappedRef = useRef(null) const popoverScheme = sanity.color.dark ? 'light' : 'dark' + const editor = usePortableTextEditor() + const contentRef = useRef(null) + const handleClosePopover = useCallback(() => { + setPopoverOpen(false) + PortableTextEditor.focus(editor) + focusTrappedRef.current = null + }, [editor]) + + // Tab to edit button on tab // Close floating toolbar on Escape - // Focus to edit button on Tab useGlobalKeyDown( useCallback( (event) => { + if (!popoverOpen) { + return + } + if (event.key === 'Tab') { + if ( + inlineObjectFocused && + event.target instanceof HTMLElement && + event.target.contentEditable && + focusTrappedRef.current === null + ) { + event.preventDefault() + editButtonRef.current?.focus() + focusTrappedRef.current = editButtonRef.current + return + } + if (event.target === deleteButtonRef.current) { + event.preventDefault() + event.stopPropagation() + focusTrappedRef.current = null + PortableTextEditor.focus(editor) + return + } + } if (event.key === 'Escape') { - event.preventDefault() - event.stopPropagation() - onClosePopover() + handleClosePopover() } }, - [onClosePopover], + [editor, inlineObjectFocused, handleClosePopover, popoverOpen], ), ) - const handleDelete = useCallback( - (event: MouseEvent) => { - if (deleteButtonRef.current?.disabled) { - return - } - event.preventDefault() - event.stopPropagation() - try { - onDelete(event) - } catch (err) { - console.error(err) - } finally { - if (deleteButtonRef.current) { - deleteButtonRef.current.disabled = true - } - } - }, - [onDelete], - ) + useEffect(() => { + focusTrappedRef.current = null + if (inlineObjectOpen) { + setPopoverOpen(false) + return + } + if (inlineObjectFocused) { + setPopoverOpen(true) + return + } + setPopoverOpen(false) + }, [inlineObjectFocused, inlineObjectOpen]) - const popoverContent = useMemo( - () => ( - - - - - {title} - - -