Skip to content

feat: add disable hotkeys feature#1380

Open
do4Mother wants to merge 7 commits intopuckeditor:mainfrom
do4Mother:main
Open

feat: add disable hotkeys feature#1380
do4Mother wants to merge 7 commits intopuckeditor:mainfrom
do4Mother:main

Conversation

@do4Mother
Copy link
Copy Markdown

when using a wysiwyg editor to the puck, using the CTRL+Z or CTRL+R keys will trigger undo or redo simultaneously.

@vercel
Copy link
Copy Markdown

vercel bot commented Oct 16, 2025

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

A member of the Team first needs to authorize it.

@FedericoBonel
Copy link
Copy Markdown
Collaborator

Hello @do4Mother!

Thank you very much for your contribution 🙏

For the record, you can actually avoid triggering Puck hotkeys by using an onKeyDown event listener on your RTE or WYSIWYG component and stopping propagation. We’re currently implementing a Puck RTE, and we needed to do this as well to prevent Puck from switching into interactive mode when pressing cmd + i.

You can check out the implementation here:
90f3cf5#diff-ae9dac1ee1543faeeb141edd12591423d6ecfa47b4cf79c5875e825f475ed95cR1-R108

In that case, we stop propagation in the capturing phase (onKeyDownCapture), but you might be better off stopping it in the bubbling phase (onKeyDown).

We normally prefer having a feature request or bug report before opening PRs. This helps us understand the need and discuss the best approach before implementation.

That said, having a set of props to control Puck hotkeys would definitely be beneficial, so I have a small request to better align it with what we’d expect to support:

Could you update the API to work as a hotkeys prop on the Puck component that receives an object, with an enabled property inside it? Something like this:

<Puck
  hotkeys={{ enabled: false }}
/>

That would allow us to add more controls in the future.

Thank you again for taking the time to contribute!

@FedericoBonel FedericoBonel self-requested a review October 16, 2025 11:15
Copy link
Copy Markdown
Collaborator

@FedericoBonel FedericoBonel left a comment

Choose a reason for hiding this comment

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

Check above!

@do4Mother
Copy link
Copy Markdown
Author

Sorry, I was so excited to contribute to this awesome project that I didn't read the documentation on how to contribute.

Additionally, in the use case project I'm working on, 80% of the work is done using the WYSIWYG editor, so I don't disable hotkeys in the editor.

And I have been updated the state. please help to check it. thanks!

};

export type AppState<UserData extends Data = Data> = {
hotkeys: HotkeyState;
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.

Can you move this to the AppStore rather than AppState?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

okay! i have done a refactor update to the appstore

onAction,
metadata,
fieldTransforms: loadedFieldTransforms,
hotkeys: hotkeys,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
hotkeys: hotkeys,
hotkeys,

onAction,
metadata,
loadedFieldTransforms,
hotkeys
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
hotkeys
hotkeys.enabled

I don't know if we want to subscribe to the whole object here (which could be re-created through re renders), it might be better to start off with hotkeys.enabled, just in case.

};
initialHistory?: InitialHistory;
metadata?: Metadata;
hotkeys?: HotkeyState
Copy link
Copy Markdown
Collaborator

@FedericoBonel FedericoBonel Nov 18, 2025

Choose a reason for hiding this comment

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

I think we need to use a Partial here and handle that optionality (applying defaults) before passing it down to the appStore.

We might introduce more options in the future, and requiring enabled could become inconvenient, some users might only want to enable a single option.

Copy link
Copy Markdown
Collaborator

@FedericoBonel FedericoBonel left a comment

Choose a reason for hiding this comment

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

Thanks again for your contribution @do4Mother!

Awesome stuff, left couple of comments above.

@do4Mother
Copy link
Copy Markdown
Author

@FedericoBonel Thanks for suggestions and build cool project!

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.

3 participants