From 209eed0e731e4a88e5371de89ff918fcdf1c52ab Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 31 Aug 2026 16:02:27 +0200 Subject: [PATCH 1/6] fix(ui): commit popover forms through submit, not a key handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creating a link on Android didn't work: the popover's URL never became a link and focus jumped to the next editor instead. The cause is that a mobile IME picks the action its Enter key performs, and with a lone text field it picks "Next" — advancing focus and dispatching no key event at all. A popover that only listens for Enter therefore has nothing to hear. Putting the fields in a real `
` is what makes the IME offer a submitting action instead, confirmed on a device; `Form.Root` was a `
`, so `onSubmit` could never fire. `Form.Root` now renders a ``, and submission runs off its `submit` event. That has three consequences worth calling out: - HTML only submits implicitly when a form has a submit button or exactly one field, so the link *edit* form — url plus title — would still reach nothing. `Form.Root` renders a submit button to cover any field count. It is visually hidden rather than absent so assistive technology still has a labelled control, and outside the tab order so sighted keyboard users never land on a control they can't see. - The browser performs implicit submission for an Enter that arrives with `isComposing: true`, so accepting an IME candidate would submit the popover mid-word. `useFormSubmit` guards that centrally, replacing the per-callsite `isComposing` checks that had already drifted apart. - With one submission path, the five Enter handlers are redundant and are removed. `EmbedTab` had no form at all and gains one; the AI prompt menu's handler and `onSubmit` disagreed about whether Enter picks the highlighted suggestion or submits the typed text, and now share one decision. `TextInput` also loses its `onSubmit` prop: every skin forwarded it to the ``, and `submit` only fires on a form and bubbles upward, so it could never have fired. `EditLinkMenuItems` passed it, which is plausibly why the gap went unnoticed. --- packages/ariakit/src/input/Form.tsx | 23 +- packages/ariakit/src/input/TextInput.tsx | 30 ++- packages/ariakit/src/style.css | 20 ++ .../core/src/editor/managers/StyleManager.ts | 8 +- packages/core/src/i18n/locales/ar.ts | 1 + packages/core/src/i18n/locales/de.ts | 1 + packages/core/src/i18n/locales/en.ts | 1 + packages/core/src/i18n/locales/es.ts | 1 + packages/core/src/i18n/locales/fa.ts | 1 + packages/core/src/i18n/locales/fr.ts | 1 + packages/core/src/i18n/locales/he.ts | 1 + packages/core/src/i18n/locales/hr.ts | 1 + packages/core/src/i18n/locales/is.ts | 1 + packages/core/src/i18n/locales/it.ts | 1 + packages/core/src/i18n/locales/ja.ts | 1 + packages/core/src/i18n/locales/ko.ts | 1 + packages/core/src/i18n/locales/nl.ts | 1 + packages/core/src/i18n/locales/no.ts | 1 + packages/core/src/i18n/locales/pl.ts | 1 + packages/core/src/i18n/locales/pt.ts | 1 + packages/core/src/i18n/locales/ru.ts | 1 + packages/core/src/i18n/locales/sk.ts | 1 + packages/core/src/i18n/locales/uk.ts | 1 + packages/core/src/i18n/locales/uz.ts | 1 + packages/core/src/i18n/locales/vi.ts | 1 + packages/core/src/i18n/locales/zh-tw.ts | 1 + packages/core/src/i18n/locales/zh.ts | 1 + packages/mantine/src/blocknoteStyles.css | 33 +++ packages/mantine/src/components.tsx | 3 +- packages/mantine/src/form/Form.tsx | 25 +++ packages/mantine/src/form/TextInput.tsx | 30 ++- packages/mantine/src/popover/Popover.tsx | 6 + .../FilePanel/DefaultTabs/EmbedTab.tsx | 41 ++-- .../DefaultButtons/CreateLinkButton.tsx | 3 + .../DefaultButtons/FileCaptionButton.tsx | 15 +- .../DefaultButtons/FileRenameButton.tsx | 15 +- .../LinkToolbar/EditLinkMenuItems.tsx | 26 +-- .../react/src/editor/ComponentsContext.tsx | 17 +- packages/react/src/hooks/useFormSubmit.ts | 49 +++++ packages/react/src/index.ts | 1 + packages/shadcn/src/form/Form.tsx | 21 +- packages/shadcn/src/form/TextInput.tsx | 30 ++- packages/shadcn/src/style.css | 20 ++ .../AIMenu/PromptSuggestionMenu.tsx | 49 +++-- .../form/compositionSubmit.test.tsx | 134 ++++++++++++ .../end-to-end/form/implicitSubmit.test.tsx | 135 ++++++++++++ .../end-to-end/form/popoverSubmit.test.tsx | 149 +++++++++++++ .../src/end-to-end/mobile/linkSubmit.test.tsx | 131 ++++++++++++ .../end-to-end/mobile/mobileToolbar.test.tsx | 198 ++++++++++++++++++ .../end-to-end/mobile/popoverScroll.test.tsx | 86 ++++++++ 50 files changed, 1195 insertions(+), 126 deletions(-) create mode 100644 packages/mantine/src/form/Form.tsx create mode 100644 packages/react/src/hooks/useFormSubmit.ts create mode 100644 tests/src/end-to-end/form/compositionSubmit.test.tsx create mode 100644 tests/src/end-to-end/form/implicitSubmit.test.tsx create mode 100644 tests/src/end-to-end/form/popoverSubmit.test.tsx create mode 100644 tests/src/end-to-end/mobile/linkSubmit.test.tsx create mode 100644 tests/src/end-to-end/mobile/mobileToolbar.test.tsx create mode 100644 tests/src/end-to-end/mobile/popoverScroll.test.tsx diff --git a/packages/ariakit/src/input/Form.tsx b/packages/ariakit/src/input/Form.tsx index bf964aee66..14fe9b9916 100644 --- a/packages/ariakit/src/input/Form.tsx +++ b/packages/ariakit/src/input/Form.tsx @@ -1,12 +1,29 @@ import { FormProvider as AriakitFormProvider } from "@ariakit/react"; import { assertEmpty } from "@blocknote/core"; -import { ComponentProps } from "@blocknote/react"; +import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, ...rest } = props; + const { children, onSubmit, ...rest } = props; + const dict = useDictionary(); + const formProps = useFormSubmit(onSubmit); assertEmpty(rest); - return {children}; + return ( + + + {children} + {/* + Gives the form a submit button, which is what makes Enter submit it at + all once a caller renders more than one field (see the `onSubmit` + contract in `ComponentsContext`). Visually hidden rather than absent, + so assistive technology still has a labelled control to activate. + */} + + + + ); }; diff --git a/packages/ariakit/src/input/TextInput.tsx b/packages/ariakit/src/input/TextInput.tsx index 555961faf0..7dfec842ee 100644 --- a/packages/ariakit/src/input/TextInput.tsx +++ b/packages/ariakit/src/input/TextInput.tsx @@ -5,7 +5,7 @@ import { import { assertEmpty, mergeCSSClasses } from "@blocknote/core"; import { ComponentProps } from "@blocknote/react"; -import { forwardRef } from "react"; +import { forwardRef, useCallback, useEffect, useRef } from "react"; export const TextInput = forwardRef< HTMLInputElement, @@ -23,7 +23,6 @@ export const TextInput = forwardRef< disabled, onKeyDown, onChange, - onSubmit, autoComplete, "aria-activedescendant": ariaActivedescendant, rightSection, @@ -32,6 +31,29 @@ export const TextInput = forwardRef< assertEmpty(rest); + // Focus with `preventScroll`, rather than the native `autofocus`: these + // inputs live in popovers that floating-ui positions *after* mount, so the + // browser's scroll-into-view runs while the popover is still at its + // pre-positioned spot and yanks the page (on mobile, right out from under + // the block being edited). + const inputRef = useRef(null); + const setRefs = useCallback( + (element: HTMLInputElement | null) => { + inputRef.current = element; + if (typeof ref === "function") { + ref(element); + } else if (ref) { + ref.current = element; + } + }, + [ref], + ); + useEffect(() => { + if (autoFocus) { + inputRef.current?.focus({ preventScroll: true }); + } + }, [autoFocus]); + return ( <> {props.label && {label}} @@ -43,15 +65,13 @@ export const TextInput = forwardRef< className || "", variant === "large" ? "bn-ak-input-large" : "", )} - ref={ref} + ref={setRefs} name={name} value={value} - autoFocus={autoFocus} placeholder={placeholder} disabled={disabled} onKeyDown={onKeyDown} onChange={onChange} - onSubmit={onSubmit} autoComplete={autoComplete} aria-activedescendant={ariaActivedescendant} /> diff --git a/packages/ariakit/src/style.css b/packages/ariakit/src/style.css index 59974a6d60..6212efe74c 100644 --- a/packages/ariakit/src/style.css +++ b/packages/ariakit/src/style.css @@ -433,3 +433,23 @@ .bn-ariakit .bn-thread.selected .bn-ak-expand-sections-prompt { color: var(--bn-colors-selected-text); } + +/* The submit button `Form.Root` renders so that Enter reaches the form + * regardless of how many fields a popover has. It carries no visual design of + * its own - the popovers commit on Enter - but it stays in the accessibility + * tree with a real label, so screen readers and voice control have a submit + * control to operate. It is out of the tab order: keeping a control nobody can + * see as a tab stop would strand sighted keyboard users on invisible focus, + * and Enter already submits for them. + */ +.bn-form-submit { + border: 0; + clip-path: inset(50%); + height: 1px; + margin: -1px; + overflow: hidden; + padding: 0; + position: absolute; + white-space: nowrap; + width: 1px; +} diff --git a/packages/core/src/editor/managers/StyleManager.ts b/packages/core/src/editor/managers/StyleManager.ts index e412160e4a..a3ddf0d52b 100644 --- a/packages/core/src/editor/managers/StyleManager.ts +++ b/packages/core/src/editor/managers/StyleManager.ts @@ -183,7 +183,13 @@ export class StyleManager< */ public getSelectedLinkUrl() { return this.editor.transact((tr) => { - return this.getLinkMarkAtPos(tr.selection.from)?.href; + // `from + 1` for the same boundary reason as `editLink` below: at the + // left edge of a link (e.g. when the whole link is selected), the mark + // lookup at `from` itself misses the mark and the link's URL would + // incorrectly read as absent. + return this.getLinkMarkAtPos( + Math.min(tr.selection.from + 1, tr.doc.content.size), + )?.href; }); } diff --git a/packages/core/src/i18n/locales/ar.ts b/packages/core/src/i18n/locales/ar.ts index 094671d920..b503d01eb9 100644 --- a/packages/core/src/i18n/locales/ar.ts +++ b/packages/core/src/i18n/locales/ar.ts @@ -406,5 +406,6 @@ export const ar: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "إرسال", }, }; diff --git a/packages/core/src/i18n/locales/de.ts b/packages/core/src/i18n/locales/de.ts index bf77a36a01..29b9eaee64 100644 --- a/packages/core/src/i18n/locales/de.ts +++ b/packages/core/src/i18n/locales/de.ts @@ -440,5 +440,6 @@ export const de: Dictionary = { }, generic: { ctrl_shortcut: "Strg", + form_submit: "Absenden", }, }; diff --git a/packages/core/src/i18n/locales/en.ts b/packages/core/src/i18n/locales/en.ts index e5386f3020..76f636bd75 100644 --- a/packages/core/src/i18n/locales/en.ts +++ b/packages/core/src/i18n/locales/en.ts @@ -421,5 +421,6 @@ export const en = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Submit", }, }; diff --git a/packages/core/src/i18n/locales/es.ts b/packages/core/src/i18n/locales/es.ts index 743a1be05c..a878b27efd 100644 --- a/packages/core/src/i18n/locales/es.ts +++ b/packages/core/src/i18n/locales/es.ts @@ -419,5 +419,6 @@ export const es: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Enviar", }, }; diff --git a/packages/core/src/i18n/locales/fa.ts b/packages/core/src/i18n/locales/fa.ts index 6b2783ab68..81d1d442bc 100644 --- a/packages/core/src/i18n/locales/fa.ts +++ b/packages/core/src/i18n/locales/fa.ts @@ -390,5 +390,6 @@ export const fa = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "ارسال", }, }; diff --git a/packages/core/src/i18n/locales/fr.ts b/packages/core/src/i18n/locales/fr.ts index ad605db24a..5f2f00559c 100644 --- a/packages/core/src/i18n/locales/fr.ts +++ b/packages/core/src/i18n/locales/fr.ts @@ -467,5 +467,6 @@ export const fr: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Envoyer", }, }; diff --git a/packages/core/src/i18n/locales/he.ts b/packages/core/src/i18n/locales/he.ts index 4662a94202..e62f1afcb5 100644 --- a/packages/core/src/i18n/locales/he.ts +++ b/packages/core/src/i18n/locales/he.ts @@ -421,5 +421,6 @@ export const he: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "שליחה", }, }; diff --git a/packages/core/src/i18n/locales/hr.ts b/packages/core/src/i18n/locales/hr.ts index 03eb016eed..649ef6c621 100644 --- a/packages/core/src/i18n/locales/hr.ts +++ b/packages/core/src/i18n/locales/hr.ts @@ -435,5 +435,6 @@ export const hr: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Pošalji", }, }; diff --git a/packages/core/src/i18n/locales/is.ts b/packages/core/src/i18n/locales/is.ts index 913b2324b0..f5fee52314 100644 --- a/packages/core/src/i18n/locales/is.ts +++ b/packages/core/src/i18n/locales/is.ts @@ -435,5 +435,6 @@ export const is: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Senda", }, }; diff --git a/packages/core/src/i18n/locales/it.ts b/packages/core/src/i18n/locales/it.ts index 44be22c1bd..b6d76420e0 100644 --- a/packages/core/src/i18n/locales/it.ts +++ b/packages/core/src/i18n/locales/it.ts @@ -443,5 +443,6 @@ export const it: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Invia", }, }; diff --git a/packages/core/src/i18n/locales/ja.ts b/packages/core/src/i18n/locales/ja.ts index ead1f2fb30..a1bc799d42 100644 --- a/packages/core/src/i18n/locales/ja.ts +++ b/packages/core/src/i18n/locales/ja.ts @@ -461,5 +461,6 @@ export const ja: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "送信", }, }; diff --git a/packages/core/src/i18n/locales/ko.ts b/packages/core/src/i18n/locales/ko.ts index 2981ff1c36..15cf0cc0fb 100644 --- a/packages/core/src/i18n/locales/ko.ts +++ b/packages/core/src/i18n/locales/ko.ts @@ -434,5 +434,6 @@ export const ko: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "제출", }, }; diff --git a/packages/core/src/i18n/locales/nl.ts b/packages/core/src/i18n/locales/nl.ts index da599e017c..0be0755e38 100644 --- a/packages/core/src/i18n/locales/nl.ts +++ b/packages/core/src/i18n/locales/nl.ts @@ -422,5 +422,6 @@ export const nl: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Verzenden", }, }; diff --git a/packages/core/src/i18n/locales/no.ts b/packages/core/src/i18n/locales/no.ts index 72efc096ed..1242b9f6a2 100644 --- a/packages/core/src/i18n/locales/no.ts +++ b/packages/core/src/i18n/locales/no.ts @@ -439,5 +439,6 @@ export const no: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Send inn", }, }; diff --git a/packages/core/src/i18n/locales/pl.ts b/packages/core/src/i18n/locales/pl.ts index d00039633c..fc4ff44055 100644 --- a/packages/core/src/i18n/locales/pl.ts +++ b/packages/core/src/i18n/locales/pl.ts @@ -412,5 +412,6 @@ export const pl: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Wyślij", }, }; diff --git a/packages/core/src/i18n/locales/pt.ts b/packages/core/src/i18n/locales/pt.ts index fe719ce023..72caf58af3 100644 --- a/packages/core/src/i18n/locales/pt.ts +++ b/packages/core/src/i18n/locales/pt.ts @@ -414,5 +414,6 @@ export const pt: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Enviar", }, }; diff --git a/packages/core/src/i18n/locales/ru.ts b/packages/core/src/i18n/locales/ru.ts index a4a7987dfc..26faa60bbf 100644 --- a/packages/core/src/i18n/locales/ru.ts +++ b/packages/core/src/i18n/locales/ru.ts @@ -465,5 +465,6 @@ export const ru: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Отправить", }, }; diff --git a/packages/core/src/i18n/locales/sk.ts b/packages/core/src/i18n/locales/sk.ts index 4e73dc7eca..7aff94394b 100644 --- a/packages/core/src/i18n/locales/sk.ts +++ b/packages/core/src/i18n/locales/sk.ts @@ -419,5 +419,6 @@ export const sk = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Odoslať", }, }; diff --git a/packages/core/src/i18n/locales/uk.ts b/packages/core/src/i18n/locales/uk.ts index e9d379ac0b..ce9aee6a8e 100644 --- a/packages/core/src/i18n/locales/uk.ts +++ b/packages/core/src/i18n/locales/uk.ts @@ -445,5 +445,6 @@ export const uk: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Надіслати", }, }; diff --git a/packages/core/src/i18n/locales/uz.ts b/packages/core/src/i18n/locales/uz.ts index 13aee55a73..984f9a844b 100644 --- a/packages/core/src/i18n/locales/uz.ts +++ b/packages/core/src/i18n/locales/uz.ts @@ -455,5 +455,6 @@ export const uz: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Yuborish", }, }; diff --git a/packages/core/src/i18n/locales/vi.ts b/packages/core/src/i18n/locales/vi.ts index 8733fbf0ba..48295ebff7 100644 --- a/packages/core/src/i18n/locales/vi.ts +++ b/packages/core/src/i18n/locales/vi.ts @@ -420,5 +420,6 @@ export const vi: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "Gửi", }, }; diff --git a/packages/core/src/i18n/locales/zh-tw.ts b/packages/core/src/i18n/locales/zh-tw.ts index 5ac37a80c7..9be4dc9fc0 100644 --- a/packages/core/src/i18n/locales/zh-tw.ts +++ b/packages/core/src/i18n/locales/zh-tw.ts @@ -462,5 +462,6 @@ export const zhTW: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "提交", }, }; diff --git a/packages/core/src/i18n/locales/zh.ts b/packages/core/src/i18n/locales/zh.ts index 3f4c90bb56..78498d0e68 100644 --- a/packages/core/src/i18n/locales/zh.ts +++ b/packages/core/src/i18n/locales/zh.ts @@ -462,5 +462,6 @@ export const zh: Dictionary = { }, generic: { ctrl_shortcut: "Ctrl", + form_submit: "提交", }, }; diff --git a/packages/mantine/src/blocknoteStyles.css b/packages/mantine/src/blocknoteStyles.css index beb3c8182f..28974e2a23 100644 --- a/packages/mantine/src/blocknoteStyles.css +++ b/packages/mantine/src/blocknoteStyles.css @@ -257,6 +257,19 @@ on touch devices (e.g. the mobile formatting toolbar). */ font-size: 12px; } +/* On touch devices, enlarge the form-popover inputs (e.g. the link popover's + URL field). The 16px font-size is load-bearing: iOS Safari auto-zooms the + page when focusing an input with a smaller computed font-size, and that zoom + perturbs the visual viewport the mobile toolbar positions itself from. The + taller min-height also gives a comfortable tap target. */ +@media (pointer: coarse) { + .bn-form-popover .mantine-TextInput-input, + .bn-form-popover .mantine-FileInput-input { + font-size: 16px; + min-height: 40px; + } +} + .bn-form-popover .mantine-FileInput-input:hover { background-color: var(--bn-colors-hovered-background); } @@ -806,3 +819,23 @@ we just don't display it in CSS instead. */ .bn-mantine .bn-badge .mantine-Chip-iconWrapper { display: none; } + +/* The submit button `Form.Root` renders so that Enter reaches the form + * regardless of how many fields a popover has. It carries no visual design of + * its own - the popovers commit on Enter - but it stays in the accessibility + * tree with a real label, so screen readers and voice control have a submit + * control to operate. It is out of the tab order: keeping a control nobody can + * see as a tab stop would strand sighted keyboard users on invisible focus, + * and Enter already submits for them. + */ +.bn-form-submit { + border: 0; + clip-path: inset(50%); + height: 1px; + margin: -1px; + overflow: hidden; + padding: 0; + position: absolute; + white-space: nowrap; + width: 1px; +} diff --git a/packages/mantine/src/components.tsx b/packages/mantine/src/components.tsx index 6c85286e7b..f39ec593fa 100644 --- a/packages/mantine/src/components.tsx +++ b/packages/mantine/src/components.tsx @@ -3,6 +3,7 @@ import { Badge, BadgeGroup } from "./badge/Badge.js"; import { Card, CardSection, ExpandSectionsPrompt } from "./comments/Card.js"; import { Comment } from "./comments/Comment.js"; import { Editor } from "./comments/Editor.js"; +import { Form } from "./form/Form.js"; import { TextInput } from "./form/TextInput.js"; import { Menu, @@ -89,7 +90,7 @@ export const components: Components = { Group: BadgeGroup, }, Form: { - Root: (props) =>
{props.children}
, + Root: Form, TextInput: TextInput, }, Menu: { diff --git a/packages/mantine/src/form/Form.tsx b/packages/mantine/src/form/Form.tsx new file mode 100644 index 0000000000..9d903bbced --- /dev/null +++ b/packages/mantine/src/form/Form.tsx @@ -0,0 +1,25 @@ +import { assertEmpty } from "@blocknote/core"; +import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; + +export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { + const { children, onSubmit, ...rest } = props; + const dict = useDictionary(); + const formProps = useFormSubmit(onSubmit); + + assertEmpty(rest); + + return ( +
+ {children} + {/* + Gives the form a submit button, which is what makes Enter submit it at + all once a caller renders more than one field (see the `onSubmit` + contract in `ComponentsContext`). Visually hidden rather than absent, + so assistive technology still has a labelled control to activate. + */} + +
+ ); +}; diff --git a/packages/mantine/src/form/TextInput.tsx b/packages/mantine/src/form/TextInput.tsx index c1630fa17f..60ea49d327 100644 --- a/packages/mantine/src/form/TextInput.tsx +++ b/packages/mantine/src/form/TextInput.tsx @@ -2,7 +2,7 @@ import { TextInput as MantineTextInput } from "@mantine/core"; import { assertEmpty, mergeCSSClasses } from "@blocknote/core"; import { ComponentProps } from "@blocknote/react"; -import { forwardRef } from "react"; +import { forwardRef, useCallback, useEffect, useRef } from "react"; export const TextInput = forwardRef< HTMLInputElement, @@ -20,7 +20,6 @@ export const TextInput = forwardRef< disabled, onKeyDown, onChange, - onSubmit, autoComplete, "aria-activedescendant": ariaActivedescendant, rightSection, @@ -29,6 +28,29 @@ export const TextInput = forwardRef< assertEmpty(rest); + // Focus with `preventScroll`, rather than the native `autofocus`: these + // inputs live in popovers that floating-ui positions *after* mount, so the + // browser's scroll-into-view runs while the popover is still at its + // pre-positioned spot and yanks the page (on mobile, right out from under + // the block being edited). + const inputRef = useRef(null); + const setRefs = useCallback( + (element: HTMLInputElement | null) => { + inputRef.current = element; + if (typeof ref === "function") { + ref(element); + } else if (ref) { + ref.current = element; + } + }, + [ref], + ); + useEffect(() => { + if (autoFocus) { + inputRef.current?.focus({ preventScroll: true }); + } + }, [autoFocus]); + return ( diff --git a/packages/mantine/src/popover/Popover.tsx b/packages/mantine/src/popover/Popover.tsx index 9a10c4ce44..c87da9aa6d 100644 --- a/packages/mantine/src/popover/Popover.tsx +++ b/packages/mantine/src/popover/Popover.tsx @@ -23,6 +23,12 @@ export const Popover = ( // Do not move focus to the dropdown on mobile, as it blurs the editor's // contentEditable and dismisses the on-screen keyboard. trapFocus={portalRoot ? false : undefined} + // Keep the dropdown visible through virtual-keyboard viewport resizes on + // mobile: hideDetached (default true) reacts to the resize by setting + // display:none on the dropdown, which blurs its focused input and + // dismisses the on-screen keyboard (the input then unmounts with the + // toolbar, so the whole UI collapses). + hideDetached={portalRoot ? false : undefined} opened={open} onChange={onOpenChange} position={position} diff --git a/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx b/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx index 9c824ba8bf..0169c96f60 100644 --- a/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx +++ b/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx @@ -7,7 +7,7 @@ import { StyleSchema, filenameFromURL, } from "@blocknote/core"; -import { ChangeEvent, KeyboardEvent, useCallback, useState } from "react"; +import { ChangeEvent, useCallback, useState } from "react"; import { useComponentsContext } from "../../../editor/ComponentsContext.js"; import { useBlockNoteEditor } from "../../../hooks/useBlockNoteEditor.js"; @@ -37,25 +37,7 @@ export const EmbedTab = < [], ); - const handleURLEnter = useCallback( - (event: KeyboardEvent) => { - if (event.key === "Enter" && !event.nativeEvent.isComposing) { - event.preventDefault(); - if (!editor.getBlock(props.blockId)) { - return; - } - editor.updateBlock(props.blockId, { - props: { - name: filenameFromURL(currentURL), - url: currentURL, - } as any, - }); - } - }, - [editor, props.blockId, currentURL], - ); - - const handleURLClick = useCallback(() => { + const embedURL = useCallback(() => { if (!editor.getBlock(props.blockId)) { return; } @@ -73,17 +55,18 @@ export const EmbedTab = < return ( - + + + {dict.file_panel.embed.embed_button[block.type] || diff --git a/packages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsx b/packages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsx index 26ce7e04a5..ef2b7cbab8 100644 --- a/packages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsx +++ b/packages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsx @@ -162,6 +162,9 @@ export const CreateLinkButton = () => { text={state.text} range={state.range} showTextField={false} + // (No explicit popover close here: any editor-state change — like + // submitting the link — already closes it via the setShowPopover + // effect above.) setToolbarOpen={(open) => formattingToolbar.store.setState(open)} /> diff --git a/packages/react/src/components/FormattingToolbar/DefaultButtons/FileCaptionButton.tsx b/packages/react/src/components/FormattingToolbar/DefaultButtons/FileCaptionButton.tsx index bd72ea451c..1065546c53 100644 --- a/packages/react/src/components/FormattingToolbar/DefaultButtons/FileCaptionButton.tsx +++ b/packages/react/src/components/FormattingToolbar/DefaultButtons/FileCaptionButton.tsx @@ -5,7 +5,7 @@ import { InlineContentSchema, StyleSchema, } from "@blocknote/core"; -import { ChangeEvent, KeyboardEvent, useCallback, useState } from "react"; +import { ChangeEvent, useCallback, useState } from "react"; import { RiInputField } from "react-icons/ri"; import { useComponentsContext } from "../../../editor/ComponentsContext.js"; @@ -88,16 +88,6 @@ export const FileCaptionButton = () => { [block, editor], ); - const handleKeyDown = useCallback( - (event: KeyboardEvent) => { - if (event.key === "Enter" && !event.nativeEvent.isComposing) { - event.preventDefault(); - setPopoverOpen(false); - } - }, - [setPopoverOpen], - ); - if (block === undefined) { return null; } @@ -127,14 +117,13 @@ export const FileCaptionButton = () => { className={"bn-popover-content bn-form-popover"} variant={"form-popover"} > - + setPopoverOpen(false)}> } value={block.props.caption} autoFocus={true} placeholder={dict.formatting_toolbar.file_caption.input_placeholder} - onKeyDown={handleKeyDown} onChange={handleChange} /> diff --git a/packages/react/src/components/FormattingToolbar/DefaultButtons/FileRenameButton.tsx b/packages/react/src/components/FormattingToolbar/DefaultButtons/FileRenameButton.tsx index b13bb45a88..0138947c24 100644 --- a/packages/react/src/components/FormattingToolbar/DefaultButtons/FileRenameButton.tsx +++ b/packages/react/src/components/FormattingToolbar/DefaultButtons/FileRenameButton.tsx @@ -5,7 +5,7 @@ import { InlineContentSchema, StyleSchema, } from "@blocknote/core"; -import { ChangeEvent, KeyboardEvent, useCallback, useState } from "react"; +import { ChangeEvent, useCallback, useState } from "react"; import { RiFontFamily } from "react-icons/ri"; import { useComponentsContext } from "../../../editor/ComponentsContext.js"; @@ -88,16 +88,6 @@ export const FileRenameButton = () => { [block, editor], ); - const handleKeyDown = useCallback( - (event: KeyboardEvent) => { - if (event.key === "Enter" && !event.nativeEvent.isComposing) { - event.preventDefault(); - setPopoverOpen(false); - } - }, - [setPopoverOpen], - ); - if (block === undefined) { return null; } @@ -133,7 +123,7 @@ export const FileRenameButton = () => { className={"bn-popover-content bn-form-popover"} variant={"form-popover"} > - + setPopoverOpen(false)}> } @@ -144,7 +134,6 @@ export const FileRenameButton = () => { block.type ] || dict.formatting_toolbar.file_rename.input_placeholder["file"] } - onKeyDown={handleKeyDown} onChange={handleChange} /> diff --git a/packages/react/src/components/LinkToolbar/EditLinkMenuItems.tsx b/packages/react/src/components/LinkToolbar/EditLinkMenuItems.tsx index 1d82a6e7cc..147404d2b8 100644 --- a/packages/react/src/components/LinkToolbar/EditLinkMenuItems.tsx +++ b/packages/react/src/components/LinkToolbar/EditLinkMenuItems.tsx @@ -3,13 +3,7 @@ import { LinkToolbarExtension, VALID_LINK_PROTOCOLS, } from "@blocknote/core/extensions"; -import { - ChangeEvent, - KeyboardEvent, - useCallback, - useEffect, - useState, -} from "react"; +import { ChangeEvent, useCallback, useEffect, useState } from "react"; import { RiLink, RiText } from "react-icons/ri"; import { useComponentsContext } from "../../editor/ComponentsContext.js"; import { useExtension } from "../../hooks/useExtension.js"; @@ -50,18 +44,6 @@ export const EditLinkMenuItems = ( setCurrentText(text); }, [text, url]); - const handleEnter = useCallback( - (event: KeyboardEvent) => { - if (event.key === "Enter" && !event.nativeEvent.isComposing) { - event.preventDefault(); - editLink(validateUrl(currentUrl), currentText, props.range.from); - props.setToolbarOpen?.(false); - props.setToolbarPositionFrozen?.(false); - } - }, - [editLink, currentUrl, currentText, props], - ); - const handleUrlChange = useCallback( (event: ChangeEvent) => setCurrentUrl(event.currentTarget.value), @@ -81,7 +63,7 @@ export const EditLinkMenuItems = ( }, [editLink, currentUrl, currentText, props]); return ( - + {/* // TODO: add labels? */} {showTextField !== false && ( } placeholder={dict.link_toolbar.form.title_placeholder} value={currentText} - onKeyDown={handleEnter} onChange={handleTextChange} - onSubmit={handleSubmit} /> )} diff --git a/packages/react/src/editor/ComponentsContext.tsx b/packages/react/src/editor/ComponentsContext.tsx index 5d71bc58dc..e142605e98 100644 --- a/packages/react/src/editor/ComponentsContext.tsx +++ b/packages/react/src/editor/ComponentsContext.tsx @@ -103,7 +103,7 @@ export type ComponentProps = { value: string; placeholder: string; onChange: (event: ChangeEvent) => void; - onKeyDown: (event: KeyboardEvent) => void; + onKeyDown?: (event: KeyboardEvent) => void; }; }; LinkToolbar: { @@ -304,6 +304,18 @@ export type ComponentProps = { Form: { Root: { children?: ReactNode; + /** + * Called on the form's `submit` event, which is how the browser + * reports Enter-to-submit — including when a mobile IME's action key + * triggers it. Implementations must render a real `
` and + * `preventDefault`, or Enter is left with no submission path at all + * on platforms that don't dispatch a key event for it. + * + * The form context is also what makes Android's IME offer a + * submitting action at all: without it, it advances focus to the next + * element on the page instead (verified on a device). + */ + onSubmit?: () => void; }; TextInput: { className?: string; @@ -316,9 +328,8 @@ export type ComponentProps = { placeholder?: string; disabled?: boolean; value: string; - onKeyDown: (event: KeyboardEvent) => void; + onKeyDown?: (event: KeyboardEvent) => void; onChange: (event: ChangeEvent) => void; - onSubmit?: () => void; autoComplete?: HTMLInputAutoCompleteAttribute; "aria-activedescendant"?: string; ref?: ForwardedRef; diff --git a/packages/react/src/hooks/useFormSubmit.ts b/packages/react/src/hooks/useFormSubmit.ts new file mode 100644 index 0000000000..e2cf4dfbde --- /dev/null +++ b/packages/react/src/hooks/useFormSubmit.ts @@ -0,0 +1,49 @@ +import { FormEvent, useCallback, useMemo, useRef } from "react"; + +/** + * Props for the `` element a `Form.Root` implementation renders, wiring + * up its `onSubmit` contract. + * + * Submission has to be suppressed while an IME composition is in progress. + * Accepting a candidate with Enter reaches the page as a `keydown` with + * `isComposing: true`, and the browser performs implicit form submission for + * it anyway — so a CJK user confirming a candidate would submit the popover + * instead of finishing their word. (Verified in Chromium; see + * tests/src/end-to-end/form/compositionSubmit.test.tsx.) + * + * Composition events bubble, so listening on the form covers every field in + * it. This is deliberately the single place that knowledge lives: the same + * guard used to be repeated in each popover's own Enter handler, which is + * exactly how the callsites drifted out of sync. + */ +export function useFormSubmit(onSubmit?: () => void) { + const composing = useRef(false); + + const handleSubmit = useCallback( + (event: FormEvent) => { + // Always prevent the default: these forms have no action and a real + // navigation would tear down the editor. + event.preventDefault(); + + if (composing.current) { + return; + } + + onSubmit?.(); + }, + [onSubmit], + ); + + return useMemo( + () => ({ + onCompositionStart: () => { + composing.current = true; + }, + onCompositionEnd: () => { + composing.current = false; + }, + onSubmit: handleSubmit, + }), + [handleSubmit], + ); +} diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index e5ba94c223..a72c2ca67a 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -136,6 +136,7 @@ export * from "./hooks/useCreateBlockNote.js"; export * from "./hooks/useEditorChange.js"; export * from "./hooks/useEditorFocus.js"; export * from "./hooks/useEditorFocusChange.js"; +export * from "./hooks/useFormSubmit.js"; export * from "./hooks/useEditorDomElement.js"; export * from "./hooks/useEditorSelectionBoundingBox.js"; export * from "./hooks/useEditorSelectionChange.js"; diff --git a/packages/shadcn/src/form/Form.tsx b/packages/shadcn/src/form/Form.tsx index 0ad9930b0e..9d903bbced 100644 --- a/packages/shadcn/src/form/Form.tsx +++ b/packages/shadcn/src/form/Form.tsx @@ -1,10 +1,25 @@ import { assertEmpty } from "@blocknote/core"; -import { ComponentProps } from "@blocknote/react"; +import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, ...rest } = props; + const { children, onSubmit, ...rest } = props; + const dict = useDictionary(); + const formProps = useFormSubmit(onSubmit); assertEmpty(rest); - return <>{children}; + return ( + + {children} + {/* + Gives the form a submit button, which is what makes Enter submit it at + all once a caller renders more than one field (see the `onSubmit` + contract in `ComponentsContext`). Visually hidden rather than absent, + so assistive technology still has a labelled control to activate. + */} + + + ); }; diff --git a/packages/shadcn/src/form/TextInput.tsx b/packages/shadcn/src/form/TextInput.tsx index 675e7409fa..c441385922 100644 --- a/packages/shadcn/src/form/TextInput.tsx +++ b/packages/shadcn/src/form/TextInput.tsx @@ -1,6 +1,6 @@ import { assertEmpty } from "@blocknote/core"; import { ComponentProps } from "@blocknote/react"; -import { forwardRef } from "react"; +import { forwardRef, useCallback, useEffect, useRef } from "react"; import { useShadCNComponentsContext } from "../ShadCNComponentsContext.js"; import { cn } from "../lib/utils.js"; @@ -21,7 +21,6 @@ export const TextInput = forwardRef< disabled, onKeyDown, onChange, - onSubmit, autoComplete: _autoComplete, "aria-activedescendant": ariaActivedescendant, rightSection, // TODO: add rightSection @@ -30,6 +29,29 @@ export const TextInput = forwardRef< assertEmpty(rest); + // Focus with `preventScroll`, rather than the native `autofocus`: these + // inputs live in popovers that floating-ui positions *after* mount, so the + // browser's scroll-into-view runs while the popover is still at its + // pre-positioned spot and yanks the page (on mobile, right out from under + // the block being edited). + const inputRef = useRef(null); + const setRefs = useCallback( + (element: HTMLInputElement | null) => { + inputRef.current = element; + if (typeof ref === "function") { + ref(element); + } else if (ref) { + ref.current = element; + } + }, + [ref], + ); + useEffect(() => { + if (autoFocus) { + inputRef.current?.focus({ preventScroll: true }); + } + }, [autoFocus]); + const ShadCNComponents = useShadCNComponentsContext()!; return ( @@ -51,14 +73,12 @@ export const TextInput = forwardRef< className={cn(className, "h-auto border-none p-0")} id={label} name={name} - autoFocus={autoFocus} placeholder={placeholder} disabled={disabled} value={value} onKeyDown={onKeyDown} onChange={onChange} - onSubmit={onSubmit} - ref={ref} + ref={setRefs} aria-activedescendant={ariaActivedescendant} />
diff --git a/packages/shadcn/src/style.css b/packages/shadcn/src/style.css index b675e6d513..e9a11db477 100644 --- a/packages/shadcn/src/style.css +++ b/packages/shadcn/src/style.css @@ -73,3 +73,23 @@ color: var(--bn-colors-highlights-red-background); font-weight: bold; } + +/* The submit button `Form.Root` renders so that Enter reaches the form + * regardless of how many fields a popover has. It carries no visual design of + * its own - the popovers commit on Enter - but it stays in the accessibility + * tree with a real label, so screen readers and voice control have a submit + * control to operate. It is out of the tab order: keeping a control nobody can + * see as a tab stop would strand sighted keyboard users on invisible focus, + * and Enter already submits for them. + */ +.bn-form-submit { + border: 0; + clip-path: inset(50%); + height: 1px; + margin: -1px; + overflow: hidden; + padding: 0; + position: absolute; + white-space: nowrap; + width: 1px; +} diff --git a/packages/xl-ai/src/components/AIMenu/PromptSuggestionMenu.tsx b/packages/xl-ai/src/components/AIMenu/PromptSuggestionMenu.tsx index 7f68224498..515fae7d1c 100644 --- a/packages/xl-ai/src/components/AIMenu/PromptSuggestionMenu.tsx +++ b/packages/xl-ai/src/components/AIMenu/PromptSuggestionMenu.tsx @@ -38,16 +38,6 @@ export const PromptSuggestionMenu = (props: PromptSuggestionMenuProps) => { const [internalPromptText, setInternalPromptText] = useState(""); const promptTextToUse = promptText || internalPromptText; - const handleEnter = useCallback( - async (event: KeyboardEvent) => { - if (event.key === "Enter" && !event.nativeEvent.isComposing) { - // console.log("ENTER", currentEditingPrompt); - onManualPromptSubmit(promptTextToUse); - } - }, - [promptTextToUse, onManualPromptSubmit], - ); - const handleChange = useCallback( (event: ChangeEvent) => { const newValue = event.currentTarget.value; @@ -75,21 +65,38 @@ export const PromptSuggestionMenu = (props: PromptSuggestionMenuProps) => { ? `bn-suggestion-menu-item-${selectedIndex}` : undefined; + /** + * What Enter does here depends on whether the menu is showing anything: + * with suggestions it picks the highlighted one, and without it submits + * whatever was typed as a prompt. + * + * Both cases are decided in {@link submit}, so that the form's `submit` + * event - which is the only signal a mobile IME's action key produces - + * makes the same choice a key press does. + */ + const submit = useCallback(() => { + if (items.length > 0) { + items[selectedIndex]?.onItemClick(); + } else { + onManualPromptSubmit(promptTextToUse); + } + }, [items, selectedIndex, onManualPromptSubmit, promptTextToUse]); + const handleKeyDown = useCallback( (event: KeyboardEvent) => { // TODO: handle backspace to close - if (event.key === "Enter" && !event.nativeEvent.isComposing) { - if (items.length > 0) { - handler(event); - } else { - // TODO: check focus? - void handleEnter(event); - } - } else { - handler(event); + if ( + event.key === "Enter" && + !event.nativeEvent.isComposing && + items.length === 0 + ) { + // `handler` swallows Enter unconditionally, so with nothing to pick it + // has to be left alone for the event to reach the form. + return; } + handler(event); }, - [handleEnter, handler, items.length], + [handler, items.length], ); // Resets index when items change @@ -114,7 +121,7 @@ export const PromptSuggestionMenu = (props: PromptSuggestionMenuProps) => { return (
- + { + form?.remove(); + form = undefined; +}); + +function buildForm() { + form = document.createElement("form"); + const submits: string[] = []; + const compositions: string[] = []; + // Mirrors what `useFormSubmit` wires onto a real `Form.Root`. + let composing = false; + form.addEventListener("compositionstart", () => (composing = true)); + form.addEventListener("compositionend", () => (composing = false)); + form.addEventListener("submit", (event) => { + event.preventDefault(); + if (composing) { + return; + } + submits.push("submit"); + }); + + const input = document.createElement("input"); + input.type = "text"; + input.name = "url"; + input.addEventListener("compositionstart", () => + compositions.push("compositionstart"), + ); + input.addEventListener("compositionend", () => + compositions.push("compositionend"), + ); + form.append(input); + + // What `Form.Root` renders, so that this mirrors a real popover form. + const button = document.createElement("button"); + button.type = "submit"; + button.tabIndex = -1; + form.append(button); + + document.body.append(form); + return { input, submits, compositions }; +} + +describeIme("Enter during an IME composition", () => { + test("accepting a candidate does not submit the form", async () => { + const { input, submits, compositions } = buildForm(); + input.focus(); + + // Accepting a candidate the way an IME does: the final text replaces the + // composing text, and the confirming key never reaches the page. + await browserCommands.imeComposition([ + { type: "setComposition", text: "にほん" }, + { type: "commit", text: "日本" }, + ]); + + expect(compositions).toContain("compositionstart"); + expect(input.value).toBe("日本"); + expect( + submits, + "accepting an IME candidate must not submit the popover", + ).toEqual([]); + }); + + test("Enter arriving mid-composition does not submit the form", async () => { + // The case that makes the guard necessary rather than defensive: the + // browser delivers this Enter as `keydown` with `isComposing: true` and + // performs implicit submission for it regardless, so without the guard a + // CJK user accepting a candidate submits the popover mid-word. + const { input, submits, compositions } = buildForm(); + const composingOnKeyDown: boolean[] = []; + input.addEventListener("keydown", (event) => + composingOnKeyDown.push(event.isComposing), + ); + input.focus(); + + await browserCommands.imeComposition([ + { type: "setComposition", text: "にほん" }, + ]); + await userEvent.keyboard("{Enter}"); + + // Pin the precondition too: if a future engine stopped delivering this + // Enter to the page, the guard would be untested rather than unnecessary. + expect( + composingOnKeyDown, + "Enter must reach the page mid-composition", + ).toEqual([true]); + expect(compositions).not.toContain("compositionend"); + expect( + submits, + "Enter must not submit while a composition is in progress", + ).toEqual([]); + }); + + test("Enter after the composition ends does submit", async () => { + // The other half of the contract: once composition is over, Enter has to + // work normally, or CJK users could never submit at all. + const { input, submits } = buildForm(); + input.focus(); + + await browserCommands.imeComposition([ + { type: "setComposition", text: "にほん" }, + { type: "commit", text: "日本" }, + ]); + await userEvent.keyboard("{Enter}"); + + expect(submits).toEqual(["submit"]); + }); +}); diff --git a/tests/src/end-to-end/form/implicitSubmit.test.tsx b/tests/src/end-to-end/form/implicitSubmit.test.tsx new file mode 100644 index 0000000000..0091450333 --- /dev/null +++ b/tests/src/end-to-end/form/implicitSubmit.test.tsx @@ -0,0 +1,135 @@ +import { afterEach, describe, expect, test } from "vite-plus/test"; +import { userEvent } from "../../utils/context.js"; + +/** + * The platform rules that `Form.Root` is built on. + * + * Since the toolbar popovers submit through the form's `submit` event rather + * than a key handler (a mobile IME's action key fires the former and not the + * latter), "does Enter reach `submit`?" became load-bearing. The answer is not + * uniform: HTML only submits implicitly when the form has a submit button, or + * exactly one field that blocks implicit submission + * (https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#implicit-submission). + * + * So these assert the rule per engine rather than trusting the spec — the + * multi-field case is exactly the link toolbar's URL + title form, and the + * hidden-button case is what `Form.Root` renders to make submission work + * regardless of how many fields a caller puts in it. + */ + +const forms: HTMLFormElement[] = []; + +afterEach(() => { + while (forms.length) { + forms.pop()!.remove(); + } +}); + +type SubmitButton = "none" | "hidden" | "visually-hidden"; + +function buildForm( + inputCount: number, + submitButton: SubmitButton, + tabIndex?: number, +) { + const form = document.createElement("form"); + const submits: string[] = []; + form.addEventListener("submit", (event) => { + event.preventDefault(); + submits.push("submit"); + }); + + const inputs: HTMLInputElement[] = []; + for (let i = 0; i < inputCount; i++) { + const input = document.createElement("input"); + input.type = "text"; + input.name = `field-${i}`; + form.append(input); + inputs.push(input); + } + + if (submitButton !== "none") { + const button = document.createElement("button"); + button.type = "submit"; + if (tabIndex !== undefined) { + button.tabIndex = tabIndex; + } + if (submitButton === "hidden") { + button.hidden = true; + } else { + button.style.cssText = + "position:absolute;width:1px;height:1px;overflow:hidden;clip:rect(0 0 0 0)"; + } + form.append(button); + } + + document.body.append(form); + forms.push(form); + return { inputs, submits }; +} + +async function pressEnterIn(input: HTMLInputElement) { + input.focus(); + await userEvent.keyboard("{Enter}"); +} + +describe("Implicit form submission", () => { + test("a single field submits without a submit button", async () => { + const { inputs, submits } = buildForm(1, "none"); + + await pressEnterIn(inputs[0]); + + expect(submits).toEqual(["submit"]); + }); + + test("several fields do NOT submit without a submit button", async () => { + // The reason `Form.Root` cannot just be a bare `
`: the link + // toolbar's edit form has two fields, so Enter would reach nothing. + const { inputs, submits } = buildForm(2, "none"); + + await pressEnterIn(inputs[0]); + + expect(submits).toEqual([]); + }); + + test("several fields submit once a hidden submit button is present", async () => { + const { inputs, submits } = buildForm(2, "hidden"); + + await pressEnterIn(inputs[0]); + expect(submits).toEqual(["submit"]); + + // From the last field too, where a mobile IME offers its action key. + await pressEnterIn(inputs[1]); + expect(submits).toEqual(["submit", "submit"]); + }); + + test("several fields submit with a visually hidden submit button", async () => { + // What `Form.Root` actually renders: clipped rather than `display: none`, + // so assistive technology still sees a submit control. Keeping it out of + // the layout must not cost the implicit submission that `hidden` provided. + const { inputs, submits } = buildForm(2, "visually-hidden"); + + await pressEnterIn(inputs[0]); + + expect(submits).toEqual(["submit"]); + }); + + test("a submit button outside the tab order still submits", async () => { + // `Form.Root` sets `tabIndex={-1}` on it, so that a control nobody can see + // never becomes a tab stop. Implicit submission looks for the form's + // default button and must not care about that. + const { inputs, submits } = buildForm(2, "visually-hidden", -1); + + await pressEnterIn(inputs[0]); + + expect(submits).toEqual(["submit"]); + }); + + test("a submit button does not make Enter submit twice", async () => { + const { inputs, submits } = buildForm(1, "hidden"); + + await pressEnterIn(inputs[0]); + + expect(submits).toEqual(["submit"]); + }); +}); diff --git a/tests/src/end-to-end/form/popoverSubmit.test.tsx b/tests/src/end-to-end/form/popoverSubmit.test.tsx new file mode 100644 index 0000000000..6f9e9b9446 --- /dev/null +++ b/tests/src/end-to-end/form/popoverSubmit.test.tsx @@ -0,0 +1,149 @@ +import TestingApp from "@examples/01-basic/testing/src/App"; +import { beforeEach, describe, expect, test, vi } from "vite-plus/test"; +import { render } from "vitest-browser-react"; + +import { userEvent } from "../../utils/context.js"; +import { EDITOR_SELECTOR, LINK_BUTTON_SELECTOR } from "../../utils/const.js"; +import { focusOnEditor, waitForSelector } from "../../utils/editor.js"; +import { executeSlashCommand } from "../../utils/slashmenu.js"; + +/** + * The toolbar popovers commit through their form's `submit` event, because a + * mobile IME's action key fires that and no key event at all. + * + * These drive Enter rather than calling the handlers, so they cover the whole + * path a browser takes to reach `onSubmit` — including whether the form is + * eligible for implicit submission at all, which depends on how many fields + * the popover happens to render (see ./implicitSubmit.test.tsx). + */ + +beforeEach(async () => { + await render(); + await waitForSelector(EDITOR_SELECTOR); +}); + +async function createLink(url: string) { + await focusOnEditor(); + await userEvent.keyboard("link me"); + await userEvent.keyboard("{Home}{Shift>}{End}{/Shift}"); + await userEvent.click(await waitForSelector(LINK_BUTTON_SELECTOR)); + const input = (await waitForSelector( + 'input[name="url"]', + )) as HTMLInputElement; + await userEvent.click(input); + await userEvent.keyboard(`${url}{Enter}`); + return waitForSelector(`a[href="https://${url}"]`); +} + +describe("Submitting a toolbar popover with Enter", () => { + test("the link edit form commits, though it has two fields", async () => { + // The regression this guards: HTML only submits a form implicitly when it + // has a submit button *or* exactly one field. The create form has one + // field (url) and submits on its own; this edit form adds the title + // field, so without the submit button `Form.Root` renders, Enter reaches + // nothing and the edit is silently dropped. + const link = await createLink("example.com"); + + await userEvent.hover(link); + await vi.waitFor(() => { + const editButton = [ + ...document.querySelectorAll(".bn-toolbar button"), + ].find((button) => button.textContent?.trim() === "Edit link"); + if (!editButton) { + throw new Error("the link toolbar's edit button never appeared"); + } + editButton.click(); + }); + + const urlInput = (await waitForSelector( + 'input[name="url"]', + )) as HTMLInputElement; + // Both fields are present — that is what makes this case different. + expect(document.querySelector('input[name="title"]')).not.toBeNull(); + + await userEvent.tripleClick(urlInput); + await userEvent.keyboard("edited.com{Enter}"); + + await vi.waitFor(() => { + if (!document.querySelector('a[href="https://edited.com"]')) { + throw new Error("Enter did not commit the two-field edit form"); + } + }); + }); + + test("the submit control stays available to assistive technology", async () => { + // `display: none` would take the button out of the accessibility tree + // entirely, leaving Enter as the only way to commit — nothing for a + // screen reader or voice control to target. It has to be clipped instead, + // and carry a real accessible name. + await createLink("example.com"); + + await userEvent.hover( + await waitForSelector('a[href="https://example.com"]'), + ); + await vi.waitFor(() => { + const editButton = [ + ...document.querySelectorAll(".bn-toolbar button"), + ].find((button) => button.textContent?.trim() === "Edit link"); + if (!editButton) { + throw new Error("the link toolbar's edit button never appeared"); + } + editButton.click(); + }); + const input = await waitForSelector('input[name="url"]'); + + const submit = input.closest("form")!.querySelector("button[type=submit]"); + expect(submit, "the form must expose a submit control").not.toBeNull(); + + const styles = getComputedStyle(submit!); + expect(styles.display).not.toBe("none"); + expect(styles.visibility).not.toBe("hidden"); + expect(submit!.textContent?.trim(), "it needs an accessible name").toBe( + "Submit", + ); + // Out of the tab order, so sighted keyboard users never land on a control + // they can't see. + expect((submit as HTMLButtonElement).tabIndex).toBe(-1); + }); + + test("the embed tab's URL field commits", async () => { + // The embed tab used to be the one input with an Enter handler and no + // form at all, so its action key did nothing on mobile. + await focusOnEditor(); + await executeSlashCommand("image"); + + await userEvent.click(await waitForSelector(`[data-test="embed-tab"]`)); + const input = (await waitForSelector( + `[data-test="embed-input"]`, + )) as HTMLInputElement; + await userEvent.click(input); + + const url = "https://placehold.co/800x540.png"; + await userEvent.keyboard(`${url}{Enter}`); + + await waitForSelector(`img[src="${url}"]`); + }); + + test("the embed tab commits exactly once", async () => { + // The embed button sits outside the form on purpose: the skins disagree on + // whether their panel button defaults to `type="submit"`, so inside one it + // would fire `onClick` *and* submit, applying the same edit twice. + await focusOnEditor(); + await executeSlashCommand("image"); + + await userEvent.click(await waitForSelector(`[data-test="embed-tab"]`)); + const input = (await waitForSelector( + `[data-test="embed-input"]`, + )) as HTMLInputElement; + await userEvent.click(input); + + const url = "https://placehold.co/400x300.png"; + await userEvent.keyboard(url); + await userEvent.click( + await waitForSelector(`[data-test="embed-input-button"]`), + ); + + await waitForSelector(`img[src="${url}"]`); + expect(document.querySelectorAll(`img[src="${url}"]`).length).toBe(1); + }); +}); diff --git a/tests/src/end-to-end/mobile/linkSubmit.test.tsx b/tests/src/end-to-end/mobile/linkSubmit.test.tsx new file mode 100644 index 0000000000..36418b731e --- /dev/null +++ b/tests/src/end-to-end/mobile/linkSubmit.test.tsx @@ -0,0 +1,131 @@ +import App from "@examples/03-ui-components/14-mobile-formatting-toolbar/src/App"; +import { + afterEach, + beforeEach, + describe, + expect, + test, + vi, +} from "vite-plus/test"; +import { render } from "vitest-browser-react"; + +import { page, userEvent } from "../../utils/context.js"; +import { EDITOR_SELECTOR, LINK_BUTTON_SELECTOR } from "../../utils/const.js"; +import { waitForSelector } from "../../utils/editor.js"; +import { ensureTouchEmulation } from "../../utils/ensureTouchEmulation.js"; + +const MOBILE_TOOLBAR_SELECTOR = ".bn-mobile-formatting-toolbar"; + +// Submitting the link popover from an editor that is *not* the last on the +// page. Reported from a device: the link was never created and focus jumped +// to the second editor instead. +// +// Coverage limit worth knowing: the device-only half of that bug is which +// action Android's IME assigns to the Enter key. Being inside a real +// is what makes it offer a submitting action instead of "Next" (advance +// focus, no key event at all) — confirmed on a device, where the popover +// commits from the first editor with no `enterkeyhint` hinting involved. +// +// No automated environment we have can exercise that choice: emulation always +// dispatches a real Enter, and on BrowserStack no input channel reaches the +// on-screen keyboard (see tests/device/README.md). What a test *can* hold onto +// is that submission works without a key event at all, which is the second +// test below; the IME's choice itself stays a release-checklist item. + +beforeEach(async () => { + ensureTouchEmulation(); + await page.viewport(393, 727); +}); + +afterEach(async () => { + await page.viewport(393, 727); +}); + +describe("Submitting the link popover", () => { + test("creates the link in its own editor and keeps focus there", async () => { + await render(); + await vi.waitFor(() => { + if (document.querySelectorAll(EDITOR_SELECTOR).length < 2) { + throw new Error("expected the example's two editors"); + } + }); + const [first, second] = + document.querySelectorAll(EDITOR_SELECTOR); + + await userEvent.click(first.querySelector("p")!); + await userEvent.keyboard( + "{Home}{Shift>}{ArrowRight}{ArrowRight}{ArrowRight}{/Shift}", + ); + await page.viewport(393, 427); + await waitForSelector(MOBILE_TOOLBAR_SELECTOR); + + await userEvent.click( + await waitForSelector( + `${MOBILE_TOOLBAR_SELECTOR} ${LINK_BUTTON_SELECTOR}`, + ), + ); + const input = (await waitForSelector( + 'input[name="url"]', + )) as HTMLInputElement; + + await userEvent.click(input); + await userEvent.keyboard("example.com{Enter}"); + + await vi.waitFor(() => { + if (!first.querySelector('a[href="https://example.com"]')) { + throw new Error( + "link was not created in the editor it was opened from", + ); + } + }); + expect(second.querySelector('a[href="https://example.com"]')).toBeNull(); + + // Focus must not have escaped into the other editor. + expect(document.activeElement?.closest(EDITOR_SELECTOR)).not.toBe(second); + }); + + // The path a mobile IME actually takes. When its action key means "submit", + // the browser submits the form — it does not necessarily deliver an Enter + // keydown, so a popover that only listens for that key has no way to + // commit. Driving the form's own submit is how that arrives, and it is the + // part of the device-only bug a test can reproduce: without a real + // wired to a submit handler, nothing happens at all. + test("submitting the form creates the link, without any key event", async () => { + await render(); + await waitForSelector(EDITOR_SELECTOR); + const [first] = document.querySelectorAll(EDITOR_SELECTOR); + + await userEvent.click(first.querySelector("p")!); + await userEvent.keyboard( + "{Home}{Shift>}{ArrowRight}{ArrowRight}{ArrowRight}{/Shift}", + ); + await page.viewport(393, 427); + await waitForSelector(MOBILE_TOOLBAR_SELECTOR); + await userEvent.click( + await waitForSelector( + `${MOBILE_TOOLBAR_SELECTOR} ${LINK_BUTTON_SELECTOR}`, + ), + ); + const input = (await waitForSelector( + 'input[name="url"]', + )) as HTMLInputElement; + await userEvent.click(input); + await userEvent.keyboard("example.com"); + + const form = input.closest("form"); + expect( + form, + "the popover must be a real , or the browser has no way to " + + "submit it when a mobile IME's action key asks it to", + ).not.toBeNull(); + + // No Enter anywhere: this is the browser submitting the form itself. + form!.requestSubmit(); + + await vi.waitFor(() => { + if (!first.querySelector('a[href="https://example.com"]')) { + throw new Error("submitting the form did not create the link"); + } + }); + }); +}); diff --git a/tests/src/end-to-end/mobile/mobileToolbar.test.tsx b/tests/src/end-to-end/mobile/mobileToolbar.test.tsx new file mode 100644 index 0000000000..57ce398af5 --- /dev/null +++ b/tests/src/end-to-end/mobile/mobileToolbar.test.tsx @@ -0,0 +1,198 @@ +import App from "@examples/01-basic/testing/src/App"; +import { afterEach, beforeEach, describe, test, vi } from "vite-plus/test"; +import { render } from "vitest-browser-react"; + +import { page, userEvent } from "../../utils/context.js"; +import { EDITOR_SELECTOR, LINK_BUTTON_SELECTOR } from "../../utils/const.js"; +import { focusOnEditor, waitForSelector } from "../../utils/editor.js"; +import { ensureTouchEmulation } from "../../utils/ensureTouchEmulation.js"; + +const MOBILE_TOOLBAR_SELECTOR = ".bn-mobile-formatting-toolbar"; +const LINK_POPOVER_SELECTOR = ".bn-form-popover"; + +// Runs in the "android" browser instance (Android UA + touch emulation at +// context level — see vite.config.browser.ts), so `isTouchDevice()` is +// genuinely true. The on-screen keyboard is emulated by resizing the +// viewport: `useVirtualKeyboard` treats a >150px height drop as the keyboard +// opening — which is exactly how a real keyboard manifests with +// `interactive-widget=resizes-content`. The extra ±60px step mimics Gboard +// showing its suggestion strip when focus moves into an input: the resize +// that used to make Mantine's `hideDetached` hide the link popover, blurring +// its focused input and collapsing the keyboard, toolbar, and popover (the +// Android Chrome bug behind PR #2982). +const VIEWPORT_WIDTH = 393; +const KEYBOARD_CLOSED = 727; +const KEYBOARD_OPEN = 427; +const KEYBOARD_OPEN_WITH_SUGGESTION_STRIP = 367; + +// Lets a viewport resize propagate: the resize event, the floating-ui +// autoUpdate pass it triggers, and React's commit each take a frame. +async function settleFrames(count = 3) { + for (let i = 0; i < count; i++) { + await new Promise(requestAnimationFrame); + } +} + +function activeUrlInput() { + const active = document.activeElement; + return active instanceof HTMLInputElement && active.name === "url" + ? active + : undefined; +} + +beforeEach(async () => { + ensureTouchEmulation(); + await page.viewport(VIEWPORT_WIDTH, KEYBOARD_CLOSED); + await render(); + await waitForSelector(EDITOR_SELECTOR); +}); + +afterEach(async () => { + await page.viewport(VIEWPORT_WIDTH, KEYBOARD_CLOSED); +}); + +describe("Mobile formatting toolbar", () => { + test("shows while the virtual keyboard is open and hides when it closes", async () => { + await focusOnEditor(); + await userEvent.keyboard("Mobile toolbar"); + + await page.viewport(VIEWPORT_WIDTH, KEYBOARD_OPEN); + await waitForSelector(MOBILE_TOOLBAR_SELECTOR); + + await page.viewport(VIEWPORT_WIDTH, KEYBOARD_CLOSED); + await vi.waitFor(() => { + if (document.querySelector(MOBILE_TOOLBAR_SELECTOR)) { + throw new Error( + "mobile toolbar still visible after the keyboard closed", + ); + } + }); + }); + + test("link popover holds focus through keyboard resizes and creates the link", async () => { + await focusOnEditor(); + await userEvent.keyboard("Link target"); + await userEvent.keyboard("{Shift>}{Home}{/Shift}"); + + await page.viewport(VIEWPORT_WIDTH, KEYBOARD_OPEN); + await waitForSelector(MOBILE_TOOLBAR_SELECTOR); + + await userEvent.click( + await waitForSelector( + `${MOBILE_TOOLBAR_SELECTOR} ${LINK_BUTTON_SELECTOR}`, + ), + ); + + // The URL input autofocuses when the popover opens. + await vi.waitFor(() => { + if (!activeUrlInput()) { + throw new Error("URL input did not receive focus on popover open"); + } + }); + + // iOS Safari auto-zooms the page when an input with a computed font-size + // under 16px takes focus, and that zoom perturbs the visual viewport the + // toolbar positions itself from. Emulation can't reproduce the zoom + // itself (it's device behaviour, not engine behaviour — the real-device + // suite asserts visualViewport.scale directly), so this guards the CSS + // contract that prevents it. + { + const fontSize = parseFloat(getComputedStyle(activeUrlInput()!).fontSize); + if (fontSize < 16) { + throw new Error( + `URL input font-size is ${fontSize}px; iOS Safari auto-zooms below ` + + `16px (see the pointer:coarse rule in blocknoteStyles.css)`, + ); + } + } + + // Focusing an input makes the keyboard show its suggestion strip, then + // settle back. The focused input must survive both resizes. + await page.viewport(VIEWPORT_WIDTH, KEYBOARD_OPEN_WITH_SUGGESTION_STRIP); + await settleFrames(); + if (!activeUrlInput()) { + throw new Error("URL input lost focus when the suggestion strip resized"); + } + + await page.viewport(VIEWPORT_WIDTH, KEYBOARD_OPEN); + await settleFrames(); + if (!activeUrlInput()) { + throw new Error( + "URL input lost focus when the suggestion strip resize settled", + ); + } + + await userEvent.keyboard("example.com"); + await userEvent.keyboard("{Enter}"); + + await waitForSelector(`${EDITOR_SELECTOR} a[href="https://example.com"]`); + + // Submitting closes the popover but leaves the toolbar up: on mobile the + // toolbar stays mounted (unlike desktop, which unmounts it and the popover + // with it), so the popover must close itself — the lingering popover + // otherwise covers the toolbar and swallows taps on its buttons. + await vi.waitFor(() => { + if (document.querySelector(LINK_POPOVER_SELECTOR)) { + throw new Error("link popover still open after submitting"); + } + if (!document.querySelector(MOBILE_TOOLBAR_SELECTOR)) { + throw new Error("mobile toolbar disappeared after submitting a link"); + } + }); + + // Reopening the popover with the whole link selected must pre-fill its + // URL: `getSelectedLinkUrl` reads the mark just inside the selection + // start, since a lookup exactly at the link's left boundary misses it. + await userEvent.keyboard("{Shift>}{Home}{/Shift}"); + await userEvent.click( + await waitForSelector( + `${MOBILE_TOOLBAR_SELECTOR} ${LINK_BUTTON_SELECTOR}`, + ), + ); + await vi.waitFor(() => { + const input = activeUrlInput(); + if (input?.value !== "https://example.com") { + throw new Error( + `URL input not pre-filled for a fully selected link (value: ${JSON.stringify(input?.value)})`, + ); + } + }); + }); + + // Closing the popover from its trigger must hand focus back to the editor: + // on a real device, focus resting on the toolbar button closes the + // on-screen keyboard (a button can't take text input) and the whole + // editing session collapses with it. + test("toggling the link popover closed returns focus to the editor", async () => { + await focusOnEditor(); + await userEvent.keyboard("Link target"); + await userEvent.keyboard("{Shift>}{Home}{/Shift}"); + + await page.viewport(VIEWPORT_WIDTH, KEYBOARD_OPEN); + await waitForSelector(MOBILE_TOOLBAR_SELECTOR); + const linkButton = await waitForSelector( + `${MOBILE_TOOLBAR_SELECTOR} ${LINK_BUTTON_SELECTOR}`, + ); + + await userEvent.click(linkButton); + await vi.waitFor(() => { + if (!(document.activeElement instanceof HTMLInputElement)) { + throw new Error("URL input did not receive focus on popover open"); + } + }); + + await userEvent.click(linkButton); + await vi.waitFor(() => { + if (document.querySelector('input[name="url"]')) { + throw new Error("popover did not close on trigger toggle"); + } + if (!document.activeElement?.closest(EDITOR_SELECTOR)) { + throw new Error( + `focus did not return to the editor (active: ${String( + document.activeElement?.className, + ).slice(0, 40)})`, + ); + } + }); + }); +}); diff --git a/tests/src/end-to-end/mobile/popoverScroll.test.tsx b/tests/src/end-to-end/mobile/popoverScroll.test.tsx new file mode 100644 index 0000000000..f77703be48 --- /dev/null +++ b/tests/src/end-to-end/mobile/popoverScroll.test.tsx @@ -0,0 +1,86 @@ +import App from "@examples/03-ui-components/14-mobile-formatting-toolbar/src/App"; +import { + afterEach, + beforeEach, + describe, + expect, + test, + vi, +} from "vite-plus/test"; +import { render } from "vitest-browser-react"; + +import { page, userEvent } from "../../utils/context.js"; +import { EDITOR_SELECTOR, LINK_BUTTON_SELECTOR } from "../../utils/const.js"; +import { waitForSelector } from "../../utils/editor.js"; +import { ensureTouchEmulation } from "../../utils/ensureTouchEmulation.js"; + +const MOBILE_TOOLBAR_SELECTOR = ".bn-mobile-formatting-toolbar"; + +// Uses the mobile-formatting-toolbar example because it is a realistic page: +// long static text with editors partway down, and two of them. Opening a +// toolbar popover there used to reset the page scroll to the top, taking the +// block being edited off screen entirely — the popover's input autofocused +// while floating-ui had not positioned the popover yet, so the browser's +// scroll-into-view chased it to its pre-positioned spot. + +beforeEach(async () => { + ensureTouchEmulation(); + await page.viewport(393, 727); +}); + +afterEach(async () => { + await page.viewport(393, 727); +}); + +describe("Opening a toolbar popover", () => { + test("does not scroll the page away from the block being edited", async () => { + await render(); + await vi.waitFor(() => { + if (document.querySelectorAll(EDITOR_SELECTOR).length < 2) { + throw new Error("expected the example's two editors"); + } + }); + + const editor = document.querySelectorAll(EDITOR_SELECTOR)[0]; + await userEvent.click(editor.querySelector("p")!); + await userEvent.keyboard( + "{Home}{Shift>}{ArrowRight}{ArrowRight}{ArrowRight}{/Shift}", + ); + + // "Keyboard opens". + await page.viewport(393, 427); + await waitForSelector(MOBILE_TOOLBAR_SELECTOR); + + // The example defaults to the pinned scroll-container layout, where that + // element scrolls rather than the document. + const scroller = + document.querySelector(".bn-scroll-container") ?? + document.scrollingElement!; + const scrollBefore = scroller.scrollTop; + const editorTopBefore = editor.getBoundingClientRect().top; + // The regression only shows when the page is actually scrolled. + expect(scrollBefore).toBeGreaterThan(0); + + await userEvent.click( + await waitForSelector( + `${MOBILE_TOOLBAR_SELECTOR} ${LINK_BUTTON_SELECTOR}`, + ), + ); + await vi.waitFor(() => { + if (!document.querySelector('input[name="url"]')) { + throw new Error("link popover did not open"); + } + }); + // Let any scroll-into-view settle before measuring. + await new Promise((resolve) => setTimeout(resolve, 400)); + + expect( + Math.abs(scroller.scrollTop - scrollBefore), + `opening the popover scrolled the page (${scrollBefore} -> ${scroller.scrollTop})`, + ).toBeLessThanOrEqual(2); + expect( + Math.abs(editor.getBoundingClientRect().top - editorTopBefore), + "the edited editor moved on screen when the popover opened", + ).toBeLessThanOrEqual(2); + }); +}); From 56e95879cb9d5c79a81fc13061daaea2489d5eda Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 31 Aug 2026 16:31:57 +0200 Subject: [PATCH 2/6] fix(ui): one submit control per form, and reuse mergeRefs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups: - The embed panel ended up with two submit controls: its own Embed button plus the hidden one `Form.Root` adds, so a screen reader announced two separate actions for the one thing that panel does. `Form.Root` now takes `hasOwnSubmitButton` for callers that supply their own. - The three `TextInput`s hand-rolled ref merging. `mergeRefs` already exists here, but returns a fresh callback per call — which detaches and reattaches the ref every render — so this adds `useMergeRefs` alongside it, memoized the way `react-merge-refs` does, and uses that. - The mantine popover keyed two behaviours off `portalRoot` while its comments explained them in terms of mobile. Same condition, but named, so the reason isn't hidden behind an unrelated prop. - `useFormSubmit` documents that it exists for `Form.Root` implementations rather than applications. --- packages/ariakit/src/input/Form.tsx | 10 ++++---- packages/ariakit/src/input/TextInput.tsx | 16 +++---------- packages/mantine/src/form/Form.tsx | 10 ++++---- packages/mantine/src/form/TextInput.tsx | 16 +++---------- packages/mantine/src/popover/Popover.tsx | 10 ++++++-- .../FilePanel/DefaultTabs/EmbedTab.tsx | 10 +++++++- .../react/src/editor/ComponentsContext.tsx | 7 ++++++ packages/react/src/hooks/useFormSubmit.ts | 5 ++++ packages/react/src/util/mergeRefs.ts | 24 +++++++++++++++++++ packages/shadcn/src/form/Form.tsx | 10 ++++---- packages/shadcn/src/form/TextInput.tsx | 16 +++---------- .../end-to-end/form/popoverSubmit.test.tsx | 14 +++++++++++ 12 files changed, 94 insertions(+), 54 deletions(-) diff --git a/packages/ariakit/src/input/Form.tsx b/packages/ariakit/src/input/Form.tsx index 14fe9b9916..f49e8bcc8f 100644 --- a/packages/ariakit/src/input/Form.tsx +++ b/packages/ariakit/src/input/Form.tsx @@ -4,7 +4,7 @@ import { assertEmpty } from "@blocknote/core"; import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, onSubmit, ...rest } = props; + const { children, onSubmit, hasOwnSubmitButton, ...rest } = props; const dict = useDictionary(); const formProps = useFormSubmit(onSubmit); @@ -20,9 +20,11 @@ export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { contract in `ComponentsContext`). Visually hidden rather than absent, so assistive technology still has a labelled control to activate. */} - + {!hasOwnSubmitButton && ( + + )} ); diff --git a/packages/ariakit/src/input/TextInput.tsx b/packages/ariakit/src/input/TextInput.tsx index 7dfec842ee..35b02b92d0 100644 --- a/packages/ariakit/src/input/TextInput.tsx +++ b/packages/ariakit/src/input/TextInput.tsx @@ -4,8 +4,8 @@ import { } from "@ariakit/react"; import { assertEmpty, mergeCSSClasses } from "@blocknote/core"; -import { ComponentProps } from "@blocknote/react"; -import { forwardRef, useCallback, useEffect, useRef } from "react"; +import { ComponentProps, useMergeRefs } from "@blocknote/react"; +import { forwardRef, useEffect, useRef } from "react"; export const TextInput = forwardRef< HTMLInputElement, @@ -37,17 +37,7 @@ export const TextInput = forwardRef< // pre-positioned spot and yanks the page (on mobile, right out from under // the block being edited). const inputRef = useRef(null); - const setRefs = useCallback( - (element: HTMLInputElement | null) => { - inputRef.current = element; - if (typeof ref === "function") { - ref(element); - } else if (ref) { - ref.current = element; - } - }, - [ref], - ); + const setRefs = useMergeRefs([inputRef, ref]); useEffect(() => { if (autoFocus) { inputRef.current?.focus({ preventScroll: true }); diff --git a/packages/mantine/src/form/Form.tsx b/packages/mantine/src/form/Form.tsx index 9d903bbced..ff9a9fc3d7 100644 --- a/packages/mantine/src/form/Form.tsx +++ b/packages/mantine/src/form/Form.tsx @@ -2,7 +2,7 @@ import { assertEmpty } from "@blocknote/core"; import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, onSubmit, ...rest } = props; + const { children, onSubmit, hasOwnSubmitButton, ...rest } = props; const dict = useDictionary(); const formProps = useFormSubmit(onSubmit); @@ -17,9 +17,11 @@ export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { contract in `ComponentsContext`). Visually hidden rather than absent, so assistive technology still has a labelled control to activate. */} - + {!hasOwnSubmitButton && ( + + )} ); }; diff --git a/packages/mantine/src/form/TextInput.tsx b/packages/mantine/src/form/TextInput.tsx index 60ea49d327..4d2e2bcb7f 100644 --- a/packages/mantine/src/form/TextInput.tsx +++ b/packages/mantine/src/form/TextInput.tsx @@ -1,8 +1,8 @@ import { TextInput as MantineTextInput } from "@mantine/core"; import { assertEmpty, mergeCSSClasses } from "@blocknote/core"; -import { ComponentProps } from "@blocknote/react"; -import { forwardRef, useCallback, useEffect, useRef } from "react"; +import { ComponentProps, useMergeRefs } from "@blocknote/react"; +import { forwardRef, useEffect, useRef } from "react"; export const TextInput = forwardRef< HTMLInputElement, @@ -34,17 +34,7 @@ export const TextInput = forwardRef< // pre-positioned spot and yanks the page (on mobile, right out from under // the block being edited). const inputRef = useRef(null); - const setRefs = useCallback( - (element: HTMLInputElement | null) => { - inputRef.current = element; - if (typeof ref === "function") { - ref(element); - } else if (ref) { - ref.current = element; - } - }, - [ref], - ); + const setRefs = useMergeRefs([inputRef, ref]); useEffect(() => { if (autoFocus) { inputRef.current?.focus({ preventScroll: true }); diff --git a/packages/mantine/src/popover/Popover.tsx b/packages/mantine/src/popover/Popover.tsx index c87da9aa6d..35a19590cf 100644 --- a/packages/mantine/src/popover/Popover.tsx +++ b/packages/mantine/src/popover/Popover.tsx @@ -13,6 +13,12 @@ export const Popover = ( ) => { const { open, onOpenChange, position, portalRoot, children, ...rest } = props; + // A `portalRoot` is only passed by the mobile toolbar, which renders its + // popovers into its own container — so it doubles as "this popover belongs + // to the mobile toolbar", which is what the two behaviours below actually + // depend on. Named here so the reason isn't hidden behind an unrelated prop. + const isMobileToolbarPopover = !!portalRoot; + assertEmpty(rest); return ( @@ -22,13 +28,13 @@ export const Popover = ( portalProps={portalRoot ? { target: portalRoot } : undefined} // Do not move focus to the dropdown on mobile, as it blurs the editor's // contentEditable and dismisses the on-screen keyboard. - trapFocus={portalRoot ? false : undefined} + trapFocus={isMobileToolbarPopover ? false : undefined} // Keep the dropdown visible through virtual-keyboard viewport resizes on // mobile: hideDetached (default true) reacts to the resize by setting // display:none on the dropdown, which blurs its focused input and // dismisses the on-screen keyboard (the input then unmounts with the // toolbar, so the whole UI collapses). - hideDetached={portalRoot ? false : undefined} + hideDetached={isMobileToolbarPopover ? false : undefined} opened={open} onChange={onOpenChange} position={position} diff --git a/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx b/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx index 0169c96f60..238701c7f3 100644 --- a/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx +++ b/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx @@ -55,7 +55,15 @@ export const EmbedTab = < return ( - + {/* + The embed button below is this form's submit control, so `Form.Root` + must not add its own — a screen reader would announce two separate + actions for the one thing this panel does. It stays outside the + `
` on purpose: the skins disagree on whether their panel button + defaults to `type="submit"`, so inside one it would fire `onClick` + *and* submit, embedding twice. + */} + void; + /** + * Set when the caller renders its own submit control inside the form. + * `Form.Root` otherwise adds a hidden one, which is what makes Enter + * submit at all once a form has more than one field - but two submit + * controls would read as two separate actions to a screen reader. + */ + hasOwnSubmitButton?: boolean; }; TextInput: { className?: string; diff --git a/packages/react/src/hooks/useFormSubmit.ts b/packages/react/src/hooks/useFormSubmit.ts index e2cf4dfbde..3d775d12bf 100644 --- a/packages/react/src/hooks/useFormSubmit.ts +++ b/packages/react/src/hooks/useFormSubmit.ts @@ -4,6 +4,11 @@ import { FormEvent, useCallback, useMemo, useRef } from "react"; * Props for the `` element a `Form.Root` implementation renders, wiring * up its `onSubmit` contract. * + * Exported because the UI-library packages implement `Form.Root` themselves + * and would otherwise each repeat the composition handling below. It is the + * contract between this package and a skin, not something an application is + * expected to reach for. + * * Submission has to be suppressed while an IME composition is in progress. * Accepting a candidate with Enter reaches the page as a `keydown` with * `isComposing: true`, and the browser performs implicit form submission for diff --git a/packages/react/src/util/mergeRefs.ts b/packages/react/src/util/mergeRefs.ts index 5137d0c030..7696ee2e8c 100644 --- a/packages/react/src/util/mergeRefs.ts +++ b/packages/react/src/util/mergeRefs.ts @@ -1,3 +1,5 @@ +import { useMemo } from "react"; + // https://github.com/gregberge/react-merge-refs/blob/main/src/index.tsx export function mergeRefs( refs: Array< @@ -14,3 +16,25 @@ export function mergeRefs( }); }; } + +/** + * {@link mergeRefs}, memoized on the refs themselves. + * + * `mergeRefs` returns a new callback on every call, and React detaches and + * reattaches a ref whose identity changed - calling it with `null` and then + * the element again on every render. Callers that keep their own ref + * alongside a forwarded one want the stable version, so this is the one to + * reach for from a component. + * + * Mirrors `react-merge-refs`' own `useMergeRefs`: the refs array is spread + * into the dependency list, which assumes a caller passes the same number of + * refs on every render - true of every use here, and of the upstream hook. + */ +export function useMergeRefs( + refs: Array< + React.MutableRefObject | React.LegacyRef | undefined | null + >, +): React.RefCallback { + // eslint-disable-next-line react-hooks/exhaustive-deps -- see above + return useMemo(() => mergeRefs(refs), refs); +} diff --git a/packages/shadcn/src/form/Form.tsx b/packages/shadcn/src/form/Form.tsx index 9d903bbced..ff9a9fc3d7 100644 --- a/packages/shadcn/src/form/Form.tsx +++ b/packages/shadcn/src/form/Form.tsx @@ -2,7 +2,7 @@ import { assertEmpty } from "@blocknote/core"; import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, onSubmit, ...rest } = props; + const { children, onSubmit, hasOwnSubmitButton, ...rest } = props; const dict = useDictionary(); const formProps = useFormSubmit(onSubmit); @@ -17,9 +17,11 @@ export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { contract in `ComponentsContext`). Visually hidden rather than absent, so assistive technology still has a labelled control to activate. */} - + {!hasOwnSubmitButton && ( + + )} ); }; diff --git a/packages/shadcn/src/form/TextInput.tsx b/packages/shadcn/src/form/TextInput.tsx index c441385922..4527984db3 100644 --- a/packages/shadcn/src/form/TextInput.tsx +++ b/packages/shadcn/src/form/TextInput.tsx @@ -1,6 +1,6 @@ import { assertEmpty } from "@blocknote/core"; -import { ComponentProps } from "@blocknote/react"; -import { forwardRef, useCallback, useEffect, useRef } from "react"; +import { ComponentProps, useMergeRefs } from "@blocknote/react"; +import { forwardRef, useEffect, useRef } from "react"; import { useShadCNComponentsContext } from "../ShadCNComponentsContext.js"; import { cn } from "../lib/utils.js"; @@ -35,17 +35,7 @@ export const TextInput = forwardRef< // pre-positioned spot and yanks the page (on mobile, right out from under // the block being edited). const inputRef = useRef(null); - const setRefs = useCallback( - (element: HTMLInputElement | null) => { - inputRef.current = element; - if (typeof ref === "function") { - ref(element); - } else if (ref) { - ref.current = element; - } - }, - [ref], - ); + const setRefs = useMergeRefs([inputRef, ref]); useEffect(() => { if (autoFocus) { inputRef.current?.focus({ preventScroll: true }); diff --git a/tests/src/end-to-end/form/popoverSubmit.test.tsx b/tests/src/end-to-end/form/popoverSubmit.test.tsx index 6f9e9b9446..fdcfd91381 100644 --- a/tests/src/end-to-end/form/popoverSubmit.test.tsx +++ b/tests/src/end-to-end/form/popoverSubmit.test.tsx @@ -106,6 +106,20 @@ describe("Submitting a toolbar popover with Enter", () => { expect((submit as HTMLButtonElement).tabIndex).toBe(-1); }); + test("the embed tab exposes exactly one submit control", async () => { + // Its own Embed button is the form's submit control, so `Form.Root` must + // not add a second hidden one — a screen reader would otherwise announce + // two separate actions for the one thing this panel does. + await focusOnEditor(); + await executeSlashCommand("image"); + await userEvent.click(await waitForSelector(`[data-test="embed-tab"]`)); + const input = await waitForSelector(`[data-test="embed-input"]`); + + const form = input.closest("form"); + expect(form, "the embed field must still be in a form").not.toBeNull(); + expect(form!.querySelectorAll("button").length).toBe(0); + }); + test("the embed tab's URL field commits", async () => { // The embed tab used to be the one input with an Enter handler and no // form at all, so its action key did nothing on mobile. From 039fb602b067c0a9b4d5f4b9a29aa86b5f685e84 Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 31 Aug 2026 16:56:55 +0200 Subject: [PATCH 3/6] test(ui): cover the composition guard, and drop two tests that couldn't fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review round, checking whether the tests added in the first one can actually fail. Two could not: - The composition tests built a synthetic form replicating what `Form.Root` does, so deleting the guard from `useFormSubmit` left them all green — the shipped code had no coverage at all. A test now drives the real link popover through a CDP composition, and fails when the guard is removed. The synthetic ones stay as what they are: the platform fact that a browser submits for an Enter carrying `isComposing: true`. - "the embed tab commits exactly once" asserted one image was present, which is true whether the update ran once or twice. Its replacement counted the form's submit events, but that cannot fail either: only mantine runs in this suite and its panel button already defaults to `type="button"`. The structural check — no button inside the form — is what actually guards both the double-commit and the duplicate-control problems, and it does fail when the button is moved inside, so that one is kept and the outcome-based tests are dropped rather than left as decoration. Also renames `hasOwnSubmitButton` to `omitSubmitButton`: EmbedTab's button sits outside the form, so the form has no submit button at all and relies on single-field implicit submission. The old name asserted something untrue of its only caller, and hid the constraint the flag carries. --- packages/ariakit/src/input/Form.tsx | 4 +- packages/mantine/src/form/Form.tsx | 4 +- .../FilePanel/DefaultTabs/EmbedTab.tsx | 2 +- .../react/src/editor/ComponentsContext.tsx | 14 ++-- packages/shadcn/src/form/Form.tsx | 4 +- .../end-to-end/form/popoverSubmit.test.tsx | 82 +++++++++++++------ 6 files changed, 71 insertions(+), 39 deletions(-) diff --git a/packages/ariakit/src/input/Form.tsx b/packages/ariakit/src/input/Form.tsx index f49e8bcc8f..cd7f46273b 100644 --- a/packages/ariakit/src/input/Form.tsx +++ b/packages/ariakit/src/input/Form.tsx @@ -4,7 +4,7 @@ import { assertEmpty } from "@blocknote/core"; import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, onSubmit, hasOwnSubmitButton, ...rest } = props; + const { children, onSubmit, omitSubmitButton, ...rest } = props; const dict = useDictionary(); const formProps = useFormSubmit(onSubmit); @@ -20,7 +20,7 @@ export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { contract in `ComponentsContext`). Visually hidden rather than absent, so assistive technology still has a labelled control to activate. */} - {!hasOwnSubmitButton && ( + {!omitSubmitButton && ( diff --git a/packages/mantine/src/form/Form.tsx b/packages/mantine/src/form/Form.tsx index ff9a9fc3d7..0ce9c1b33d 100644 --- a/packages/mantine/src/form/Form.tsx +++ b/packages/mantine/src/form/Form.tsx @@ -2,7 +2,7 @@ import { assertEmpty } from "@blocknote/core"; import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, onSubmit, hasOwnSubmitButton, ...rest } = props; + const { children, onSubmit, omitSubmitButton, ...rest } = props; const dict = useDictionary(); const formProps = useFormSubmit(onSubmit); @@ -17,7 +17,7 @@ export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { contract in `ComponentsContext`). Visually hidden rather than absent, so assistive technology still has a labelled control to activate. */} - {!hasOwnSubmitButton && ( + {!omitSubmitButton && ( diff --git a/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx b/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx index 238701c7f3..0462bc89a4 100644 --- a/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx +++ b/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx @@ -63,7 +63,7 @@ export const EmbedTab = < defaults to `type="submit"`, so inside one it would fire `onClick` *and* submit, embedding twice. */} - + void; /** - * Set when the caller renders its own submit control inside the form. - * `Form.Root` otherwise adds a hidden one, which is what makes Enter - * submit at all once a form has more than one field - but two submit - * controls would read as two separate actions to a screen reader. + * Suppresses the hidden submit button `Form.Root` otherwise renders, + * for callers that provide their own submission affordance and would + * otherwise expose two submit controls to assistive technology. + * + * Note what the hidden button is for: it is what makes Enter submit a + * form with more than one field at all. A caller that omits it takes + * on that constraint - the form must have exactly one field, or Enter + * reaches nothing. */ - hasOwnSubmitButton?: boolean; + omitSubmitButton?: boolean; }; TextInput: { className?: string; diff --git a/packages/shadcn/src/form/Form.tsx b/packages/shadcn/src/form/Form.tsx index ff9a9fc3d7..0ce9c1b33d 100644 --- a/packages/shadcn/src/form/Form.tsx +++ b/packages/shadcn/src/form/Form.tsx @@ -2,7 +2,7 @@ import { assertEmpty } from "@blocknote/core"; import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, onSubmit, hasOwnSubmitButton, ...rest } = props; + const { children, onSubmit, omitSubmitButton, ...rest } = props; const dict = useDictionary(); const formProps = useFormSubmit(onSubmit); @@ -17,7 +17,7 @@ export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { contract in `ComponentsContext`). Visually hidden rather than absent, so assistive technology still has a labelled control to activate. */} - {!hasOwnSubmitButton && ( + {!omitSubmitButton && ( diff --git a/tests/src/end-to-end/form/popoverSubmit.test.tsx b/tests/src/end-to-end/form/popoverSubmit.test.tsx index fdcfd91381..a327ac7a61 100644 --- a/tests/src/end-to-end/form/popoverSubmit.test.tsx +++ b/tests/src/end-to-end/form/popoverSubmit.test.tsx @@ -2,11 +2,16 @@ import TestingApp from "@examples/01-basic/testing/src/App"; import { beforeEach, describe, expect, test, vi } from "vite-plus/test"; import { render } from "vitest-browser-react"; -import { userEvent } from "../../utils/context.js"; +import { browserName, commands, userEvent } from "../../utils/context.js"; +import type { ImeCompositionCommand } from "../../utils/imeComposition.js"; import { EDITOR_SELECTOR, LINK_BUTTON_SELECTOR } from "../../utils/const.js"; import { focusOnEditor, waitForSelector } from "../../utils/editor.js"; import { executeSlashCommand } from "../../utils/slashmenu.js"; +const browserCommands = commands as typeof commands & { + imeComposition: ImeCompositionCommand; +}; + /** * The toolbar popovers commit through their form's `submit` event, because a * mobile IME's action key fires that and no key event at all. @@ -106,10 +111,18 @@ describe("Submitting a toolbar popover with Enter", () => { expect((submit as HTMLButtonElement).tabIndex).toBe(-1); }); - test("the embed tab exposes exactly one submit control", async () => { - // Its own Embed button is the form's submit control, so `Form.Root` must - // not add a second hidden one — a screen reader would otherwise announce - // two separate actions for the one thing this panel does. + test("the embed tab keeps its button out of the form", async () => { + // Two things ride on the button staying outside the `
`, which is why + // this asserts the structure rather than an outcome: + // + // - `Form.Root` must not also add its hidden submit button, or a screen + // reader announces two separate actions for the one thing this panel + // does. + // - Inside the form the button would fire `onClick` *and* submit on the + // skins whose panel button defaults to `type="submit"` (ariakit and + // shadcn; mantine's defaults to `type="button"`), embedding twice. + // Only mantine runs in this suite, so a double-commit assertion here + // could never fail — the structural check is what actually guards it. await focusOnEditor(); await executeSlashCommand("image"); await userEvent.click(await waitForSelector(`[data-test="embed-tab"]`)); @@ -138,26 +151,41 @@ describe("Submitting a toolbar popover with Enter", () => { await waitForSelector(`img[src="${url}"]`); }); - test("the embed tab commits exactly once", async () => { - // The embed button sits outside the form on purpose: the skins disagree on - // whether their panel button defaults to `type="submit"`, so inside one it - // would fire `onClick` *and* submit, applying the same edit twice. - await focusOnEditor(); - await executeSlashCommand("image"); - - await userEvent.click(await waitForSelector(`[data-test="embed-tab"]`)); - const input = (await waitForSelector( - `[data-test="embed-input"]`, - )) as HTMLInputElement; - await userEvent.click(input); - - const url = "https://placehold.co/400x300.png"; - await userEvent.keyboard(url); - await userEvent.click( - await waitForSelector(`[data-test="embed-input-button"]`), - ); - - await waitForSelector(`img[src="${url}"]`); - expect(document.querySelectorAll(`img[src="${url}"]`).length).toBe(1); - }); + // `Input.imeSetComposition` is CDP-only, so the real composition state can + // only be entered in chromium. + test.skipIf(browserName !== "chromium")( + "Enter mid-composition does not commit the popover", + async () => { + // The platform performs implicit submission for an Enter delivered with + // `isComposing: true` (see ./compositionSubmit.test.tsx), so accepting + // an IME candidate would otherwise commit the link mid-word. This drives + // the real popover rather than a stand-in, so it covers the guard + // `Form.Root` actually ships. + await focusOnEditor(); + await userEvent.keyboard("link me"); + await userEvent.keyboard("{Home}{Shift>}{End}{/Shift}"); + await userEvent.click(await waitForSelector(LINK_BUTTON_SELECTOR)); + const input = (await waitForSelector( + 'input[name="url"]', + )) as HTMLInputElement; + await userEvent.click(input); + + await browserCommands.imeComposition([ + { type: "setComposition", text: "にほん" }, + ]); + await userEvent.keyboard("{Enter}"); + + expect( + document.querySelector(`${EDITOR_SELECTOR} a`), + "accepting an IME candidate must not commit the link", + ).toBeNull(); + + // And once composition is over, Enter still works. + await browserCommands.imeComposition([ + { type: "commit", text: "example.com" }, + ]); + await userEvent.keyboard("{Enter}"); + await waitForSelector(`${EDITOR_SELECTOR} a`); + }, + ); }); From a59179245a27e396c70fa845b16b6ea2ebd65440 Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 31 Aug 2026 17:52:14 +0200 Subject: [PATCH 4/6] fix(core): scan the selection for the link URL instead of probing a boundary The `from + 1` probe fixed the left-edge case (`marks()` excludes a link at its left boundary) but is still fragile: browsers disagree by a position on where a selection over a link starts, so a single-position lookup can land outside the mark either way. For a non-empty selection, scan the selected range for the first link mark instead; an empty selection keeps the plain position lookup. --- .../core/src/editor/managers/StyleManager.ts | 27 ++++++++++++++----- .../end-to-end/mobile/mobileToolbar.test.tsx | 6 +++-- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/core/src/editor/managers/StyleManager.ts b/packages/core/src/editor/managers/StyleManager.ts index a3ddf0d52b..6e802a4c17 100644 --- a/packages/core/src/editor/managers/StyleManager.ts +++ b/packages/core/src/editor/managers/StyleManager.ts @@ -183,13 +183,26 @@ export class StyleManager< */ public getSelectedLinkUrl() { return this.editor.transact((tr) => { - // `from + 1` for the same boundary reason as `editLink` below: at the - // left edge of a link (e.g. when the whole link is selected), the mark - // lookup at `from` itself misses the mark and the link's URL would - // incorrectly read as absent. - return this.getLinkMarkAtPos( - Math.min(tr.selection.from + 1, tr.doc.content.size), - )?.href; + const { from, to, empty } = tr.selection; + if (empty) { + return this.getLinkMarkAtPos(from)?.href; + } + // For a non-empty selection, probing a single boundary position is + // fragile twice over: `marks()` excludes a link at its left edge, and + // browsers disagree by a position on where a selection over a link + // starts. Scan the selected range for the first link mark instead. + let href: string | undefined; + tr.doc.nodesBetween(from, to, (node) => { + if (href !== undefined) { + return false; + } + const linkMark = node.marks.find((mark) => mark.type.name === "link"); + if (linkMark) { + href = linkMark.attrs.href; + } + return href === undefined; + }); + return href; }); } diff --git a/tests/src/end-to-end/mobile/mobileToolbar.test.tsx b/tests/src/end-to-end/mobile/mobileToolbar.test.tsx index 57ce398af5..11c86e5027 100644 --- a/tests/src/end-to-end/mobile/mobileToolbar.test.tsx +++ b/tests/src/end-to-end/mobile/mobileToolbar.test.tsx @@ -141,8 +141,10 @@ describe("Mobile formatting toolbar", () => { }); // Reopening the popover with the whole link selected must pre-fill its - // URL: `getSelectedLinkUrl` reads the mark just inside the selection - // start, since a lookup exactly at the link's left boundary misses it. + // URL: `getSelectedLinkUrl` scans the selected range for the link mark, + // since a probe at a single boundary position misses it — `marks()` + // excludes a link at its left edge, and browsers disagree by a position + // on where a selection over a link starts. await userEvent.keyboard("{Shift>}{Home}{/Shift}"); await userEvent.click( await waitForSelector( From e736b32137db72d3826baff23e58b8efed730115 Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 31 Aug 2026 19:28:56 +0200 Subject: [PATCH 5/6] =?UTF-8?q?fix(ui):=20drop=20the=20composition=20guard?= =?UTF-8?q?=20=E2=80=94=20native=20submission=20already=20handles=20IMEs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard answered the wrong category of problem. `isComposing` checks are needed in *keydown* handlers, because an IME-consumed key still dispatches to JS — that is what the five removed Enter handlers were. Native form submission never sees that key: the IME consumes the confirming Enter (it reaches the page as keyCode 229, which the browser runs no default action for), so implicit submission cannot fire mid-composition. This is why no plain form on the web carries composition handling. The state the guard defended — composition open, unconsumed trusted Enter delivered — is one only CDP emulation can fabricate: `imeSetComposition` sets composition state with no IME in the loop to consume the key. No real IME produces the sequence. Worse, the guard carried real risk in the other direction: Gboard's action key commits the composition and submits in one press, so if any IME delivers `submit` before `compositionend`, the guard would swallow a legitimate submission — the original bug, reintroduced for exactly the users it claimed to protect. `Form.Root` goes back to plain `preventDefault` wiring, `useFormSubmit` is deleted, and the composition tests now pin the *native* contract against the real popover: accepting a candidate does not submit, Enter afterwards does. --- packages/ariakit/src/input/Form.tsx | 11 ++- packages/mantine/src/form/Form.tsx | 11 ++- packages/react/src/hooks/useFormSubmit.ts | 54 --------------- packages/react/src/index.ts | 1 - packages/shadcn/src/form/Form.tsx | 11 ++- .../form/compositionSubmit.test.tsx | 68 ++++++------------- .../end-to-end/form/popoverSubmit.test.tsx | 24 +++---- 7 files changed, 55 insertions(+), 125 deletions(-) delete mode 100644 packages/react/src/hooks/useFormSubmit.ts diff --git a/packages/ariakit/src/input/Form.tsx b/packages/ariakit/src/input/Form.tsx index cd7f46273b..819bf4f3c7 100644 --- a/packages/ariakit/src/input/Form.tsx +++ b/packages/ariakit/src/input/Form.tsx @@ -1,18 +1,23 @@ import { FormProvider as AriakitFormProvider } from "@ariakit/react"; import { assertEmpty } from "@blocknote/core"; -import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; +import { ComponentProps, useDictionary } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { const { children, onSubmit, omitSubmitButton, ...rest } = props; const dict = useDictionary(); - const formProps = useFormSubmit(onSubmit); assertEmpty(rest); return ( - + { + // These forms have no action — a real submission would navigate. + event.preventDefault(); + onSubmit?.(); + }} + > {children} {/* Gives the form a submit button, which is what makes Enter submit it at diff --git a/packages/mantine/src/form/Form.tsx b/packages/mantine/src/form/Form.tsx index 0ce9c1b33d..f0cc1e7d0e 100644 --- a/packages/mantine/src/form/Form.tsx +++ b/packages/mantine/src/form/Form.tsx @@ -1,15 +1,20 @@ import { assertEmpty } from "@blocknote/core"; -import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; +import { ComponentProps, useDictionary } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { const { children, onSubmit, omitSubmitButton, ...rest } = props; const dict = useDictionary(); - const formProps = useFormSubmit(onSubmit); assertEmpty(rest); return ( - + { + // These forms have no action — a real submission would navigate. + event.preventDefault(); + onSubmit?.(); + }} + > {children} {/* Gives the form a submit button, which is what makes Enter submit it at diff --git a/packages/react/src/hooks/useFormSubmit.ts b/packages/react/src/hooks/useFormSubmit.ts deleted file mode 100644 index 3d775d12bf..0000000000 --- a/packages/react/src/hooks/useFormSubmit.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { FormEvent, useCallback, useMemo, useRef } from "react"; - -/** - * Props for the `` element a `Form.Root` implementation renders, wiring - * up its `onSubmit` contract. - * - * Exported because the UI-library packages implement `Form.Root` themselves - * and would otherwise each repeat the composition handling below. It is the - * contract between this package and a skin, not something an application is - * expected to reach for. - * - * Submission has to be suppressed while an IME composition is in progress. - * Accepting a candidate with Enter reaches the page as a `keydown` with - * `isComposing: true`, and the browser performs implicit form submission for - * it anyway — so a CJK user confirming a candidate would submit the popover - * instead of finishing their word. (Verified in Chromium; see - * tests/src/end-to-end/form/compositionSubmit.test.tsx.) - * - * Composition events bubble, so listening on the form covers every field in - * it. This is deliberately the single place that knowledge lives: the same - * guard used to be repeated in each popover's own Enter handler, which is - * exactly how the callsites drifted out of sync. - */ -export function useFormSubmit(onSubmit?: () => void) { - const composing = useRef(false); - - const handleSubmit = useCallback( - (event: FormEvent) => { - // Always prevent the default: these forms have no action and a real - // navigation would tear down the editor. - event.preventDefault(); - - if (composing.current) { - return; - } - - onSubmit?.(); - }, - [onSubmit], - ); - - return useMemo( - () => ({ - onCompositionStart: () => { - composing.current = true; - }, - onCompositionEnd: () => { - composing.current = false; - }, - onSubmit: handleSubmit, - }), - [handleSubmit], - ); -} diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index a72c2ca67a..e5ba94c223 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -136,7 +136,6 @@ export * from "./hooks/useCreateBlockNote.js"; export * from "./hooks/useEditorChange.js"; export * from "./hooks/useEditorFocus.js"; export * from "./hooks/useEditorFocusChange.js"; -export * from "./hooks/useFormSubmit.js"; export * from "./hooks/useEditorDomElement.js"; export * from "./hooks/useEditorSelectionBoundingBox.js"; export * from "./hooks/useEditorSelectionChange.js"; diff --git a/packages/shadcn/src/form/Form.tsx b/packages/shadcn/src/form/Form.tsx index 0ce9c1b33d..f0cc1e7d0e 100644 --- a/packages/shadcn/src/form/Form.tsx +++ b/packages/shadcn/src/form/Form.tsx @@ -1,15 +1,20 @@ import { assertEmpty } from "@blocknote/core"; -import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react"; +import { ComponentProps, useDictionary } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { const { children, onSubmit, omitSubmitButton, ...rest } = props; const dict = useDictionary(); - const formProps = useFormSubmit(onSubmit); assertEmpty(rest); return ( - + { + // These forms have no action — a real submission would navigate. + event.preventDefault(); + onSubmit?.(); + }} + > {children} {/* Gives the form a submit button, which is what makes Enter submit it at diff --git a/tests/src/end-to-end/form/compositionSubmit.test.tsx b/tests/src/end-to-end/form/compositionSubmit.test.tsx index 79c4d7a49b..6867a93792 100644 --- a/tests/src/end-to-end/form/compositionSubmit.test.tsx +++ b/tests/src/end-to-end/form/compositionSubmit.test.tsx @@ -3,15 +3,23 @@ import { browserName, commands, userEvent } from "../../utils/context.js"; import type { ImeCompositionCommand } from "../../utils/imeComposition.js"; /** - * Every popover Enter handler used to guard on `isComposing`, so that Enter - * pressed to accept an IME candidate committed the candidate instead of the - * form. Those handlers are gone — submission now runs off the form's `submit` - * event — which moves the question to the platform: can a composition-ending - * Enter reach a form as an implicit submission? + * Why the popover forms need no composition guard. * - * If it can, dropping the guards regressed CJK input everywhere, and the - * guards have to come back at the form level. So it is asserted rather than - * assumed. + * The Enter handlers that `Form.Root`'s submit path replaced all guarded on + * `isComposing` — necessary for a *keydown* handler, because the keydown for + * an IME-consumed key still dispatches to JS. Native form submission is a + * different category: the IME consumes the confirming Enter (it reaches the + * page as keyCode 229, which the browser runs no default action for), so + * implicit submission never fires mid-composition. This is why no plain + * `` in the world carries composition handling. + * + * These tests pin the two halves of that contract on the real IME event + * sequence. What they deliberately do *not* do is inject a bare Enter while + * composition is held open: CDP can fabricate that state, and the browser + * does submit on it, but no real IME delivers an unconsumed Enter + * mid-composition — and guarding against the fabricated state would mean + * betting that every IME fires `compositionend` before the submit it + * triggers, or a Gboard-style single-press commit-and-submit gets swallowed. */ const browserCommands = commands as typeof commands & { @@ -34,15 +42,8 @@ function buildForm() { form = document.createElement("form"); const submits: string[] = []; const compositions: string[] = []; - // Mirrors what `useFormSubmit` wires onto a real `Form.Root`. - let composing = false; - form.addEventListener("compositionstart", () => (composing = true)); - form.addEventListener("compositionend", () => (composing = false)); form.addEventListener("submit", (event) => { event.preventDefault(); - if (composing) { - return; - } submits.push("submit"); }); @@ -67,13 +68,14 @@ function buildForm() { return { input, submits, compositions }; } -describeIme("Enter during an IME composition", () => { +describeIme("IME composition and form submission", () => { test("accepting a candidate does not submit the form", async () => { + // The real accept path: the IME replaces the composition with the final + // text (`insertText`), and the confirming key never reaches the page as + // an actionable Enter — so nothing submits, natively. const { input, submits, compositions } = buildForm(); input.focus(); - // Accepting a candidate the way an IME does: the final text replaces the - // composing text, and the confirming key never reaches the page. await browserCommands.imeComposition([ { type: "setComposition", text: "にほん" }, { type: "commit", text: "日本" }, @@ -87,36 +89,6 @@ describeIme("Enter during an IME composition", () => { ).toEqual([]); }); - test("Enter arriving mid-composition does not submit the form", async () => { - // The case that makes the guard necessary rather than defensive: the - // browser delivers this Enter as `keydown` with `isComposing: true` and - // performs implicit submission for it regardless, so without the guard a - // CJK user accepting a candidate submits the popover mid-word. - const { input, submits, compositions } = buildForm(); - const composingOnKeyDown: boolean[] = []; - input.addEventListener("keydown", (event) => - composingOnKeyDown.push(event.isComposing), - ); - input.focus(); - - await browserCommands.imeComposition([ - { type: "setComposition", text: "にほん" }, - ]); - await userEvent.keyboard("{Enter}"); - - // Pin the precondition too: if a future engine stopped delivering this - // Enter to the page, the guard would be untested rather than unnecessary. - expect( - composingOnKeyDown, - "Enter must reach the page mid-composition", - ).toEqual([true]); - expect(compositions).not.toContain("compositionend"); - expect( - submits, - "Enter must not submit while a composition is in progress", - ).toEqual([]); - }); - test("Enter after the composition ends does submit", async () => { // The other half of the contract: once composition is over, Enter has to // work normally, or CJK users could never submit at all. diff --git a/tests/src/end-to-end/form/popoverSubmit.test.tsx b/tests/src/end-to-end/form/popoverSubmit.test.tsx index a327ac7a61..f12492ccad 100644 --- a/tests/src/end-to-end/form/popoverSubmit.test.tsx +++ b/tests/src/end-to-end/form/popoverSubmit.test.tsx @@ -154,13 +154,13 @@ describe("Submitting a toolbar popover with Enter", () => { // `Input.imeSetComposition` is CDP-only, so the real composition state can // only be entered in chromium. test.skipIf(browserName !== "chromium")( - "Enter mid-composition does not commit the popover", + "accepting an IME candidate does not commit the popover", async () => { - // The platform performs implicit submission for an Enter delivered with - // `isComposing: true` (see ./compositionSubmit.test.tsx), so accepting - // an IME candidate would otherwise commit the link mid-word. This drives - // the real popover rather than a stand-in, so it covers the guard - // `Form.Root` actually ships. + // The real accept path: the IME consumes the confirming key and + // replaces the composition with the final text, so no actionable Enter + // reaches the page and nothing submits — natively, with no composition + // guard in `Form.Root` (see ./compositionSubmit.test.tsx for why none + // is needed). await focusOnEditor(); await userEvent.keyboard("link me"); await userEvent.keyboard("{Home}{Shift>}{End}{/Shift}"); @@ -171,19 +171,17 @@ describe("Submitting a toolbar popover with Enter", () => { await userEvent.click(input); await browserCommands.imeComposition([ - { type: "setComposition", text: "にほん" }, + { type: "setComposition", text: "example.co" }, + { type: "commit", text: "example.com" }, ]); - await userEvent.keyboard("{Enter}"); + expect(input.value).toBe("example.com"); expect( document.querySelector(`${EDITOR_SELECTOR} a`), - "accepting an IME candidate must not commit the link", + "accepting a candidate must not commit the link", ).toBeNull(); - // And once composition is over, Enter still works. - await browserCommands.imeComposition([ - { type: "commit", text: "example.com" }, - ]); + // Enter after the composition commits it as usual. await userEvent.keyboard("{Enter}"); await waitForSelector(`${EDITOR_SELECTOR} a`); }, From 103cb263eadeedb944e8cbfed34a1a69f5d64ded Mon Sep 17 00:00:00 2001 From: yousefed Date: Tue, 1 Sep 2026 07:58:16 +0200 Subject: [PATCH 6/6] test(device): cover the link-popover flow through real on-screen input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The toolbar/link suite on both device targets: the keyboard opening and resizing the viewport, toolbar taps registering, the popover holding focus through the IME reconfigure, and submission by pressing the on-screen keyboard's actual Enter/action key. The IME-action-key test is the former manual release-checklist item: Android's IME only offers a submitting action inside a real — with a lone field it picks Next, the original create-link bug. Pairs with the emulated linkSubmit.test.tsx. --- tests/device/formattingToolbar.device.test.ts | 185 ++++++++++++++++++ tests/device/linkPopover.ts | 90 +++++++++ 2 files changed, 275 insertions(+) create mode 100644 tests/device/formattingToolbar.device.test.ts create mode 100644 tests/device/linkPopover.ts diff --git a/tests/device/formattingToolbar.device.test.ts b/tests/device/formattingToolbar.device.test.ts new file mode 100644 index 0000000000..361a614b6a --- /dev/null +++ b/tests/device/formattingToolbar.device.test.ts @@ -0,0 +1,185 @@ +import { afterAll, beforeAll, describe, expect, test } from "vite-plus/test"; + +import { activeDevices } from "./devices.js"; +import { tapElement } from "./lib/gestures.js"; +import { + docState, + MOBILE_TOOLBAR, + openExample, + startEditing, + viewportHeight, +} from "./lib/editorPage.js"; +import { + LINK_POPOVER, + openLinkPopover, + selectFirstWord, + typeAndSubmit, +} from "./linkPopover.js"; +import type { DeviceSession } from "./lib/session.js"; + +const KEYBOARD_MIN_HEIGHT = 150; + +function sleep(ms: number) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +for (const device of await activeDevices()) { + describe(`mobile formatting toolbar on ${device.id}`, () => { + let session: DeviceSession; + let baselineHeight: number; + + beforeAll(async () => { + session = await device.createSession(); + await openExample(session, "/ui-components/mobile-formatting-toolbar"); + baselineHeight = await viewportHeight(session); + }); + + afterAll(async () => { + if (session) { + await session.screenshot(`formatting-toolbar-final`); + await session.close(); + } + }); + + test("tapping the editor opens the keyboard and shows the mobile toolbar", async () => { + await startEditing(session); + + // The toolbar only renders while `useVirtualKeyboard` sees the + // keyboard, so its presence + the viewport drop prove the real + // on-screen keyboard opened. + expect(await viewportHeight(session)).toBeLessThan( + baselineHeight - KEYBOARD_MIN_HEIGHT, + ); + }); + + test("toolbar buttons apply reliably", async () => { + await startEditing(session); + await selectFirstWord(session); + // Three bold toggles; every tap must register (covers the reported + // "buttons sometimes don't work", which traced back to a lingering + // popover overlaying the toolbar). + for (const expected of [true, false, true]) { + await tapElement(session, `${MOBILE_TOOLBAR} [data-test="bold"]`, { + keyboard: "open", + verify: `return { ok: ${expected} === !!document.querySelector('.bn-editor strong') };`, + }); + } + }); + + test("link popover holds focus through the IME and creates a link", async () => { + // Captured before the popover opens: iOS Safari auto-zooms the page + // when an input with a computed font-size under 16px takes focus, and + // that zoom perturbs the visual viewport the mobile toolbar positions + // itself from. The `pointer: coarse` rule in blocknoteStyles.css + // prevents it; this pins the behaviour rather than the rule. + const scaleBefore = await session.exec( + `return window.visualViewport ? window.visualViewport.scale : 1;`, + ); + + await openLinkPopover(session); + + // Focusing an input makes the IME reconfigure (on Android this + // resizes the viewport), which historically hid the popover and + // collapsed the keyboard/toolbar (the Mantine `hideDetached` bug). + // The input must still hold focus once that settles. + await sleep(2_500); + const survival = await session.exec<{ + focused: boolean; + popover: boolean; + toolbar: boolean; + }>(` + const active = document.activeElement; + return { + focused: !!(active && active.tagName === 'INPUT' && active.getAttribute('name') === 'url'), + popover: !!document.querySelector(${JSON.stringify(LINK_POPOVER)}), + toolbar: !!document.querySelector(${JSON.stringify(MOBILE_TOOLBAR)}), + };`); + await session.screenshot("link-popover-open"); + + // Focusing the URL input must not have zoomed the page. + const scaleAfter = await session.exec( + `return window.visualViewport ? window.visualViewport.scale : 1;`, + ); + expect( + scaleAfter, + `focusing the URL input zoomed the page (${scaleBefore} -> ${scaleAfter}); ` + + `check the pointer:coarse font-size rule for .bn-form-popover inputs`, + ).toBeLessThanOrEqual(scaleBefore + 0.01); + + expect(survival).toEqual({ + focused: true, + popover: true, + toolbar: true, + }); + + await typeAndSubmit( + session, + `${LINK_POPOVER} input`, + "example.com", + `return { + ok: !!document.querySelector('.bn-editor a[href="https://example.com"]') + && !document.querySelector(${JSON.stringify(LINK_POPOVER)}), + link: !!document.querySelector('.bn-editor a[href="https://example.com"]'), + popoverGone: !document.querySelector(${JSON.stringify(LINK_POPOVER)}), + };`, + ); + + expect((await docState(session)).links).toContain("https://example.com"); + // Submitting must not dismiss the keyboard — but Appium's typing can + // itself hide the keyboard as an automation side effect (observed on + // Android), which the product can't distinguish from the user closing + // it. So only assert the toolbar survived while the keyboard is + // actually still up; the emulation suite covers this invariant + // deterministically. + if ( + (await viewportHeight(session)) < + baselineHeight - KEYBOARD_MIN_HEIGHT + ) { + expect( + await session.exec( + `return !!document.querySelector(${JSON.stringify(MOBILE_TOOLBAR)});`, + ), + ).toBe(true); + } + }); + + // The flow that used to be a manual release-checklist item: Android's + // IME decides what its action key does — with a lone text field outside + // a it picks "Next" (advance focus, no key event at all), the + // original create-link bug. Only a backend that can press the on-screen + // keyboard can test the IME's actual choice. + test.skipIf(device.kind !== "local-android")( + "the IME action key submits the link popover", + async () => { + // Fresh document — the earlier tests linked the first word, and a + // linked selection opens the *edit* popover (pre-filled URL) instead + // of the create popover this flow is about. + await openExample(session, "/ui-components/mobile-formatting-toolbar"); + await startEditing(session); + await openLinkPopover(session); + + await session.elementValue(`${LINK_POPOVER} input`, "example.com"); + + if (!session.pressImeActionKey) { + throw new Error("this target must expose the IME action key"); + } + await session.pressImeActionKey( + `return { + ok: !!document.querySelector('.bn-editor a[href="https://example.com"]') + && !document.querySelector(${JSON.stringify(LINK_POPOVER)}), + link: !!document.querySelector('.bn-editor a[href="https://example.com"]'), + popoverGone: !document.querySelector(${JSON.stringify(LINK_POPOVER)}), + };`, + ); + + // The action must not have advanced focus out of the editor — that + // was the original bug's symptom (focus jumping to the next editor). + const state = await session.exec<{ inFirstEditor: boolean }>( + `const editors = [...document.querySelectorAll(".bn-editor")]; + return { inFirstEditor: editors[0].contains(document.activeElement) };`, + ); + expect(state.inFirstEditor).toBe(true); + }, + ); + }); +} diff --git a/tests/device/linkPopover.ts b/tests/device/linkPopover.ts new file mode 100644 index 0000000000..8d621ff204 --- /dev/null +++ b/tests/device/linkPopover.ts @@ -0,0 +1,90 @@ +/** + * Helpers for the create-link flow on real devices — next to the tests that + * use them, since only the link tests speak these concepts. + */ +import { MOBILE_TOOLBAR, PARAGRAPH, startEditing } from "./lib/editorPage.js"; +import { pressSoftKeyboardEnter, tapElement } from "./lib/gestures.js"; +import type { DeviceSession } from "./lib/session.js"; + +export const LINK_BUTTON = `${MOBILE_TOOLBAR} [data-test="createLink"]`; +export const LINK_POPOVER = ".bn-form-popover"; + +/** + * Selects the first word of the first paragraph via a DOM range (ProseMirror + * syncs its selection from `selectionchange`, so no editor handle is needed). + * iOS intermittently collapses programmatic selections, so the wait re-applies + * the range on every poll until the toolbar's link button confirms the editor + * sees a non-empty selection. + */ +export async function selectFirstWord(session: DeviceSession): Promise { + const applyAndCheck = ` + if (getSelection().isCollapsed) { + const p = document.querySelector(${JSON.stringify(PARAGRAPH)}); + const textNode = [...p.childNodes].find((n) => n.nodeType === 3) || p.firstChild; + const range = document.createRange(); + range.setStart(textNode, 0); + range.setEnd(textNode, Math.min(7, textNode.textContent.length)); + const selection = getSelection(); + selection.removeAllRanges(); + selection.addRange(range); + } + return { + ok: !getSelection().isCollapsed + && !!document.querySelector(${JSON.stringify(LINK_BUTTON)}), + };`; + await session.waitFor("selection + link button", applyAndCheck, 25_000); +} + +/** + * Opens the create-link popover from the mobile toolbar and waits for its URL + * input to hold focus. A mis-aimed tap (iOS chrome-offset guessing) can hit + * the keyboard's accessory bar and collapse the whole editing state, so each + * attempt rebuilds editing + selection from scratch before tapping. + */ +export async function openLinkPopover(session: DeviceSession): Promise { + let lastError: Error | undefined; + for (let attempt = 0; attempt < 4; attempt++) { + await startEditing(session); + await selectFirstWord(session); + await session.exec(` + const toolbar = document.querySelector(${JSON.stringify(MOBILE_TOOLBAR)}); + toolbar.querySelectorAll('*').forEach((el) => { + if (el.scrollWidth > el.clientWidth + 5) el.scrollLeft = el.scrollWidth; + });`); + try { + await tapElement(session, LINK_BUTTON, { + keyboard: "open", + verify: ` + const active = document.activeElement; + return { + ok: !!document.querySelector(${JSON.stringify(LINK_POPOVER)}) + && active && active.tagName === 'INPUT' + && active.getAttribute('name') === 'url', + };`, + }); + return; + } catch (error) { + lastError = error as Error; + } + } + throw new Error(`Could not open the link popover: ${lastError?.message}`); +} + +/** + * Types into a popover field and submits it by pressing the on-screen + * keyboard's Enter/action key — the real user gesture on both platforms (see + * `pressSoftKeyboardEnter`), driving the real submission path: key press -> + * implicit form submission -> the popover's `submit` handling. + * + * `verify` is a page script returning `{ ok: boolean }` observing the + * submission's effect — the tap ladders need it to know a tap landed. + */ +export async function typeAndSubmit( + session: DeviceSession, + css: string, + text: string, + verify: string, +): Promise { + await session.elementValue(css, text); + await pressSoftKeyboardEnter(session, verify); +}