diff --git a/lib/components/BlockWorkPackage/BlockCards.tsx b/lib/components/BlockWorkPackage/BlockCards.tsx index 7875bde..eaa9b85 100644 --- a/lib/components/BlockWorkPackage/BlockCards.tsx +++ b/lib/components/BlockWorkPackage/BlockCards.tsx @@ -103,6 +103,10 @@ export const BlockCardM = ({ className="op-bn-work-package op-bn-work-package--m" $inDropdown={inDropdown} onClick={onClick} + // role=button so iOS treats the card as interactive and fires the click on + // the first tap (a plain div inside the contenteditable needs two). Interim: + // mirrors the inline chip; + role={onClick ? 'button' : undefined} data-testid="block-card" style={onClick ? { cursor: 'pointer' } : undefined} > @@ -138,6 +142,7 @@ export const BlockCardL = ({ className="op-bn-work-package op-bn-work-package--l" $inDropdown={inDropdown} onClick={onClick} + role={onClick ? 'button' : undefined} data-testid="block-card" style={onClick ? { cursor: 'pointer' } : undefined} > @@ -187,6 +192,7 @@ export const BlockCardXL = ({ className="op-bn-work-package op-bn-work-package--xl" $inDropdown={inDropdown} onClick={onClick} + role={onClick ? 'button' : undefined} data-testid="block-card" style={onClick ? { cursor: 'pointer' } : undefined} > diff --git a/lib/components/BlockWorkPackage/BlockWorkPackage.tsx b/lib/components/BlockWorkPackage/BlockWorkPackage.tsx index b8e0b3c..fec1f40 100644 --- a/lib/components/BlockWorkPackage/BlockWorkPackage.tsx +++ b/lib/components/BlockWorkPackage/BlockWorkPackage.tsx @@ -19,11 +19,13 @@ import { defaultWpVariables } from '../WorkPackage/atoms'; import { CHIP_STYLES } from '../WorkPackage/tokens'; import { moveCursorAfterBlock } from '../../utils/cursor'; import { pendingBlockRegistry } from './pendingBlockRegistry'; +import { useSuppressFormattingToolbar } from '../../hooks/useSuppressFormattingToolbar'; const Block = styled.div.attrs({ className: 'op-bn-extensions', 'data-testid': 'block-wp-wrapper' })<{ $pending?:boolean; $selected?:boolean }>` ${defaultWpVariables} background-color: ${({ $pending }) => ($pending ? 'transparent' : 'var(--op-chip-bg)')}; - user-select: all; + user-select: none; + -webkit-touch-callout: none; border-radius: var(--bn-border-radius); box-shadow: ${({ $selected }) => ($selected ? CHIP_STYLES.focusShadow : 'none')}; ${({ $pending }) => $pending && 'position: relative;'} @@ -68,6 +70,8 @@ export const BlockWorkPackageComponent = ({ const isBlockSelected = selectedBlocks.some((b) => b.id === block.id); const [isOptionsOpen, setIsOptionsOpen] = useState(false); + useSuppressFormattingToolbar(editor, isOptionsOpen); + const workPackageResult = useWorkPackage(block.props.wpid); const selectedWorkPackage = workPackageResult.workPackage; @@ -178,6 +182,7 @@ export const BlockWorkPackageComponent = ({ {!workPackageResult.loading && (workPackageResult.error ?? workPackageResult.unauthorized) && ( { e.stopPropagation(); setIsOptionsOpen((prev) => !prev); diff --git a/lib/components/InlineWorkPackage/InlineWorkPackageChip.tsx b/lib/components/InlineWorkPackage/InlineWorkPackageChip.tsx index b2b41e0..c3b0169 100644 --- a/lib/components/InlineWorkPackage/InlineWorkPackageChip.tsx +++ b/lib/components/InlineWorkPackage/InlineWorkPackageChip.tsx @@ -25,6 +25,7 @@ import { useTranslation } from 'react-i18next'; import { defaultWpVariables } from '../WorkPackage/atoms'; import { formatWorkPackageId } from '../../utils/id'; import { useIsNodeInSelection } from '../../hooks/useIsNodeInSelection'; +import { useSuppressFormattingToolbar } from '../../hooks/useSuppressFormattingToolbar'; import type { BlockNoteEditor } from '@blocknote/core'; export interface InlineWorkPackageChipProps { @@ -95,6 +96,8 @@ export const InlineWorkPackageChip = ({ inlineContent, contentRef, editor, updat const isEditorSelected = useIsNodeInSelection(chipRef, editor); + useSuppressFormattingToolbar(editor, isSelected); + const setRef = (node:HTMLElement | null) => { chipRef.current = node; contentRef(node); diff --git a/lib/components/WorkPackage/OptionsPopover.tsx b/lib/components/WorkPackage/OptionsPopover.tsx index 40e9030..db10248 100644 --- a/lib/components/WorkPackage/OptionsPopover.tsx +++ b/lib/components/WorkPackage/OptionsPopover.tsx @@ -12,6 +12,7 @@ import { ChevronDownIcon, } from '@primer/octicons-react'; import {formatWorkPackageId} from '../../utils/id'; +import { supportsHover } from '../../utils/device'; export interface WpOptionsProps { wp?:WorkPackage; @@ -166,6 +167,7 @@ export const WpOptionsPopover = ({ popoverRef, placement: 'above', onClose, + closeOnScroll: supportsHover(), }); const isBlock = currentSize === undefined; @@ -179,7 +181,9 @@ export const WpOptionsPopover = ({ }; const content = ( - // Prevent editor/parent handlers from stealing focus or closing the popover + // stopPropagation stops the outside-tap handlers from closing the popover. + // Do NOT add preventDefault: on iOS it suppresses the first tap's click, so + // every button then needs a priming tap. e.stopPropagation()}> {wp && ( <> @@ -212,7 +216,7 @@ export const WpOptionsPopover = ({ {showSizes && ( - e.stopPropagation()}> + {t('options.inlineSizeLabel')} {INLINE_SIZE_OPTIONS.map((size) => { return ( diff --git a/lib/components/WorkPackage/anchoredPopover.tsx b/lib/components/WorkPackage/anchoredPopover.tsx index e5bed40..f98c507 100644 --- a/lib/components/WorkPackage/anchoredPopover.tsx +++ b/lib/components/WorkPackage/anchoredPopover.tsx @@ -22,6 +22,8 @@ export interface AnchoredPopoverOptions { placement:'above' | 'below'; offset?:number; onClose:() => void; + // When false, a scroll repositions the popover instead of closing it. + closeOnScroll?:boolean; } export const useAnchoredPopover = ({ @@ -30,6 +32,7 @@ export const useAnchoredPopover = ({ placement, offset = DEFAULT_ANCHOR_OFFSET, onClose, + closeOnScroll = true, }:AnchoredPopoverOptions) => { const [anchorRect, setAnchorRect] = useState( () => (anchorEl ? getAnchorRect(anchorEl) : null) @@ -38,16 +41,24 @@ export const useAnchoredPopover = ({ useEffect(() => { if (!anchorEl) return; const update = () => setAnchorRect(getAnchorRect(anchorEl)); - const handleScroll = () => onClose(); + + // Coalesce reposition work to one run per frame - scroll fires far faster. + let frame = 0; + const scheduleUpdate = () => { + if (frame) return; + frame = requestAnimationFrame(() => { frame = 0; update(); }); + }; + const handleScroll = () => (closeOnScroll ? onClose() : scheduleUpdate()); update(); window.addEventListener('scroll', handleScroll, true); - window.addEventListener('resize', update); + window.addEventListener('resize', scheduleUpdate); return () => { + if (frame) cancelAnimationFrame(frame); window.removeEventListener('scroll', handleScroll, true); - window.removeEventListener('resize', update); + window.removeEventListener('resize', scheduleUpdate); }; - }, [anchorEl, onClose]); + }, [anchorEl, onClose, closeOnScroll]); // Position before paint so the popover never flashes at its CSS fallback spot. useLayoutEffect(() => { diff --git a/lib/hooks/useSuppressFormattingToolbar.ts b/lib/hooks/useSuppressFormattingToolbar.ts new file mode 100644 index 0000000..913ee65 --- /dev/null +++ b/lib/hooks/useSuppressFormattingToolbar.ts @@ -0,0 +1,32 @@ +import { useEffect } from 'react'; + +interface ToolbarStore { + state:boolean; + setState:(value:boolean) => void; + subscribe:(listener:() => void) => () => void; +} + +interface ToolbarHost { + getExtension:(key:string) => { store?:ToolbarStore } | undefined; +} + +// Keeps BlockNote's built-in formatting toolbar closed while `active`. BlockNote +// re-opens it on any pointerup over a focused editor with a non-empty selection, +// and our chip/block holds a NodeSelection the whole time its popover is open - +// so each tap on a popover button would otherwise stack a second toolbar. +export function useSuppressFormattingToolbar( + editor:ToolbarHost | undefined, + active:boolean, +):void { + useEffect(() => { + if (!active || !editor) return; + const store = editor.getExtension('formattingToolbar')?.store; + if (!store) return; + + const enforceClosed = () => { + if (store.state) store.setState(false); + }; + enforceClosed(); + return store.subscribe(enforceClosed); + }, [editor, active]); +} diff --git a/lib/hooks/useWorkPackagePreview.ts b/lib/hooks/useWorkPackagePreview.ts index a177731..1cc251a 100644 --- a/lib/hooks/useWorkPackagePreview.ts +++ b/lib/hooks/useWorkPackagePreview.ts @@ -7,6 +7,7 @@ import { type MouseEvent as ReactMouseEvent, type PointerEvent as ReactPointerEvent, } from 'react'; +import { supportsHover } from '../utils/device'; const PREVIEW_OPEN_DELAY = 300; const PREVIEW_CLOSE_DELAY = 150; @@ -49,13 +50,7 @@ interface WorkPackagePreview { // inline work package chip: open/close timing, touch long-press detection, and // the props to wire onto the chip and the preview card. export function useWorkPackagePreview({ enabled, suppressed }:UseWorkPackagePreviewOptions):WorkPackagePreview { - const canHover = useMemo( - () => - typeof window !== 'undefined' && - typeof window.matchMedia === 'function' && - window.matchMedia('(hover: hover)').matches, - [] - ); + const canHover = useMemo(() => supportsHover(), []); const [previewOpen, setPreviewOpen] = useState(false); const openTimer = useRef(undefined); diff --git a/lib/utils/device.ts b/lib/utils/device.ts new file mode 100644 index 0000000..f806156 --- /dev/null +++ b/lib/utils/device.ts @@ -0,0 +1,9 @@ +// True when the primary pointer can hover (desktop mouse) rather than a touch +// screen like iOS - used to branch touch-specific behaviour. +export function supportsHover():boolean { + return ( + typeof window !== 'undefined' && + typeof window.matchMedia === 'function' && + window.matchMedia('(hover: hover)').matches + ); +} diff --git a/test/lib/components/integration/editor.blockWorkPackage.browser.test.tsx b/test/lib/components/integration/editor.blockWorkPackage.browser.test.tsx index 4b51c90..c895b30 100644 --- a/test/lib/components/integration/editor.blockWorkPackage.browser.test.tsx +++ b/test/lib/components/integration/editor.blockWorkPackage.browser.test.tsx @@ -124,4 +124,26 @@ describe('Block card - remove', () => { await expect.element(page.getByTestId('block-card')).not.toBeInTheDocument(); await expect.element(page.getByText('Fix login bug')).not.toBeInTheDocument(); }); +}); + +describe('Block card - selection', () => { + it('the card wrapper is not text-selectable', async () => { + renderEditor(); + await insertInlineWorkPackageViaSlashMenu(); + await convertToCompactCard(); + + const wrapper = document.querySelector('[data-testid="block-wp-wrapper"]')!; + // user-select: all made a single click highlight the whole card's text. + expect(getComputedStyle(wrapper).userSelect).toBe('none'); + }); + + // role=button is what makes iOS fire the click on the first tap (a plain div + // in the contenteditable needs two); see BlockCards. + it('the clickable card exposes role=button', async () => { + renderEditor(); + await insertInlineWorkPackageViaSlashMenu(); + await convertToCompactCard(); + + expect(page.getByTestId('block-card').element().getAttribute('role')).toBe('button'); + }); }); \ No newline at end of file diff --git a/test/lib/components/integration/wpOptionsPopover.browser.test.tsx b/test/lib/components/integration/wpOptionsPopover.browser.test.tsx index f8324e9..2de849c 100644 --- a/test/lib/components/integration/wpOptionsPopover.browser.test.tsx +++ b/test/lib/components/integration/wpOptionsPopover.browser.test.tsx @@ -7,14 +7,22 @@ import { renderEditor } from '../../../helpers/renderEditor'; import { insertInlineWorkPackageViaSlashMenu, openInlineWorkPackagePopover, - convertToCompactCard, openBlockCardPopover, + convertToCompactCard, } from '../../../helpers/editorHelpers'; afterEach(() => { cleanup(); }); +// BlockNote leaves an empty display:none toolbar container in the DOM when +// closed, so presence alone is not enough - only a rendered one counts. +const formattingToolbarVisible = () => + Array.from(document.querySelectorAll('[class*="formatting-toolbar"]')).some((el) => { + const he = el as HTMLElement; + return getComputedStyle(he).display !== 'none' && he.childElementCount > 0; + }); + function ChipWrapper({ initialSize, wpid = '123' }:{ initialSize:string; wpid?:string; @@ -294,4 +302,49 @@ describe('Inline chip popover UX', () => { await expect.element(page.getByRole('button', { name: label, exact: true })).toBeVisible(); } }); +}); + +describe('Options popover coexists with the editor', () => { + it('inline chip: opening the size menu summons no formatting toolbar', async () => { + renderEditor(); + await insertInlineWorkPackageViaSlashMenu(); + await openInlineWorkPackagePopover(); + + await userEvent.click(page.getByTitle('Change size')); + + await expect.element(page.getByTestId('size-menu')).toBeVisible(); + expect(formattingToolbarVisible()).toBe(false); + }); + + it('inline chip: resizing summons no formatting toolbar', async () => { + renderEditor(); + await insertInlineWorkPackageViaSlashMenu(); + await openInlineWorkPackagePopover(); + await userEvent.click(page.getByTitle('Change size')); + + await userEvent.click(page.getByRole('button', { name: 'Compact', exact: true })); + + expect(formattingToolbarVisible()).toBe(false); + }); + + it('inline chip: a scroll closes the popover on desktop', async () => { + renderEditor(); + await insertInlineWorkPackageViaSlashMenu(); + await openInlineWorkPackagePopover(); + + window.dispatchEvent(new Event('scroll')); + + await expect.element(page.getByTestId('popover-content')).not.toBeInTheDocument(); + }); + + it('block card: opening the popover summons no formatting toolbar', async () => { + renderEditor(); + await insertInlineWorkPackageViaSlashMenu(); + await convertToCompactCard(); + + await openBlockCardPopover(); + await expect.element(page.getByTestId('popover-content')).toBeVisible(); + + expect(formattingToolbarVisible()).toBe(false); + }); }); \ No newline at end of file