Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undo breakage with Spacer block and Row/Stack #68863

Open
3 of 6 tasks
stokesman opened this issue Jan 24, 2025 · 2 comments · May be fixed by #68869
Open
3 of 6 tasks

Undo breakage with Spacer block and Row/Stack #68863

stokesman opened this issue Jan 24, 2025 · 2 comments · May be fixed by #68869
Assignees
Labels
[Block] Spacer Affects the Spacer Block [Feature] History History, undo, redo, revisions, autosave. [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@stokesman
Copy link
Contributor

Description

Inserting or moving a Spacer block into a Row/Stack block stops undo from working. Moving a Spacer block out of a Row/Stack has the same effect. I expect undo to keep working.

Step-by-step reproduction instructions

  1. Insert a Row or Stack block.
  2. Insert a Spacer into the previously inserted Row or Stack.
  3. Try to undo.

Screenshots, screen recording, code snippet

spacer-v-row-stack-history-breakage.mp4

Environment info

WP 6.7.1 / Gutenberg plugin trunk
Chrome 130
macOS 13.7.1

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes

Please confirm which theme type you used for testing.

  • Block
  • Classic
  • Hybrid (e.g. classic with theme.json)
  • Not sure
@stokesman stokesman added [Block] Spacer Affects the Spacer Block [Feature] History History, undo, redo, revisions, autosave. [Type] Bug An existing feature does not function as intended labels Jan 24, 2025
@stokesman
Copy link
Contributor Author

It looks like this is caused by an effect in the Spacer block that sets some attributes.

Short-circuit the effect in question.

diff --git a/packages/block-library/src/spacer/edit.js b/packages/block-library/src/spacer/edit.js
index 1e0ffb9700..cf8fd7d144 100644
--- a/packages/block-library/src/spacer/edit.js
+++ b/packages/block-library/src/spacer/edit.js
@@ -256,6 +256,7 @@ const SpacerEdit = ( {
 	};
 
 	useEffect( () => {
+		return;
 		if (
 			isFlexLayout &&
 			selfStretch !== 'fill' &&

I suppose it may be fixed by some application of __unstableMarkNextChangeAsNotPersistent.

@yogeshbhutkar
Copy link
Contributor

I believe these direct setAttribute calls are causing this issue, and using __unstableMarkNextChangeAsNotPersistent() before calling setAttribute should fix the issue.

I'll try creating a PR to incorporate this fix.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Spacer Affects the Spacer Block [Feature] History History, undo, redo, revisions, autosave. [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants