Refactor useShowMoversGestures hook
#52792
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
ntsekouras
added
[Type] Bug
|
Size Change: -164 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 6cf2003. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5610610622
|
mcsf
approved these changes
Jul 31, 2023
ntsekouras
force-pushed
the
refactor/show-movers-hook
branch
from
4fb0ea8
to
a944784
Compare
ntsekouras
added
[Type] Bug
|
Sorry for the late reply; the refactoring and the fix look good to me. Maybe a different name would be more appropriate for this hook. Unless I misunderstand the meaning of "gestures" this hook has nothing to do with them. I usually associate them with special handlers for touch devices. I can only think of the |
Yeah, I had some trouble choosing a name.. I get your point but still the hook is more about hovering/focus, despite of the fact that now it highlights a block. With something like |
Sounds good to me 👍 |
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?
This PR fixed a stale clientId issue in
useShowMoversGesturescallback. After that, @Mamaduka spotted a similar issue for a follow up. I had trouble understanding whatuseShowMoversGesturesis and after investigating I think the name should be about handling when a node is focused or hovered - thus I renamed the hook touseShowHoveredOrFocusedGestures.The original hook's name would have to do with the fact that
moversweren't in the toolbar(as are for a while now) and we were showing when the blocktype and parent blocktype icons were focused or hovered. Since the only thing we do for them is highlight a block, I removed the callback and baked thetoggleBlockHighlightaction inside the internal hook, which is not publicly exported.Testing Instructions
select parentin block settings dropdownCleanShot.2023-07-19.at.14.22.13.mp4