Fix selection clearer ref passed to motion.div #31469
Merged
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
|
Size Change: +6 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
ellatrix
commented
May 4, 2021
| @@ -43,7 +39,7 @@ export function useBlockSelectionClearer( onlySelfClicks = false ) { | |||
| node.removeEventListener( 'mousedown', onMouseDown ); | |||
| }; | |||
| }, | |||
| [ hasSelection, clearSelectedBlock ] | |||
| [ hasSelectedBlock, hasMultiSelection, clearSelectedBlock ] | |||
There was a problem hiding this comment.
Of course a change in these dependencies won't do anything, but they should remain content anyway. Not sure if we should pass it through useMergeRefs after all.
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.
Description
Clicking on the padding around the template editor currently doesn't clear the selected block.
I tracked this down to
motion.divnot calling a ref callback when it changes, as React normally does.When the clearer was added to the padding (#30658),
motionwasn't introduced yet.https://github.com/framer/motion/blob/f8d08b271ae9b021521d0b80cdf1b3f787ccde13/src/motion/utils/use-motion-ref.ts#L16-L34
externalRefis not passed as a dependency here, so ouruseRefEffectwon't work properly when its dependencies change (hasSelection).There's two solutions: wrap the ref in
useMergeRefs, even though there's only one, but this hook will handle dependencies correctly, or call the selectors within the even callback, which we anyway want to move towards, so I opted for that.Of course, it would be good if this gets fixed in https://github.com/framer/motion, so maybe we should submit a PR there.
How has this been tested?
Edit a page and open the template editor. Select a block, then click on the dark frame. The selection should be cleared.
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).