Fix: Space visualizer showing previous value

[karthikeya-io]

What?

Part of #63191

Why?

Fixes the issue that Margin and Padding visualizer shows the previous value. This issue doesn't occur in Safari but occurs in Chromium based browsers.

Related PR #59227

How?

Use MutationObserver to observe the changes in block.

Testing Instructions

1. Open a post or page.
2. Insert a block and tweak the margin or padding styles.
3. The Space visualizer should appear accurately.

Testing Instructions for Keyboard

Same

Screenshots or screencast

Before	After
space-visualizer-before.mov	space-visualizer-after.mov

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @karthikeya-io! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[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: karthikeya-io <[email protected]>
Co-authored-by: stokesman <[email protected]>

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

[stokesman]

This is great! Thanks for working on it. I’ve experimented with some alternatives and it does seem like leveraging a MutationObserver is the best bet.

I’ve tested this and didn’t find any problems. I think it might not be necessary to observe the class attribute and just the style may suffice but I don’t think it’s likely to be critical for performance and may be helpful in some less common cases.

I’d be okay with this landing as is but I have a few suggestions.

[stokesman]

The value no longer needs to be a dependency. Omitting it will prevent excessive turnover of mutation observer instances. Along with that this doesn’t need to be a layout effect. I can’t easily leave inline suggestions for what I’d like to so here’s a diff:

diff --git a/packages/block-editor/src/hooks/spacing-visualizer.js b/packages/block-editor/src/hooks/spacing-visualizer.js
index 6593a34fe8..0a058a70be 100644
--- a/packages/block-editor/src/hooks/spacing-visualizer.js
+++ b/packages/block-editor/src/hooks/spacing-visualizer.js
@@ -1,13 +1,7 @@
 /**
  * WordPress dependencies
  */
-import {
-	useState,
-	useRef,
-	useLayoutEffect,
-	useEffect,
-	useReducer,
-} from '@wordpress/element';
+import { useState, useRef, useEffect, useReducer } from '@wordpress/element';
 import isShallowEqual from '@wordpress/is-shallow-equal';
 
 /**
@@ -22,13 +16,13 @@ function SpacingVisualizer( { clientId, value, computeStyle, forceShow } ) {
 		computeStyle( blockElement )
 	);
 
-	useLayoutEffect( () => {
+	// It's not sufficient to read the block’s computed style when `value` changes because
+	// the effect would run before the block’s style has updated. Thus observing mutations
+	// to the block’s attributes is used to trigger updates to the visualizer’s styles.
+	useEffect( () => {
 		if ( ! blockElement ) {
 			return;
 		}
-		// It's not sufficient to read the computed spacing value when value.spacing changes as
-		// useEffect may run before the browser recomputes CSS. So we use a MutationObserver to track style/class changes.
-		// Fixes issue where visualizer applies previous margin/padding due to delayed style updates in Chromium based browsers.
 		const observer = new window.MutationObserver( updateStyle );
 		observer.observe( blockElement, {
 			attributes: true,
@@ -37,7 +31,7 @@ function SpacingVisualizer( { clientId, value, computeStyle, forceShow } ) {
 		return () => {
 			observer.disconnect();
 		};
-	}, [ blockElement, value ] );
+	}, [ blockElement ] );
 
 	const previousValueRef = useRef( value );
 	const [ isActive, setIsActive ] = useState( false );

[karthikeya-io]

Thanks for reviewing the PR. I have implemented the suggestions and updated the PR, please take a look.