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

fix: update Puck after clearing local changes and fix undo state #18

Merged
merged 5 commits into from
Aug 15, 2024

Conversation

ehallerYext
Copy link
Contributor

@ehallerYext ehallerYext commented Aug 9, 2024

SUMO-6143

This fixes the Clear Local Changes button to:

  1. Close the modal on confirmation
  2. Reset the Puck data state without a page refresh

Additionally, this fixes the undo button to properly undo to the original state before there were any changes instead of getting stuck at the first change.

@mkilpatrick
Copy link
Collaborator

SUMO-6143

This change is implemented to update the header after the user confirms that they would like to clear their local changes.

Tested by packing VE and using the locally packed version when running the VE starter in dev mode.

The second part of this item is to refresh the header after the user publishes their changes. It seems this may require changes to Puck. Per the Puck Docs, the initial data passed to the component cannot be updated without remounting the component. Doing so causes some glitchy UI and is not a great experience. Another potential solution for this is to set a "initial history index" to mark as the new beginning of history after changes have been published. The issue with this is that every time a change is made in the puck editor, the header is being remounted, clearing the "initial history index" state. Would love to hear any other ideas of how we can get around this, but thought I would leave this here in case I can't get to it before I leave.

Setting initialHistory won't work as that only gets used on Puck mount. It might work by setting the puck history. If that doesn't seem to work then we'll need to have the page refresh.

@mkilpatrick mkilpatrick changed the title fix: update header after clearing local changes fix: update Puck after clearing local changes and fix undo state Aug 15, 2024
Copy link
Contributor

@jwartofsky-yext jwartofsky-yext left a comment

Choose a reason for hiding this comment

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

lgtm assuming all is tested and working as expected

@mkilpatrick mkilpatrick merged commit 3b33631 into main Aug 15, 2024
9 of 10 checks passed
@mkilpatrick mkilpatrick deleted the ethantest branch August 15, 2024 21:39
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.

6 participants