Drag and drop: fix firefox compat logic #67439
Merged
+66
−10
Conversation
This file contains 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
ellatrix
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +199 B (+0.01%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
ellatrix
commented
Nov 29, 2024
| @@ -125,7 +123,7 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) { | |||
| isEnabled: isSectionBlock, | |||
| } ), | |||
| useScrollIntoView( { isSelected } ), | |||
| canDrag ? ffDragRef : undefined, | |||
| canMove ? ffDragRef : undefined, | |||
There was a problem hiding this comment.
This fixes the issue where you can't drag a quote block after selecting a paragraph inside it in trunk.
ellatrix
commented
Dec 2, 2024
| @@ -13,13 +75,9 @@ import { useRefEffect } from '@wordpress/compose'; | |||
| */ | |||
| export function useFirefoxDraggableCompatibility() { | |||
| return useRefEffect( ( node ) => { | |||
| function onDown( event ) { | |||
| node.draggable = ! event.target.isContentEditable; | |||
There was a problem hiding this comment.
Notice that we're currently setting all blocks to be draggable whenever a non editable element is clicked! Also, we're constantly setting draggable true/false for all blocks on click.
ellatrix
commented
Dec 2, 2024
| node.draggable = ! event.target.isContentEditable; | ||
| } | ||
| const { ownerDocument } = node; | ||
| ownerDocument.addEventListener( 'pointerdown', onDown ); |
There was a problem hiding this comment.
I've also opted to avoid adding an event listener for every single block and maintain a set instead.
youknowriad
approved these changes
Dec 5, 2024
|
Thanks @youknowriad! |
michalczaplinski
pushed a commit
that referenced
this pull request
Dec 5, 2024
Co-authored-by: ellatrix <[email protected]> Co-authored-by: youknowriad <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What? Why? How?
While trying
cursor:grabfor draggable blocks, I noticed thatdraggableis currently being set to true for a lot of block by accident that shouldn't be draggable after you click on them. That's because of a mistake in the Firefox compatibility logic. It doesn't check if the block is draggable first. We'll also have to set a temporary attribute to store the previous state so we only restore it totruewhen it was previously draggable.Also fixes an edge case where you can't drag the block you select an inner block.
Also improves the performance of this hook by only adding a single event handler.
Testing Instructions
Use Firefox. Make sure we the compatibility logic still works; it should still be possible to select within editable fields within a block such as the quote block.
Also click in a paragraph for example and out of it. The paragraph should not be left with a
draggableattribute.Now also click a paragraph in a quote and then try to drag the quote. In trunk this doesn't work. With this PR it does.
Testing Instructions for Keyboard
Screenshots or screencast