fix: Image block selection clears on mouse leave in Safari#2613
fix: Image block selection clears on mouse leave in Safari#2613matthewlipski merged 4 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds file-type metadata to Image/Audio/Video React blocks and refactors resize-handle behavior by keeping handles in the DOM and toggling visibility via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
🧹 Nitpick comments (1)
packages/react/src/schema/ReactBlockSpec.tsx (1)
320-347:isFileBlockprop missing in non-nodeView render path.The
nodeViewrender path (line 296) passesisFileBlock={!!blockImplementation.meta?.fileBlockAccept}, but the non-nodeView render path in this else block does not passisFileBlocktoBlockContentWrapper. This inconsistency means file blocks rendered via the non-nodeView path won't have thedata-file-blockattribute set.If this path is exercised for file blocks, the CSS selectors targeting
[data-file-block]won't apply correctly.Proposed fix
<BlockContentWrapper blockType={block.type} blockProps={block.props} propSchema={blockConfig.propSchema} domAttributes={this.blockContentDOMAttributes} + isFileBlock={!!blockImplementation.meta?.fileBlockAccept} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/schema/ReactBlockSpec.tsx` around lines 320 - 347, The non-nodeView render branch in ReactBlockSpec (the else that builds BlockContent via renderToDOMSpec) omits the isFileBlock prop on BlockContentWrapper; update that BlockContentWrapper invocation to include isFileBlock={!!blockImplementation.meta?.fileBlockAccept} (same value used in the nodeView path where isFileBlock is already set) so the data-file-block attribute gets applied for file blocks rendered by the non-nodeView path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react/src/schema/ReactBlockSpec.tsx`:
- Around line 320-347: The non-nodeView render branch in ReactBlockSpec (the
else that builds BlockContent via renderToDOMSpec) omits the isFileBlock prop on
BlockContentWrapper; update that BlockContentWrapper invocation to include
isFileBlock={!!blockImplementation.meta?.fileBlockAccept} (same value used in
the nodeView path where isFileBlock is already set) so the data-file-block
attribute gets applied for file blocks rendered by the non-nodeView path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7061287-6828-470d-8e94-1df12a174f3a
📒 Files selected for processing (6)
packages/core/src/blocks/File/helpers/render/createResizableFileBlockWrapper.tspackages/react/src/blocks/Audio/block.tsxpackages/react/src/blocks/File/helpers/render/ResizableFileBlockWrapper.tsxpackages/react/src/blocks/Image/block.tsxpackages/react/src/blocks/Video/block.tsxpackages/react/src/schema/ReactBlockSpec.tsx
nperez0111
left a comment
nperez0111
left a comment
There was a problem hiding this comment.
Good, this is simpler
@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: |
2 participants
Summary
This PR fixes an issue specific to Safari where when a image/video/audio block is selected and be mouse moves off of it, the selection collapses to just before the block.
The surface level cause of this is the resize handles, which are added to & removed from the DOM when the mouse cursor enters & leaves the image/video/audio element. The DOM mutation triggers a selection change event in Safari only.
This is apparently a known issue in Safari, i.e. that DOM mutations within an active selection trigger selection updates, which does not happen in other browsers. This comment supports that reasoning, though after searching for this bug/issue in WebKit online, I couldn't find anything referring to this exact problem.
Either way, this PR fixes the issue by making the resize handles appear and disappear using CSS and be permanently attached to the DOM.
Closes #2541
Rationale
This is an issue especially because it makes the formatting toolbar inaccesssible using the mouse for media blocks.
Changes
See above.
Impact
N/A
Testing
Slight problem here - I tried adding an e2e test to ensure that moving the mouse off of an image block while it's selected keeps it selected. However, I couldn't reproduce the issue using Playwright mouse handling, so need to discuss this.
Screenshots/Video
N/A
Checklist
Additional Notes
Summary by CodeRabbit
New Features
Improvements
Tests