fix: prevent FloatingFocusManager from resetting editor selection (#2525)#2664
Open
fix: prevent FloatingFocusManager from resetting editor selection (#2525)#2664
Conversation
) When FloatingFocusManager is used inside a FloatingPortal, floating-ui inserts a hidden <span> after the domReference via insertAdjacentElement. If the reference element is inside the ProseMirror contenteditable, this triggers PM's MutationObserver and resets the editor selection. Skip setting domReference when FloatingFocusManager is active and the reference is within the editor DOM. Positioning still works via the separate setPositionReference call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated GenericPopover to only set the floating reference when a real reference element is present and either FloatingFocusManager is disabled or the element is outside the editor. Added an end-to-end Playwright test verifying ProseMirror selection persists after clicking the AI toolbar button. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page
participant Editor
participant ToolbarButton as AI_Button
participant AI_Menu
User->>Page: focus editor (keyboard)
Page->>Editor: set selection (Home, Shift+End)
Page->>Page: capture selection via page.evaluate
User->>AI_Button: click (mouse at bounding-box center)
AI_Button->>Page: open AI menu
Page->>AI_Menu: wait for input visible
Page->>Editor: read selection via page.evaluate
Editor-->>Page: return selection (from/to)
Page-->>User: assert selection unchanged
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/src/end-to-end/ai/ai-selection.test.ts (1)
42-45: Align the regression note with this PR’s fix.The assertion is right, but the comment points to a
preventDefaulttoolbar fix instead of theFloatingFocusManager/refs.setReferencebehavior this PR changes.📝 Proposed comment update
- // Without the preventDefault fix on toolbar buttons, the browser - // moves focus to the portaled button on mousedown, which blurs the - // editor and clears the selection. + // Without skipping refs.setReference for editor references while + // FloatingFocusManager is active, floating-ui inserts focus-return + // elements near the ProseMirror reference and PM resets the selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/src/end-to-end/ai/ai-selection.test.ts` around lines 42 - 45, Update the regression comment to reference the actual fix introduced in this PR: replace mention of the toolbar buttons' preventDefault behavior with the FloatingFocusManager/refs.setReference change; specifically, state that without the FloatingFocusManager and proper refs.setReference handling the browser moves focus on mousedown to the portaled button which blurs the editor and clears the selection, and keep the assertion text unchanged in ai-selection.test.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react/src/components/Popovers/GenericPopover.tsx`:
- Around line 158-171: The conditional that decides whether to call
refs.setReference currently treats props.focusManagerProps?.disabled ===
undefined as "disabled" (because it checks !== false) and thus still sets the
DOM reference; change the guard to only set the reference when the focus manager
is explicitly disabled (props.focusManagerProps?.disabled === true) or when the
reference element is outside the editor (use (props.focusManagerProps?.disabled
=== true || !editor.isWithinEditor(element)) in the refs.setReference branch),
and apply the same explicit-true logic in the other related block that mirrors
this behavior around FloatingFocusManager rendering.
In `@tests/src/end-to-end/ai/ai-selection.test.ts`:
- Line 6: The test's AI_BUTTON_SELECTOR (`AI_BUTTON_SELECTOR =
[data-test="editwithAI"]`) doesn't match the actual rendered AIToolbarButton,
causing failures; either add the missing data-test="editwithAI" attribute to the
AIToolbarButton component (where it's defined) or update `AI_BUTTON_SELECTOR` to
use the selector that the component actually renders (e.g., the component's real
data-test value, role, aria-label, or text content); make the same change for
the other occurrences referenced around lines 32-35 so all selectors in this
file target the rendered element.
---
Nitpick comments:
In `@tests/src/end-to-end/ai/ai-selection.test.ts`:
- Around line 42-45: Update the regression comment to reference the actual fix
introduced in this PR: replace mention of the toolbar buttons' preventDefault
behavior with the FloatingFocusManager/refs.setReference change; specifically,
state that without the FloatingFocusManager and proper refs.setReference
handling the browser moves focus on mousedown to the portaled button which blurs
the editor and clears the selection, and keep the assertion text unchanged in
ai-selection.test.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e528995-4170-4d77-886e-8d3451ddbe6b
📒 Files selected for processing (2)
packages/react/src/components/Popovers/GenericPopover.tsxtests/src/end-to-end/ai/ai-selection.test.ts
| import { AI_URL } from "../../utils/const.js"; | ||
| import { focusOnEditor } from "../../utils/editor.js"; | ||
|
|
||
| const AI_BUTTON_SELECTOR = `[data-test="editwithAI"]`; |
There was a problem hiding this comment.
Make the AI button selector match the rendered component.
AIToolbarButton does not expose data-test="editwithAI" in the provided component snippet, so this test can fail before reaching the regression assertion. Add the attribute to the button component or switch the test to a selector that exists.
Also applies to: 32-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/src/end-to-end/ai/ai-selection.test.ts` at line 6, The test's
AI_BUTTON_SELECTOR (`AI_BUTTON_SELECTOR = [data-test="editwithAI"]`) doesn't
match the actual rendered AIToolbarButton, causing failures; either add the
missing data-test="editwithAI" attribute to the AIToolbarButton component (where
it's defined) or update `AI_BUTTON_SELECTOR` to use the selector that the
component actually renders (e.g., the component's real data-test value, role,
aria-label, or text content); make the same change for the other occurrences
referenced around lines 32-35 so all selectors in this file target the rendered
element.
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@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: |
…e in GenericPopover
…ing-ui focus-return elements
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
1 participant
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.
Summary
Fixes a regression where AI operations apply to the whole page instead of the user's selection, caused by FloatingFocusManager resetting the ProseMirror editor selection when the AI menu opens.
Rationale
After PR #2591 (portaling all floating UI elements to
document.body), floating-ui'sFloatingFocusManagerbegan inserting a hidden focus-return<span>element inside the ProseMirror contenteditable DOM viainsertAdjacentElement('afterend', ...)on the reference element. This triggers PM's MutationObserver, which resets the editor selection — causing the AI menu to see no selection and show whole-page operations instead of selection-specific ones.Changes
packages/react/src/components/Popovers/GenericPopover.tsx: Skip callingrefs.setReference(element)whenFloatingFocusManageris active (disabled !== false) and the reference element is inside the editor DOM (editor.isWithinEditor(element)). Position is still handled correctly via the separaterefs.setPositionReference()call.tests/src/end-to-end/ai/ai-selection.test.ts: New Playwright e2e test that selects text, clicks the AI toolbar button with real mouse coordinates, and verifies PM selection is preserved after the AI menu opens.Impact
FloatingFocusManagerwith a reference element inside the editor (currently: AI menu, floating comment composer).focusManagerProps.disabled: true(formatting toolbar, suggestion menu, side menu, etc.) are unaffected.setPositionReferenceis always called.Testing
Checklist
Additional Notes
Root cause:
floating-ui/react@0.27.19—FloatingFocusManagerline 2057-2058 inserts a fallback element whenisInsidePortal && domReference. By not settingdomReferencefor in-editor references, we prevent the insertion entirely.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests