From c8582b92587cf94068a97ca3c9bac03a99ec69bf Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 31 Aug 2026 16:02:27 +0200 Subject: [PATCH 1/9] 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..1c19b810dd 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..45ff9341d8 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/en.ts b/packages/core/src/i18n/locales/en.ts index e5386f3020..307ba90c22 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/es.ts b/packages/core/src/i18n/locales/es.ts index 743a1be05c..b2c05ca6b2 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: "Aceptar", }, }; diff --git a/packages/core/src/i18n/locales/fa.ts b/packages/core/src/i18n/locales/fa.ts index 6b2783ab68..405cf87ddf 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..4807927655 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/he.ts b/packages/core/src/i18n/locales/he.ts index 4662a94202..1b9338b77b 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..998a245f20 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: "U redu", }, }; diff --git a/packages/core/src/i18n/locales/is.ts b/packages/core/src/i18n/locales/is.ts index 913b2324b0..e7effe3827 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: "Í lagi", }, }; diff --git a/packages/core/src/i18n/locales/it.ts b/packages/core/src/i18n/locales/it.ts index 44be22c1bd..782a3c7fc4 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/ja.ts b/packages/core/src/i18n/locales/ja.ts index ead1f2fb30..8bac14021d 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/ko.ts b/packages/core/src/i18n/locales/ko.ts index 2981ff1c36..de94329b19 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..a90210b572 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/no.ts b/packages/core/src/i18n/locales/no.ts index 72efc096ed..9ed6388dc7 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/pl.ts b/packages/core/src/i18n/locales/pl.ts index d00039633c..95751640b9 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/pt.ts b/packages/core/src/i18n/locales/pt.ts index fe719ce023..6914de9d2c 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/ru.ts b/packages/core/src/i18n/locales/ru.ts index a4a7987dfc..db116a3c4c 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..f53c4c39d1 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/uk.ts b/packages/core/src/i18n/locales/uk.ts index e9d379ac0b..e6101c8f69 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..23b0f4f1a7 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/vi.ts b/packages/core/src/i18n/locales/vi.ts index 8733fbf0ba..d52db4d48d 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: "OK", }, }; diff --git a/packages/core/src/i18n/locales/zh-tw.ts b/packages/core/src/i18n/locales/zh-tw.ts index 5ac37a80c7..0aba71ead4 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..0017c86672 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 ce56eac806..ab0482700f 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -135,6 +135,7 @@ export * from "./hooks/useBlockNoteEditor.js"; export * from "./hooks/useCreateBlockNote.js"; export * from "./hooks/useEditorChange.js"; export * from "./hooks/useEditorFocus.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..29f90cc657 --- /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( + "OK", + ); + // 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 c47db57fb0ab501bae2c5cb65200fcd8e60f8a14 Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 31 Aug 2026 16:31:57 +0200 Subject: [PATCH 2/9] 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 29f90cc657..818a8d1a96 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 2a52fe6c768e3b65a0c99d87de3d655563d5f96d Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 31 Aug 2026 16:56:55 +0200 Subject: [PATCH 3/9] 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 818a8d1a96..1f29fc6b63 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 1435d9bbb979c33ab7177b85ba3f9b195b90523d Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 31 Aug 2026 19:28:56 +0200 Subject: [PATCH 4/9] =?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 ab0482700f..ce56eac806 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -135,7 +135,6 @@ export * from "./hooks/useBlockNoteEditor.js"; export * from "./hooks/useCreateBlockNote.js"; export * from "./hooks/useEditorChange.js"; export * from "./hooks/useEditorFocus.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 1f29fc6b63..daaa6c7ba3 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 07f12859c19c1a156372c1f79fa1dc1b8ea0b185 Mon Sep 17 00:00:00 2001 From: yousefed Date: Tue, 1 Sep 2026 13:32:14 +0200 Subject: [PATCH 5/9] fix(ui): every form declares its submit control explicitly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Form.Root's optional omitSubmitButton becomes a required submitButton: ReactElement | "none" — the compiler now forces every caller to decide the form's one submit affordance instead of a boolean opt-out defaulting silently (ReactElement, not ReactNode, so the "none" sentinel can't be satisfied by an arbitrary string). @blocknote/react owns the default control (ScreenReaderOnlySubmit, which reads the dictionary) and its clip CSS, in one copy; the three skin Forms collapse to rendering whatever they're given. The embed tab shows why: its visible button is now the form's submitButton — inside the , one commit path for click, Enter and a mobile IME's action key, one labelled action for assistive technology. That required FilePanel.Button to take an explicit type ("button" | "submit"): the skins disagreed on the default (shadcn hardcoded submit, Mantine defaults to button), which is exactly what had forced the button outside the form before. Layout-wise the is a semantic wrapper only, never a box: every skin renders it as class bn-form and one shared rule gives it display: contents. Ariakit and shadcn rendered no wrapper element at all before the real arrived, so their flex+gap containers (popover contents, the file panel's tab column) lay out fields as direct children; mantine's containers are block, making contents a no-op there — verified per skin against computed layout. Breaking (release notes): Form.Root requires submitButton; FilePanel.Button requires type. The pointer:coarse input sizing in the same stylesheet region is from #2982. --- packages/ariakit/src/input/Form.tsx | 18 +++-------- packages/ariakit/src/panel/PanelButton.tsx | 3 +- packages/ariakit/src/style.css | 20 ------------ packages/mantine/src/blocknoteStyles.css | 22 +------------ packages/mantine/src/form/Form.tsx | 18 +++-------- packages/mantine/src/panel/PanelButton.tsx | 3 +- .../FilePanel/DefaultTabs/EmbedTab.tsx | 32 ++++++++++--------- .../Form/ScreenReaderOnlySubmit.tsx | 22 +++++++++++++ .../DefaultButtons/FileCaptionButton.tsx | 6 +++- .../DefaultButtons/FileRenameButton.tsx | 6 +++- .../LinkToolbar/EditLinkMenuItems.tsx | 6 +++- .../react/src/editor/ComponentsContext.tsx | 29 +++++++++++------ packages/react/src/editor/styles.css | 28 ++++++++++++++++ packages/react/src/index.ts | 1 + packages/shadcn/src/form/Form.tsx | 18 +++-------- packages/shadcn/src/panel/PanelButton.tsx | 4 +-- packages/shadcn/src/style.css | 20 ------------ .../AIMenu/PromptSuggestionMenu.tsx | 6 +++- .../end-to-end/form/popoverSubmit.test.tsx | 28 ++++++++-------- 19 files changed, 141 insertions(+), 149 deletions(-) create mode 100644 packages/react/src/components/Form/ScreenReaderOnlySubmit.tsx diff --git a/packages/ariakit/src/input/Form.tsx b/packages/ariakit/src/input/Form.tsx index 819bf4f3c7..83bf0af250 100644 --- a/packages/ariakit/src/input/Form.tsx +++ b/packages/ariakit/src/input/Form.tsx @@ -1,17 +1,17 @@ import { FormProvider as AriakitFormProvider } from "@ariakit/react"; import { assertEmpty } from "@blocknote/core"; -import { ComponentProps, useDictionary } from "@blocknote/react"; +import { ComponentProps } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, onSubmit, omitSubmitButton, ...rest } = props; - const dict = useDictionary(); + const { children, onSubmit, submitButton, ...rest } = props; assertEmpty(rest); return ( { // These forms have no action — a real submission would navigate. event.preventDefault(); @@ -19,17 +19,7 @@ export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { }} > {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. - */} - {!omitSubmitButton && ( - - )} + {submitButton === "none" ? null : submitButton} ); diff --git a/packages/ariakit/src/panel/PanelButton.tsx b/packages/ariakit/src/panel/PanelButton.tsx index b793112416..7e067279bd 100644 --- a/packages/ariakit/src/panel/PanelButton.tsx +++ b/packages/ariakit/src/panel/PanelButton.tsx @@ -8,12 +8,13 @@ export const PanelButton = forwardRef< HTMLButtonElement, ComponentProps["FilePanel"]["Button"] >((props, ref) => { - const { className, children, onClick, label, ...rest } = props; + const { className, children, type, onClick, label, ...rest } = props; assertEmpty(rest); return ( { - const { children, onSubmit, omitSubmitButton, ...rest } = props; - const dict = useDictionary(); + const { children, onSubmit, submitButton, ...rest } = props; assertEmpty(rest); return (
{ // These forms have no action — a real submission would navigate. event.preventDefault(); @@ -16,17 +16,7 @@ export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { }} > {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. - */} - {!omitSubmitButton && ( - - )} + {submitButton === "none" ? null : submitButton}
); }; diff --git a/packages/mantine/src/panel/PanelButton.tsx b/packages/mantine/src/panel/PanelButton.tsx index 73336a5375..95fc0152a7 100644 --- a/packages/mantine/src/panel/PanelButton.tsx +++ b/packages/mantine/src/panel/PanelButton.tsx @@ -8,12 +8,13 @@ export const PanelButton = forwardRef< HTMLButtonElement, ComponentProps["FilePanel"]["Button"] >((props, ref) => { - const { className, children, onClick, label, ...rest } = props; + const { className, children, type, onClick, label, ...rest } = props; assertEmpty(rest); 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. + The visible embed button IS the form's submit control: one commit + path (the form's `submit` event) whether it is clicked, Enter is + pressed, or a mobile IME's action key fires — and one labelled + action for assistive technology. */} - + + {dict.file_panel.embed.embed_button[block.type] || + dict.file_panel.embed.embed_button["file"]} + + } + > - - {dict.file_panel.embed.embed_button[block.type] || - dict.file_panel.embed.embed_button["file"]} - ); }; diff --git a/packages/react/src/components/Form/ScreenReaderOnlySubmit.tsx b/packages/react/src/components/Form/ScreenReaderOnlySubmit.tsx new file mode 100644 index 0000000000..d80abc5a48 --- /dev/null +++ b/packages/react/src/components/Form/ScreenReaderOnlySubmit.tsx @@ -0,0 +1,22 @@ +import { useDictionary } from "../../i18n/dictionary.js"; + +/** + * The default submit control for `Components.Generic.Form.Root`: visually + * hidden (clipped, not `display: none`, so it stays in the accessibility + * tree as a labelled control), out of the tab order so sighted keyboard + * users never land on a control they can't see. Its presence is what makes + * Enter submit a form with more than one field. + */ +export function ScreenReaderOnlySubmit() { + const dict = useDictionary(); + + return ( + + ); +} diff --git a/packages/react/src/components/FormattingToolbar/DefaultButtons/FileCaptionButton.tsx b/packages/react/src/components/FormattingToolbar/DefaultButtons/FileCaptionButton.tsx index 1065546c53..cc4b568476 100644 --- a/packages/react/src/components/FormattingToolbar/DefaultButtons/FileCaptionButton.tsx +++ b/packages/react/src/components/FormattingToolbar/DefaultButtons/FileCaptionButton.tsx @@ -9,6 +9,7 @@ import { ChangeEvent, useCallback, useState } from "react"; import { RiInputField } from "react-icons/ri"; import { useComponentsContext } from "../../../editor/ComponentsContext.js"; +import { ScreenReaderOnlySubmit } from "../../Form/ScreenReaderOnlySubmit.js"; import { useUIMode } from "../../../editor/UIModeContext.js"; import { useBlockNoteEditor } from "../../../hooks/useBlockNoteEditor.js"; import { useEditorState } from "../../../hooks/useEditorState.js"; @@ -117,7 +118,10 @@ export const FileCaptionButton = () => { className={"bn-popover-content bn-form-popover"} variant={"form-popover"} > - setPopoverOpen(false)}> + setPopoverOpen(false)} + submitButton={} + > } diff --git a/packages/react/src/components/FormattingToolbar/DefaultButtons/FileRenameButton.tsx b/packages/react/src/components/FormattingToolbar/DefaultButtons/FileRenameButton.tsx index 0138947c24..39f080538a 100644 --- a/packages/react/src/components/FormattingToolbar/DefaultButtons/FileRenameButton.tsx +++ b/packages/react/src/components/FormattingToolbar/DefaultButtons/FileRenameButton.tsx @@ -9,6 +9,7 @@ import { ChangeEvent, useCallback, useState } from "react"; import { RiFontFamily } from "react-icons/ri"; import { useComponentsContext } from "../../../editor/ComponentsContext.js"; +import { ScreenReaderOnlySubmit } from "../../Form/ScreenReaderOnlySubmit.js"; import { useUIMode } from "../../../editor/UIModeContext.js"; import { useBlockNoteEditor } from "../../../hooks/useBlockNoteEditor.js"; import { useEditorState } from "../../../hooks/useEditorState.js"; @@ -123,7 +124,10 @@ export const FileRenameButton = () => { className={"bn-popover-content bn-form-popover"} variant={"form-popover"} > - setPopoverOpen(false)}> + setPopoverOpen(false)} + submitButton={} + > } diff --git a/packages/react/src/components/LinkToolbar/EditLinkMenuItems.tsx b/packages/react/src/components/LinkToolbar/EditLinkMenuItems.tsx index 147404d2b8..0106f350d9 100644 --- a/packages/react/src/components/LinkToolbar/EditLinkMenuItems.tsx +++ b/packages/react/src/components/LinkToolbar/EditLinkMenuItems.tsx @@ -6,6 +6,7 @@ import { import { ChangeEvent, useCallback, useEffect, useState } from "react"; import { RiLink, RiText } from "react-icons/ri"; import { useComponentsContext } from "../../editor/ComponentsContext.js"; +import { ScreenReaderOnlySubmit } from "../Form/ScreenReaderOnlySubmit.js"; import { useExtension } from "../../hooks/useExtension.js"; import { useDictionary } from "../../i18n/dictionary.js"; import { LinkToolbarProps } from "./LinkToolbarProps.js"; @@ -63,7 +64,10 @@ export const EditLinkMenuItems = ( }, [editLink, currentUrl, currentText, props]); return ( - + } + > {/* // TODO: add labels? */} void; + /** + * Explicit, because the skins' underlying buttons disagree on the + * default (Mantine's is `type="button"`, shadcn's was `"submit"`) and + * a submit button inside a `Form.Root` must reliably submit on every + * skin. `"submit"` buttons need no `onClick` - the form's `onSubmit` + * is the single commit path, so clicking cannot fire twice. + */ + type: "button" | "submit"; + onClick?: () => void; } & ( | { children: ReactNode; label?: string } | { children?: undefined; label: string } @@ -317,16 +326,18 @@ export type ComponentProps = { */ onSubmit?: () => void; /** - * 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. + * The form's submit control, rendered inside the ``. Required, + * because it decides how the form can be committed at all: a submit + * button is what makes Enter submit a form with more than one field, + * and it is the control assistive technology activates. * - * 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. + * Pass `ScreenReaderOnlySubmit` for the usual case (a visually + * hidden, labelled control), a visible `type="submit"` button to make + * it double as the form's one submit affordance (the embed tab), or + * `"none"` to opt out explicitly - then the form must have exactly + * one field, or Enter reaches nothing. */ - omitSubmitButton?: boolean; + submitButton: ReactElement | "none"; }; TextInput: { className?: string; diff --git a/packages/react/src/editor/styles.css b/packages/react/src/editor/styles.css index a40e9fe306..ed4d440630 100644 --- a/packages/react/src/editor/styles.css +++ b/packages/react/src/editor/styles.css @@ -565,3 +565,31 @@ body:has(.bn-scroll-container) { .bn-root em-emoji-picker { max-height: 100%; } + +/* Form.Root's is a semantic wrapper, never a layout box: the skins' + * containers (popover contents, the file panel's tab column) lay out the + * fields and the embed tab's submit button as their own flex children, and + * two skins rendered no wrapper element at all before the real + * arrived. `contents` keeps that geometry in every skin — verified against + * each skin's containers (mantine's are block, so this is a no-op there; + * ariakit/shadcn have flex+gap containers that need it). Skins must not + * style .bn-form as a box. */ +.bn-form { + display: contents; +} + +/* The Form.Root default submit control (`ScreenReaderOnlySubmit`): visually + * hidden but NOT `display: none` - it must stay in the accessibility tree as + * a labelled control, and its presence is what makes Enter submit a form + * with more than one field. */ +.bn-screen-reader-only-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/react/src/index.ts b/packages/react/src/index.ts index ce56eac806..37bee7a38c 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -31,6 +31,7 @@ export * from "./components/FormattingToolbar/DefaultButtons/AddTiptapCommentBut export * from "./components/FormattingToolbar/DefaultButtons/BasicTextStyleButton.js"; export * from "./components/FormattingToolbar/DefaultButtons/ColorStyleButton.js"; export * from "./components/FormattingToolbar/DefaultButtons/CreateLinkButton.js"; +export * from "./components/Form/ScreenReaderOnlySubmit.js"; export * from "./components/FormattingToolbar/DefaultButtons/FileCaptionButton.js"; export * from "./components/FormattingToolbar/DefaultButtons/FileDeleteButton.js"; export * from "./components/FormattingToolbar/DefaultButtons/FileDownloadButton.js"; diff --git a/packages/shadcn/src/form/Form.tsx b/packages/shadcn/src/form/Form.tsx index f0cc1e7d0e..d1b23d72b5 100644 --- a/packages/shadcn/src/form/Form.tsx +++ b/packages/shadcn/src/form/Form.tsx @@ -1,14 +1,14 @@ import { assertEmpty } from "@blocknote/core"; -import { ComponentProps, useDictionary } from "@blocknote/react"; +import { ComponentProps } from "@blocknote/react"; export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { - const { children, onSubmit, omitSubmitButton, ...rest } = props; - const dict = useDictionary(); + const { children, onSubmit, submitButton, ...rest } = props; assertEmpty(rest); return ( { // These forms have no action — a real submission would navigate. event.preventDefault(); @@ -16,17 +16,7 @@ export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => { }} > {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. - */} - {!omitSubmitButton && ( - - )} + {submitButton === "none" ? null : submitButton} ); }; diff --git a/packages/shadcn/src/panel/PanelButton.tsx b/packages/shadcn/src/panel/PanelButton.tsx index ecaa806b8a..9633ff9294 100644 --- a/packages/shadcn/src/panel/PanelButton.tsx +++ b/packages/shadcn/src/panel/PanelButton.tsx @@ -8,7 +8,7 @@ export const PanelButton = forwardRef< HTMLButtonElement, ComponentProps["FilePanel"]["Button"] >((props, ref) => { - const { className, children, onClick, label, ...rest } = props; + const { className, children, type, onClick, label, ...rest } = props; assertEmpty(rest); @@ -16,7 +16,7 @@ export const PanelButton = forwardRef< return ( { return (
- + } + > { expect((submit as HTMLButtonElement).tabIndex).toBe(-1); }); - 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. + test("the embed tab's visible button is the form's one submit control", async () => { + // The embed button lives inside the `` as its `submitButton`, so + // clicking it, pressing Enter, and a mobile IME's action key all commit + // through the same `submit` event — and assistive technology sees exactly + // one labelled action for the one thing this panel does. Exactly one: + // a second (hidden) submit control would announce two. 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); + expect(form, "the embed field must be in a form").not.toBeNull(); + const submits = form!.querySelectorAll("button[type=submit]"); + expect(submits.length).toBe(1); + expect( + (submits[0] as HTMLElement).textContent?.trim(), + "the submit control is the visible, labelled embed button", + ).not.toBe("Submit"); + expect(form!.querySelectorAll("button").length).toBe(1); }); test("the embed tab's URL field commits", async () => { From 1e75c2c320097499560698590d69e9442c77c90e Mon Sep 17 00:00:00 2001 From: yousefed Date: Tue, 1 Sep 2026 13:32:32 +0200 Subject: [PATCH 6/9] test: platform-contract suites get their own directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit implicitSubmit and compositionSubmit assert browser behavior against raw createElement fixtures — nothing BlockNote in them — so they move out of form/ into end-to-end/platform/ with a README stating the contract: these are the per-engine platform facts Form.Root's design rests on, and a red test here after a browser update points at the platform fact that moved, not at BlockNote. --- tests/src/end-to-end/platform/README.md | 19 +++++++++++++++++++ .../compositionSubmit.test.tsx | 0 .../implicitSubmit.test.tsx | 0 3 files changed, 19 insertions(+) create mode 100644 tests/src/end-to-end/platform/README.md rename tests/src/end-to-end/{form => platform}/compositionSubmit.test.tsx (100%) rename tests/src/end-to-end/{form => platform}/implicitSubmit.test.tsx (100%) diff --git a/tests/src/end-to-end/platform/README.md b/tests/src/end-to-end/platform/README.md new file mode 100644 index 0000000000..e2a72c7120 --- /dev/null +++ b/tests/src/end-to-end/platform/README.md @@ -0,0 +1,19 @@ +# Platform-contract tests + +Nothing in here tests BlockNote. These suites assert **browser platform +behavior** that BlockNote's design depends on, against raw +`document.createElement` fixtures — per engine, so a deviation names the +platform fact that broke instead of surfacing as a mystery failure in a +BlockNote suite: + +- `implicitSubmit`: the HTML implicit-form-submission rules (a single field + submits without a submit button; multiple fields need a submit control; a + visually-hidden-but-clipped button still counts; no double submit). This is + what `Form.Root`'s required `submitButton` prop is built on. +- `compositionSubmit`: the IME consumes the confirming Enter (keyCode 229, no + default action), so implicit submission cannot fire mid-composition — the + fact that made `Form.Root`'s submit path safe without any `isComposing` + guard. + +If a test here goes red after a browser update, the fix likely belongs in +`Form.Root`'s contract, not in the test. diff --git a/tests/src/end-to-end/form/compositionSubmit.test.tsx b/tests/src/end-to-end/platform/compositionSubmit.test.tsx similarity index 100% rename from tests/src/end-to-end/form/compositionSubmit.test.tsx rename to tests/src/end-to-end/platform/compositionSubmit.test.tsx diff --git a/tests/src/end-to-end/form/implicitSubmit.test.tsx b/tests/src/end-to-end/platform/implicitSubmit.test.tsx similarity index 100% rename from tests/src/end-to-end/form/implicitSubmit.test.tsx rename to tests/src/end-to-end/platform/implicitSubmit.test.tsx From ec8ed010420b479710fdec421622d57f8633dd4f Mon Sep 17 00:00:00 2001 From: yousefed Date: Tue, 1 Sep 2026 13:32:35 +0200 Subject: [PATCH 7/9] docs(mantine): portalling TODO and the focus-trap contract The portalRoot-implies-mobile inference in Popover gets a TODO pending the portalling discussion. TextInput documents why its manual preventScroll focus and Mantine's traps can never fight: no trap runs in the form popovers at all (Popover's trapFocus defaults to false), and in trap-active subtrees nearby (toolbar Tab-cycling, desktop menus) the data-autofocus attribute makes a trap pick this same element. --- packages/mantine/src/form/TextInput.tsx | 7 +++++++ packages/mantine/src/popover/Popover.tsx | 3 +++ 2 files changed, 10 insertions(+) diff --git a/packages/mantine/src/form/TextInput.tsx b/packages/mantine/src/form/TextInput.tsx index 4d2e2bcb7f..cee3083392 100644 --- a/packages/mantine/src/form/TextInput.tsx +++ b/packages/mantine/src/form/TextInput.tsx @@ -33,6 +33,13 @@ export const TextInput = forwardRef< // 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). + // + // No Mantine focus trap competes with this in the form popovers (Popover's + // `trapFocus` defaults to false), but trap-active subtrees do exist nearby + // (the toolbar's Tab-cycling trap; Menu's default trap on desktop) — the + // `data-autofocus` below makes any such trap pick this same element + // instead of falling back to "first focusable", so the two mechanisms can + // never fight over where focus lands. const inputRef = useRef(null); const setRefs = useMergeRefs([inputRef, ref]); useEffect(() => { diff --git a/packages/mantine/src/popover/Popover.tsx b/packages/mantine/src/popover/Popover.tsx index 35a19590cf..e7c5b11c3a 100644 --- a/packages/mantine/src/popover/Popover.tsx +++ b/packages/mantine/src/popover/Popover.tsx @@ -17,6 +17,9 @@ export const Popover = ( // 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. + // TODO: clean this up once we've settled on a proper portalling solution + // (pending discussion) — inferring mobile-ness from `portalRoot` should + // become an explicit signal. const isMobileToolbarPopover = !!portalRoot; assertEmpty(rest); From 34c528b87da3286a0fbb3265aa32c167b096fc7d Mon Sep 17 00:00:00 2001 From: yousefed Date: Tue, 1 Sep 2026 20:03:32 +0200 Subject: [PATCH 8/9] refactor(ui): one autofocus implementation, shared as useAutoFocus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From review: the manual focus block was copy-pasted across the three skins and had already drifted (mantine's copy had grown a comment and a data-autofocus attribute the others lacked). The hook in @blocknote/react now owns the ref, the focus({ preventScroll: true }) effect, and the rationale — including how the official implementations compare (React's autoFocus is a bare .focus() at commit, which is exactly the scroll-yank this exists to avoid; Mantine defers via setTimeout; floating-ui via microtask + rAF) and why no extra deferral is used here: nothing to wait for, and added hops erode the user-gesture window in which iOS Safari lets a programmatic focus open the keyboard (validated on real iOS). data-autofocus is set per skin only where the UI library reads it: Mantine's focus trap and Ariakit's dialog initial-focus both select it (Ariakit's popovers run autoFocusOnShow by default, so the attribute makes their pick explicit instead of positional); shadcn's Base UI has no attribute convention — its mechanism is the initialFocus prop — so that skin omits it. --- packages/ariakit/src/input/TextInput.tsx | 19 +++------ packages/mantine/src/form/TextInput.tsx | 25 +++--------- packages/react/src/hooks/useAutoFocus.ts | 51 ++++++++++++++++++++++++ packages/react/src/index.ts | 1 + packages/shadcn/src/form/TextInput.tsx | 18 +++------ 5 files changed, 68 insertions(+), 46 deletions(-) create mode 100644 packages/react/src/hooks/useAutoFocus.ts diff --git a/packages/ariakit/src/input/TextInput.tsx b/packages/ariakit/src/input/TextInput.tsx index 35b02b92d0..3856d0e85e 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, useMergeRefs } from "@blocknote/react"; -import { forwardRef, useEffect, useRef } from "react"; +import { ComponentProps, useMergeRefs, useAutoFocus } from "@blocknote/react"; +import { forwardRef } from "react"; export const TextInput = forwardRef< HTMLInputElement, @@ -31,18 +31,10 @@ 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); + // Rationale (and the trap contract `data-autofocus` serves) in the hook. + + const inputRef = useAutoFocus(autoFocus); const setRefs = useMergeRefs([inputRef, ref]); - useEffect(() => { - if (autoFocus) { - inputRef.current?.focus({ preventScroll: true }); - } - }, [autoFocus]); return ( <> @@ -56,6 +48,7 @@ export const TextInput = forwardRef< variant === "large" ? "bn-ak-input-large" : "", )} ref={setRefs} + data-autofocus={autoFocus ? "true" : undefined} name={name} value={value} placeholder={placeholder} diff --git a/packages/mantine/src/form/TextInput.tsx b/packages/mantine/src/form/TextInput.tsx index cee3083392..9b05e66bbb 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, useMergeRefs } from "@blocknote/react"; -import { forwardRef, useEffect, useRef } from "react"; +import { ComponentProps, useMergeRefs, useAutoFocus } from "@blocknote/react"; +import { forwardRef } from "react"; export const TextInput = forwardRef< HTMLInputElement, @@ -28,25 +28,10 @@ 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). - // - // No Mantine focus trap competes with this in the form popovers (Popover's - // `trapFocus` defaults to false), but trap-active subtrees do exist nearby - // (the toolbar's Tab-cycling trap; Menu's default trap on desktop) — the - // `data-autofocus` below makes any such trap pick this same element - // instead of falling back to "first focusable", so the two mechanisms can - // never fight over where focus lands. - const inputRef = useRef(null); + // Rationale (and the trap contract `data-autofocus` serves) in the hook. + + const inputRef = useAutoFocus(autoFocus); const setRefs = useMergeRefs([inputRef, ref]); - useEffect(() => { - if (autoFocus) { - inputRef.current?.focus({ preventScroll: true }); - } - }, [autoFocus]); return ( el.focus({ preventScroll: true }))`; floating-ui's + * FloatingFocusManager is layout effect → microtask → rAF → + * `focus({ preventScroll })`. Ours is a plain effect with unconditional + * `preventScroll` — stronger scroll-safety than floating-ui (which lets a + * chosen initial element scroll), and deliberately *without* their extra + * deferral layers: we have no tabIndex setters to wait for, and every added + * hop erodes the user-gesture window inside which iOS Safari allows a + * programmatic focus to open the on-screen keyboard (the real-device suite + * validated the keyboard appears with this timing). `preventScroll` also + * makes the timing not load-bearing for layout: no ordering relative to + * floating-ui's positioning can scroll the page. + * + * Skins whose UI library reads `data-autofocus` should also set it (value + * "true") on the same element: Mantine's focus trap and Ariakit's dialog + * initial-focus both select `[data-autofocus]`, so the attribute makes any + * trap that activates pick this same element instead of falling back to + * "first focusable" — the two mechanisms can never fight over where focus + * lands. Skins on libraries without that convention omit it as dead markup: + * the shadcn skin's Base UI has no attribute-based initial focus at all — + * its mechanism is the `initialFocus` prop on popups. + * + * Returns the ref to attach; merge it with a forwarded ref via + * `useMergeRefs`. + */ +export function useAutoFocus( + autoFocus: boolean | undefined, +): RefObject { + const elementRef = useRef(null); + + useEffect(() => { + if (autoFocus) { + elementRef.current?.focus({ preventScroll: true }); + } + }, [autoFocus]); + + return elementRef; +} diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 37bee7a38c..821cd4bc92 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -135,6 +135,7 @@ export * from "./hooks/useActiveStyles.js"; export * from "./hooks/useBlockNoteEditor.js"; export * from "./hooks/useCreateBlockNote.js"; export * from "./hooks/useEditorChange.js"; +export * from "./hooks/useAutoFocus.js"; export * from "./hooks/useEditorFocus.js"; export * from "./hooks/useEditorDomElement.js"; export * from "./hooks/useEditorSelectionBoundingBox.js"; diff --git a/packages/shadcn/src/form/TextInput.tsx b/packages/shadcn/src/form/TextInput.tsx index 4527984db3..72b5abb6d1 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, useMergeRefs } from "@blocknote/react"; -import { forwardRef, useEffect, useRef } from "react"; +import { ComponentProps, useMergeRefs, useAutoFocus } from "@blocknote/react"; +import { forwardRef } from "react"; import { useShadCNComponentsContext } from "../ShadCNComponentsContext.js"; import { cn } from "../lib/utils.js"; @@ -29,18 +29,10 @@ 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); + // Rationale (and the trap contract `data-autofocus` serves) in the hook. + + const inputRef = useAutoFocus(autoFocus); const setRefs = useMergeRefs([inputRef, ref]); - useEffect(() => { - if (autoFocus) { - inputRef.current?.focus({ preventScroll: true }); - } - }, [autoFocus]); const ShadCNComponents = useShadCNComponentsContext()!; From ce456bcf910706dbabb396630c269ba1ae0d275f Mon Sep 17 00:00:00 2001 From: yousefed Date: Tue, 1 Sep 2026 20:29:49 +0200 Subject: [PATCH 9/9] refactor(ui): review follow-ups on focus ownership and docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - EmbedTab's submit callback renamed to handleSubmit (consistency with EditLinkMenuItems). - The Form contract docs in ComponentsContext trimmed to the contract; the device-verified rationale lives in the PR and its tests. - One focus principle across skins, now enforced rather than raced: BlockNote owns focus in its popovers (useAutoFocus). Ariakit's autoFocusOnShow is disabled — its default bare-focuses the first tabbable (no preventScroll, plus a Safari scrollIntoView), the exact scroll-yank useAutoFocus avoids, previously masked only by effect ordering. Mantine's popover trapFocus pinned to its (identical) default, dead ternary dropped. data-autofocus stays only where the library reads it AND focuses safely — Mantine's trap; Ariakit fails the second test, Base UI the first. --- packages/ariakit/src/input/TextInput.tsx | 1 - packages/ariakit/src/popover/Popover.tsx | 6 ++++ packages/mantine/src/popover/Popover.tsx | 7 ++-- .../FilePanel/DefaultTabs/EmbedTab.tsx | 4 +-- .../react/src/editor/ComponentsContext.tsx | 27 +++++---------- packages/react/src/hooks/useAutoFocus.ts | 33 +++++++------------ 6 files changed, 32 insertions(+), 46 deletions(-) diff --git a/packages/ariakit/src/input/TextInput.tsx b/packages/ariakit/src/input/TextInput.tsx index 3856d0e85e..bf4e859670 100644 --- a/packages/ariakit/src/input/TextInput.tsx +++ b/packages/ariakit/src/input/TextInput.tsx @@ -48,7 +48,6 @@ export const TextInput = forwardRef< variant === "large" ? "bn-ak-input-large" : "", )} ref={setRefs} - data-autofocus={autoFocus ? "true" : undefined} name={name} value={value} placeholder={placeholder} diff --git a/packages/ariakit/src/popover/Popover.tsx b/packages/ariakit/src/popover/Popover.tsx index df8e01128b..cc124249cc 100644 --- a/packages/ariakit/src/popover/Popover.tsx +++ b/packages/ariakit/src/popover/Popover.tsx @@ -40,6 +40,12 @@ export const PopoverContent = forwardRef< className || "", variant === "panel-popover" ? "bn-ak-panel-popover" : "", )} + // BlockNote owns focus in its popovers (useAutoFocus, which prevents + // scrolling). Ariakit's default would bare-focus the first tabbable — + // in form popovers the very input the hook handles, re-introducing + // the scroll-yank it exists to avoid. No other skin's library moves + // focus to an input on open either. + autoFocusOnShow={false} portalElement={portalRoot ?? undefined} ref={ref} > diff --git a/packages/mantine/src/popover/Popover.tsx b/packages/mantine/src/popover/Popover.tsx index e7c5b11c3a..422520b830 100644 --- a/packages/mantine/src/popover/Popover.tsx +++ b/packages/mantine/src/popover/Popover.tsx @@ -29,9 +29,10 @@ export const Popover = ( middlewares={{ size: { padding: 20 } }} withinPortal={!!portalRoot} 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={isMobileToolbarPopover ? false : undefined} + // Pins Mantine's default: a trap would move focus into the dropdown, + // which on mobile blurs the contentEditable and dismisses the + // keyboard. BlockNote owns focus in its popovers (useAutoFocus). + trapFocus={false} // 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 diff --git a/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx b/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx index 08d1dd9a49..52ef673e3b 100644 --- a/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx +++ b/packages/react/src/components/FilePanel/DefaultTabs/EmbedTab.tsx @@ -37,7 +37,7 @@ export const EmbedTab = < [], ); - const embedURL = useCallback(() => { + const handleSubmit = useCallback(() => { if (!editor.getBlock(props.blockId)) { return; } @@ -62,7 +62,7 @@ export const EmbedTab = < action for assistive technology. */} ` 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). + * Called on the form's `submit` event. Implementations must render a + * real `` and `preventDefault`: native submission is the only + * path that works for every input source — mobile IMEs commit + * through it without dispatching any key event. */ onSubmit?: () => void; /** - * The form's submit control, rendered inside the ``. Required, - * because it decides how the form can be committed at all: a submit - * button is what makes Enter submit a form with more than one field, - * and it is the control assistive technology activates. - * - * Pass `ScreenReaderOnlySubmit` for the usual case (a visually - * hidden, labelled control), a visible `type="submit"` button to make - * it double as the form's one submit affordance (the embed tab), or - * `"none"` to opt out explicitly - then the form must have exactly - * one field, or Enter reaches nothing. + * The form's submit control, rendered inside the ``. Usually + * `ScreenReaderOnlySubmit`; a visible `type="submit"` button to make + * it the form's one affordance (the embed tab); or `"none"` — then + * the form must have exactly one field, or Enter submits nothing. */ submitButton: ReactElement | "none"; }; diff --git a/packages/react/src/hooks/useAutoFocus.ts b/packages/react/src/hooks/useAutoFocus.ts index dcd4ae5dad..d7f0bdba5d 100644 --- a/packages/react/src/hooks/useAutoFocus.ts +++ b/packages/react/src/hooks/useAutoFocus.ts @@ -10,28 +10,19 @@ import { RefObject, useEffect, useRef } from "react"; * would run while the popover is still at its pre-positioned spot and yank * the page (on mobile, right out from under the block being edited). * - * The shape follows the official implementations for this situation, which - * all defer and/or prevent scrolling: Mantine's focus-on-open is - * `setTimeout(() => el.focus({ preventScroll: true }))`; floating-ui's - * FloatingFocusManager is layout effect → microtask → rAF → - * `focus({ preventScroll })`. Ours is a plain effect with unconditional - * `preventScroll` — stronger scroll-safety than floating-ui (which lets a - * chosen initial element scroll), and deliberately *without* their extra - * deferral layers: we have no tabIndex setters to wait for, and every added - * hop erodes the user-gesture window inside which iOS Safari allows a - * programmatic focus to open the on-screen keyboard (the real-device suite - * validated the keyboard appears with this timing). `preventScroll` also - * makes the timing not load-bearing for layout: no ordering relative to - * floating-ui's positioning can scroll the page. + * The shape matches the official popover-autofocus implementations + * (Mantine: setTimeout + preventScroll; floating-ui: microtask + rAF + + * preventScroll), minus their deferral layers: there is nothing here to + * wait for, and added hops erode the user-gesture window in which iOS + * Safari lets a programmatic focus open the keyboard (validated on real + * iOS). `preventScroll` also makes the timing not load-bearing: no + * ordering relative to floating-ui's positioning can scroll the page. * - * Skins whose UI library reads `data-autofocus` should also set it (value - * "true") on the same element: Mantine's focus trap and Ariakit's dialog - * initial-focus both select `[data-autofocus]`, so the attribute makes any - * trap that activates pick this same element instead of falling back to - * "first focusable" — the two mechanisms can never fight over where focus - * lands. Skins on libraries without that convention omit it as dead markup: - * the shadcn skin's Base UI has no attribute-based initial focus at all — - * its mechanism is the `initialFocus` prop on popups. + * A skin sets `data-autofocus` on the same element only when its UI + * library both reads the attribute AND focuses safely: Mantine's trap + * does (`focus({ preventScroll: true })`). Ariakit reads it but + * bare-focuses, so that skin disables its `autoFocusOnShow` instead (see + * its Popover); Base UI has no attribute convention. * * Returns the ref to attach; merge it with a forwarded ref via * `useMergeRefs`.