fix(mobile): Android Chrome keyboard collapse, Enter handling, and focus API - #3025
Draft
YousefED wants to merge 21 commits into
Draft
fix(mobile): Android Chrome keyboard collapse, Enter handling, and focus API#3025YousefED wants to merge 21 commits into
YousefED wants to merge 21 commits into
Conversation
…esizes
Mantine Popover's hideDetached (default true) reacts to the viewport resize
that the on-screen keyboard causes whenever focus moves into an input by
setting display:none on the still-mounted dropdown. A display:none ancestor
cannot hold focus, so the popover's input blurred, the toolbar unmounted, and
the keyboard dismissed — the Android Chrome collapse this branch set out to
fix. Disabling hideDetached for portalled (mobile) popovers fixes the
collapse at its root, so the previous workaround (autoFocus={false} plus a
scrollIntoView on click, which regressed desktop and never prevented the
collapse) is reverted, restoring autofocus and removing the onClickCapture
plumbing.
Also closes the create-link popover explicitly on submit: the desktop toolbar
unmounts on the store update (taking the popover with it), but the mobile
toolbar stays mounted, so without this the popover lingered over the toolbar
and swallowed taps on its buttons.
prosemirror-view ignores Enter keydowns entirely on Android Chrome and falls back to parsing the browser's native DOM split, whose recognition heuristic fails against BlockNote's nested block DOM: depending on timing a single Enter did nothing, split twice, or corrupted the document (#3001 — Enter inserting a space, breaking tables; the stuck down-arrow was collateral of the corrupted state). This is not a regression: every published pairing back to 0.29.0 with its contemporary prosemirror-view races the same way. Intercept the beforeinput (insertParagraph/insertLineBreak) the IME emits, force-flush pending DOM observations (parity prosemirror-view skips on its Android bail; without it the synthesized Enter can run against a stale selection), and dispatch Enter through the full keymap chain — bypassing the fragile DOM diffing entirely. Hardware and IME Enter both converge on this beforeinput on Android, so one interception covers both.
Adds editor.onFocusChange(cb, { includeFloatingUI }) and the matching
isFocused({ includeFloatingUI }) option, plus a useEditorFocusChange hook.
The default reports raw content-area focus/blur (tiptap events, now exposed
through EventManager like onChange/onSelectionChange instead of reaching
into _tiptapEditor). With includeFloatingUI, document-level tracking treats
everything portalled into editor.portalElement as focused and emits only
settled, deduplicated transitions — covering the case tiptap events cannot
(focus leaving from the floating UI), with a next-frame re-check because
activeElement is transiently <body> during focus handoffs and relatedTarget
is unreliable on mobile.
MobileFormattingToolbarController now consumes this API (holding only the
settled values — reading focus live during a render can observe a
mid-handoff frame). The callback hooks use the latest-ref pattern so
unmemoized callbacks no longer resubscribe every render.
getSelectedLinkUrl looked up the link mark exactly at selection.from, which misses the mark at the link's left boundary — selecting a whole link made the link popover's URL field come up empty. Use from + 1, the same boundary correction editLink already applies.
Adds a fourth browser instance, "android" (chromium with an Android UA, touch, and a phone viewport via a per-instance provider override — instance contextOptions are silently ignored), running the mobile-specific tests in end-to-end/mobile/ plus the screenshot-free behavioral suites where Android genuinely differs: keyboardhandlers, emojipicker, copypaste. In this context isTouchDevice() and prosemirror-view's Android detection are genuinely true, so the mobile tests need no platform stubs. The mobile tests cover the keyboard-lifecycle contract (toolbar shows with the keyboard, hides when it closes), the link popover surviving IME resizes through creation and URL pre-fill, and the Android Enter beforeinput path — each proven to fail with its fix reverted. Tests driving selection/resizing with positional mouse idioms carry skipIf(onAndroid) guards with the reason in place. Iframe-element screenshots permanently drop the context's touch emulation (plain element shots and viewport calls are fine), so screenshot suites stay out of this instance and ensureTouchEmulation() self-heals detection if that ever regresses.
Adds tests/device/: a dependency-free WebDriver REST client, a gesture layer holding every platform quirk (iOS only honors Appium native taps for focus/keyboard, with Safari-chrome screen offsets and verify-and-recover ladders; Android element clicks and key events suffice, with soft Enter converging on the same beforeinput path as the IME), BlockNote page helpers, a BrowserStackLocal tunnel + host-rewriting proxy as global setup, and a two-device matrix (Galaxy S22, iPhone 16e). Run with pnpm run test:device against a local dev server; suites self-skip without BROWSERSTACK_* credentials. Covers the mobile toolbar lifecycle, button reliability, the full link flow, typing, and soft-keyboard Enter (fails-without-fix verified on device for the Enter and popover fixes). Includes a nightly workflow (requires the BROWSERSTACK_* repo secrets).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| runs-on: ubuntu-latest | ||
| timeout-minutes: 45 | ||
| steps: | ||
| - uses: actions/checkout@v4 |
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: pnpm/action-setup@v4 |
|
|
||
| - uses: pnpm/action-setup@v4 | ||
|
|
||
| - uses: actions/setup-node@v4 |
|
|
||
| - name: Upload screenshots | ||
| if: always() | ||
| uses: actions/upload-artifact@v4 |
| runs-on: ubuntu-latest | ||
| timeout-minutes: 45 | ||
| steps: | ||
| - uses: actions/checkout@v4 |
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/diagram-block
@blocknote/mantine
@blocknote/math-block
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
|
The pointer:coarse rule cherry-picked in the previous commit had no test. Two layers, because the rule and the behaviour it prevents are testable in different places: - mobileToolbar (android instance): assert the URL input's computed font-size is at least 16px while the popover is open. Emulation can't reproduce the zoom itself — it's device behaviour, not engine behaviour — so this guards the CSS contract. Proven red with the rule removed (reports 12px and names the rule). - formattingToolbar.device (real iPhone): capture visualViewport.scale before opening the popover and assert focusing the URL input doesn't increase it. That pins the actual behaviour on the only place it happens.
Review follow-ups on the focus-within-UI tracking added earlier in this PR: - `includeFloatingUI` -> `includeEditorUI`. The old name read as the floating-ui library (which is in fact what these popovers are built on), when the option really means "count the editor's own UI — toolbars, menus, popovers — as focused, not just the content area". - Replace requestAnimationFrame with setTimeout for the blur-side settle. Both defer past the current task, which is what's actually needed; a frame buys nothing here (nothing is being painted) and doesn't run in a background tab. Measured rather than assumed: settling synchronously fails, and a microtask fails too — the ariakit and shadcn link popovers restore focus a task later — while setTimeout passes. - Drop the cancel-on-focusin. It only avoided a redundant re-check: the emit is already deduplicated, so the trailing settle is a no-op. What stays, and why it isn't further reducible: document-level listeners (portalled UI sits outside the content area, so tiptap's focus/blur can't see it), the deferral above, the dedupe (these listeners see every focus change on the page, most unrelated to the editor), and `editor.isFocused()` alongside the activeElement check — the latter alone would break in shadow DOM, where document.activeElement is the host rather than the editor. Android instance: 45 passed, 4 expected skips.
The focus API had no direct coverage — only indirect exercise through the mobile toolbar's behaviour. These run in the browser suite (all three engines) because every interesting case is DOM semantics that jsdom can't reproduce. Two of them pin the platform contract the tracker is designed around, so a future engine change surfaces here rather than as a mystery in the mobile UI: the documented focus event order (blur, focusout, focus, focusin — verified identical on chromium, firefox and webkit), and the fact that `document.activeElement` reads as `<body>` during focusout, which is precisely why that side has to be deferred. The rest cover the edge cases: content focus vs. focus in portalled UI, the content -> popover handoff that must not report a blur, focus moving between two UI elements, focus changes elsewhere on the page that must not reach subscribers at all (these are document-level listeners), two editors on one page staying independent, unsubscribing, and two subscribers sharing the reference-counted listeners. Note the harness mounts each editor inside a container, mirroring BlockNoteView. That is load-bearing: `isWithinEditor` treats the mount element's *parent* as the editor boundary (pre-existing, #2591), so an editor mounted directly into <body> would treat the whole page as its own UI. Worth tightening separately — it also drives SideMenu's click-outside detection.
`editor.mount(element)` turns `element` itself into the content area, so
the check's boundary — deliberately the content area's *parent*, so that
sibling (non-portalled) UI counts — is whatever that element's parent
happens to be.
With BlockNoteView the mount element always sits inside the
`bn-container` wrapper, so the boundary is that wrapper. But mounting a
bare element that is a direct child of `<body>`, which is exactly what
the vanilla-JS setup in the docs does, makes the boundary the whole
page: every element then counts as "within the editor", so
`isFocused({ includeEditorUI: true })` is always true, focus events with
that option never report a blur, and the side menu's click-outside check
never matches.
Comment only — fixing it means identifying the editor's UI without
relying on the parent, which changes behaviour for all three callers
(focus tracking, side menu, UniqueID) and deserves its own change.
`useEditorFocusChange` only offered a callback, so anything that wanted to *render* based on focus had to wire up useState + useEffect itself — which MobileFormattingToolbarController did, including a manual re-sync for focus that changed before the subscription attached. `useEditorFocus(options?, editor?)` returns that as state. The two hooks now mirror the split the codebase already has between `useEditorState` and `useEditorChange`: state for rendering, callback for side effects. Built on useSyncExternalStore, so there's no extra render on mount and no tearing. The snapshot is deliberately the last *settled* value rather than a live `isFocused()` read: reading focus during an arbitrary render can catch a mid-handoff frame where document.activeElement is transiently <body> and the editor looks unfocused. The controller now reads as one line, and no longer needs the editor instance at all. Covered by browser tests for the behaviours that justify the hook: content focus/blur, staying focused across a handoff into portalled UI, not re-rendering for unrelated focus changes elsewhere on the page (these are document-level listeners, so it sees them all), and not re-rendering while typing.
Adds a regression test for the latest-ref pattern in `useEditorFocusChange`: an inline callback (a new identity every render) must not cause the editor subscription to be torn down and re-attached. Measured both ways before writing it — the naive implementation, with the callback in the dependency array, resubscribes once per render (6 after 5 re-renders); the latest-ref version stays at 1. That churn is worse than it looks with `includeEditorUI`, where the subscription is reference-counted: each cycle detaches and re-attaches the document focus listeners and resets the settled baseline. Also corrects a comment in useEditorFocus: React re-checks the snapshot *after* subscribing (its subscribe effect is registered before the consistency-check effect — verified in react-dom's source), so the re-sync inside subscribe is what makes the "focus changed between render and subscription" case work.
… for The previous wording said it covers "UI rendered as a sibling of the content" without saying which UI that is — and BlockNote's own default UI is entirely portalled (a mounted editor's container has exactly two children: the content element and the portal), so on its own that justification doesn't hold up. The real case is UI the host app passes as `BlockNoteView` children, which React renders as siblings of the content element. The shipped "Static Formatting Toolbar" example is exactly that: `<FormattingToolbar />` rendered inline rather than through a controller, putting eleven focusable buttons next to the content. Without the hop, focusing one of them reads as "outside the editor", which would dismiss the mobile toolbar mid-interaction and trip the side menu's click-outside check.
Opening a toolbar popover reset the page scroll to the top, taking the
block being edited off screen entirely. Measured on the
mobile-formatting-toolbar example: scrollTop 451 -> 0, putting the
edited editor at y=800 in a 427px viewport.
Cause: the input's native `autofocus` fires while floating-ui has not
positioned the popover yet, so the browser scrolls to the popover's
pre-positioned spot — the top of the container — instead of where it
ends up. Focusing through a ref with `{ preventScroll: true }` keeps
the page still; floating-ui positions the popover regardless, so it
still appears in the right place.
Applied in all three UI packages' form TextInput, since they all take
`autoFocus` for popover inputs (link, file embed, rename, caption).
Covered by a mobile e2e test asserting neither the scroll position nor
the edited editor moves when the popover opens — proven red without the
fix (451 -> 0). Reported from a real device.
Reported from a device: select text in the first of two editors, open the link popover, type a URL, press Enter — no link is created and focus jumps to the *second* editor. Android's IME chooses the Enter key's action itself. With no surrounding <form> and no enterkeyhint, Chrome picks IME_ACTION_NEXT whenever another focusable element follows, and Next advances focus rather than dispatching a key event — so the keydown handler that creates the link never runs. That also explains the asymmetry in the report: from the last editor on the page nothing focusable follows, so Chrome picks Done instead, Enter is dispatched, and the link is created as expected. Setting enterkeyhint="done" on the popover inputs makes the key report itself as Enter everywhere. Applied in all three UI packages, so it covers the link, file embed, rename and caption popovers. The mobile toolbar test now asserts the attribute — a DOM-contract check rather than a behavioural one, since emulation always dispatches a real Enter and so cannot reproduce the IME's action choice.
Replaces the enterkeyhint attribute assertion with a test of what the user actually reported: opening the link popover from the first of two editors, submitting, and expecting the link in *that* editor with focus still there — rather than the link missing and focus in the second one. The attribute assertion stays, but as one line inside that behavioural test rather than as the test itself, because it is the only part of the IME contract a test can hold onto. The device-only half — which action Android assigns to the Enter key — is not reachable from any automated environment we have: emulation always dispatches a real Enter, and on BrowserStack no input channel reaches the on-screen keyboard. That half is now a release-checklist item in the device README instead of being silently uncovered.
Follow-up to the enterkeyhint fix, which had no red-first test: nothing
in an emulated browser can reproduce an IME choosing to advance focus
instead of dispatching Enter, so the only thing a test could assert was
the attribute itself.
The deeper problem is that these popovers had no submission path at all
besides a keydown listener. `Form.Root` rendered a plain `<div>`, so
the `onSubmit` prop the TextInputs already accepted could never fire —
`submit` is dispatched on forms, not inputs. When a platform reports
Enter-to-submit as a form submission rather than a key event, nothing
happened.
`Form.Root` now renders a real `<form>` and takes `onSubmit`, wired up
in all four callers (link, file rename, file caption, AI prompt). It
always preventDefaults, so a caller that doesn't pass a handler can
never navigate away. The dead per-input `onSubmit` props are removed.
This also gives the browser proper form context, which is what it uses
to decide the IME's action key in the first place.
Now testable without any key event: fill the popover, call
`form.requestSubmit()` — exactly what the browser does when the IME
action means submit — and assert the link is created. Proven red with
`Form.Root` back to a `<div>` ("the popover must be a real <form>").
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the Android Chrome keyboard/popover collapse at its root, plus the editing and focus bugs found while chasing it, and adds the mobile test infrastructure that keeps them fixed.
Branched off
mobile-toolbar-demo, and supersedes #2982.#2982 approached the collapse from the symptom end —
autoFocus={false}on the link URL input, plus anonClickCapturehandler that scrolls the input into view and restores the editor scroll a second later, with theonClickCaptureprop plumbed throughComponentsContextand all three UI packages'TextInputs. This PR fixes the root cause instead (see below), which makes all of that unnecessary: it costs autofocus on desktop, and adds public API surface for a workaround nothing needs any more.The one part of #2982 that is not superseded — the
pointer: coarserule that keeps.bn-form-popoverinputs at 16px so iOS Safari doesn't auto-zoom on focus — is cherry-picked here with Matthew's authorship intact, and now has the test coverage it previously lacked. So #2982 can be closed once this lands.Fixes
Mantine popover collapse (the original bug). Mantine's
hideDetached(defaulttrue) reacts to the viewport resize the on-screen keyboard causes by settingdisplay: noneon the still-mounted dropdown. Adisplay: noneancestor can't hold focus, so the popover's input blurred, the toolbar unmounted, and the keyboard dismissed — the whole collapse cascade. DisablinghideDetachedfor portalled (mobile) popovers fixes it at the source.Enter on Android (#3001). prosemirror-view ignores Enter keydowns on Android Chrome and falls back to parsing the browser's native DOM split, which misparses BlockNote's nested block DOM and corrupts the document — Enter inserting a space, doing nothing, or breaking tables. BlockNote now intercepts the
beforeinputinstead and dispatches through the keymap chain, restoring thedomObserver.forceFlush()parity prosemirror's Android bail skips. Verified as not a recent regression: the same race exists in every version pairing back to BlockNote 0.29 / prosemirror-view 1.38.1, bisected with era-correct overrides.Public focus API.
editor.isFocused({ includeFloatingUI })andeditor.onFocusChange(cb, { includeFloatingUI })answer "is the user still interacting with this editor" — content focus or focus inside the editor's own floating UI. Replaces reaching intoeditor._tiptapEditor.on("focus"/"blur"), which can't see focus moving into a portalled popover. Events are settled (a rAF re-check, becausedocument.activeElementis transiently<body>mid-handoff andrelatedTargetis unreliable on mobile). The React callback hooks move to the latest-ref pattern, so unmemoized callbacks no longer resubscribe on every render.iOS input auto-zoom (cherry-picked from #2982). Safari zooms the page when an input with a computed font-size below 16px takes focus, and that zoom perturbs the visual viewport the mobile toolbar positions itself from.
Selected link URL. Reading the URL from a single boundary position is fragile twice over:
marks()excludes a link at its left edge, and engines disagree by one position on where a selection over a link starts. Now scans the selected range for the first link mark, which fixes the link popover opening empty for a fully selected link on WebKit.Test infrastructure
Android-emulated browser instance. A fourth e2e instance (Android UA, touch, phone viewport) so
isTouchDevice()is genuinely true and prosemirror takes its Android code paths — no stubs. It runs the newend-to-end/mobile/suites plus the behavioral suites where Android genuinely differs. Note the per-instancecontextOptionsare silently ignored by vitest; the instance needs its ownprovider: playwright({ contextOptions }).Real-device suite on BrowserStack (
tests/device/,pnpm run test:device). A dependency-free WebDriver REST client, a gesture layer holding every platform quirk, and suites for the toolbar lifecycle and basic editing including soft-keyboard Enter. Self-skips without credentials, so it's safe to invoke anywhere. Nightly workflow included (needsBROWSERSTACK_*secrets).Verification
Every fix has an e2e test proven to fail without it — each was verified by reverting the fix and confirming the test reports the original symptom. Android instance: 45 passed, 4 expected skips. Also verified on real devices (Samsung Galaxy S22, iPhone 16e) for the Enter and popover fixes.
The iOS auto-zoom fix is covered at two levels, since the rule and the behaviour it prevents are testable in different places: the emulated suite asserts the input's computed font-size stays at/above 16px (the CSS contract), and the real-device suite asserts
visualViewport.scaledoesn't increase when the URL input takes focus (the actual behaviour).Draft: a follow-up branch stacks ~27 more commits on top of this one (comments/toolbar handoff, IME composition coverage, visual baselines, accessibility scanning, a tablet device target).