Focal Point Picker: Set grid overlay state when dragging

[Mamaduka]

What?

PR updates the FocalPointPicker to set the showGridOverlay state via onDragStart and onDragEnd callbacks.

Why?

The component has clear events that trigger point updates; we can set related states during these events and avoid effect synchronization.

Testing Instructions

1. Open a post or page.
2. Insert a Cover block and set an image.
3. Try updating the image focal point.
4. The grid overlay should be displayed as before.

Testing Instructions for Keyboard

Same.

Screenshots or screencast

CleanShot.2024-07-03.at.15.54.40.mp4

[Mamaduka]

I've omitted this logic from the update. I'm happy to restore it, but I've not noticed a big difference.

[stokesman]

In general I'm a fan of avoiding effects. The difference this makes is that the grid overlay is not shown when using the inputs to change the focal point.

Possibly there’s a way to rework how the Grid visibility changes so that it show/hides without using state and avoid the effect but I'm not sure.

I don’t have much of an opinion on whether it’s okay to omit showing the grid for changes from the inputs.

[github-actions]

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: stokesman <presstoke@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[ciampo]

As @stokesman also mentioned, the grid overlay currently shows any time the value of the focal point changes — that can happen in a few ways:

• user interactions:
  • user clicking on the canvas
  • user dragging the "dot"
  • user changing x / y values in the number inputs
• any other external controlled updates by changing the value prop

Currently, in this PR the grid shows only when dragging the dot. Personally, I feel like we should continue to show the grid overlay on all user interactions (including clicks and number input interactions).

Also, out of curiosity — is there a particular reason why we need to rework this logic? As much as I don't like even synchronisation, the current implementation seemed to work well.

[Mamaduka]

I totally missed that grid was showing on any other interaction 😅

Got idea of the idea of this refactoring after I fixed a bug in this area.

I agree that with described terms effect sync makes sense.

Thanks for the feedback 🙇