Skip to content

fix: prevent unintended history entry#1533

Merged
chrisvxd merged 7 commits intopuckeditor:mainfrom
DamianKocjan:fix-unintended-history-entry
Mar 30, 2026
Merged

fix: prevent unintended history entry#1533
chrisvxd merged 7 commits intopuckeditor:mainfrom
DamianKocjan:fix-unintended-history-entry

Conversation

@DamianKocjan
Copy link
Copy Markdown
Contributor

Closes #1521

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 22, 2026

@DamianKocjan is attempting to deploy a commit to the Puck Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
puck-demo Ready Ready Preview, Comment Feb 9, 2026 2:59am
puck-docs Ready Ready Preview Feb 9, 2026 2:59am

Request Review

Comment thread packages/core/components/Puck/components/Canvas/index.tsx Outdated
Comment thread packages/core/components/Puck/components/Canvas/index.tsx Outdated
Avoid creating history entries when the field content hasn't actually changed. This occurs when rapidly clicking a RichText field and deselecting block, which would previously record a history entry despite no visual changes to the document.

The fix compares the current field value with the debounced content and skips the `onChange` callback if they match, preventing unnecessary history pollution.
Copy link
Copy Markdown
Contributor

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works great now, just 1 comment on unrelated code and a conflict to address

Comment on lines +95 to +101
const currentData = appStoreApi.getState().getCurrentData();
const currentFieldValue = getDeep(currentData.props, name ?? "");

// Avoid echoing back same content
if (currentFieldValue === debouncedState.html) {
return;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like its leaked into this PR. If this is intentional, can you capture as a separate PR with relevant issue?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was intentional. When I was testing against unintentional history entries I found that issue, so I fixed it. Thought it'd be ideal to merge it here with rest of related changes.

Commit addressing this issue with explanation: fc58153

I'll create issue and PR later that day. Or we can add one more issue that this PR is addressing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if this is fixed in #1587

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about it too. So I'll probably undo this commit from this pr and see if issue still persist after fix in #1587

Copy link
Copy Markdown
Contributor

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops approved prematurely! Needs conflicts addressed :)

@DamianKocjan
Copy link
Copy Markdown
Contributor Author

@chrisvxd You can merge. Tested once again to be sure

@chrisvxd chrisvxd merged commit 39a4e15 into puckeditor:main Mar 30, 2026
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unintended history entry

3 participants