Prevent history commands when view is not editable #10

Merged
guarmo merged 1 commit from fix-history-editable-check into master 2024-10-24 12:21:05 +02:00
guarmo commented 2024-10-24 10:44:32 +02:00 (Migrated from github.com)

Description

This pull request fixes an issue where history commands (undo/redo) could still be triggered even when the editor view is not editable. The change ensures that the beforeinput event handler properly checks the view.editable flag before executing history commands.

Why this is needed:

The current behavior allows undo/redo operations even when the editor is not in an editable state, which is inconsistent with expected behavior. This fix resolves that issue by ensuring that history commands are only available in editable mode.

## Description This pull request fixes an issue where history commands (undo/redo) could still be triggered even when the editor view is not editable. The change ensures that the `beforeinput` event handler properly checks the `view.editable` flag before executing history commands. ### Why this is needed: The current behavior allows undo/redo operations even when the editor is not in an editable state, which is inconsistent with expected behavior. This fix resolves that issue by ensuring that history commands are only available in editable mode.
marijnh commented 2024-10-24 11:06:55 +02:00 (Migrated from github.com)

None of the other commands make such a check. EditorView.editable indicates whether the DOM has been marked as editable, but is not intended to be a flag that commands consult to determine whether the user may change the content.

How are the commands being called, in this case? Through a keymap? Or by some other means?

None of the other commands make such a check. `EditorView.editable` indicates whether the DOM has been marked as editable, but is not intended to be a flag that commands consult to determine whether the user may change the content. How are the commands being called, in this case? Through a keymap? Or by some other means?
guarmo commented 2024-10-24 12:14:54 +02:00 (Migrated from github.com)

How are the commands being called, in this case? Through a keymap? Or by some other means?

The prosemirror-history plugin is binding a defaultbeforeinput handler which is called even when there is no keymap. Our preference is that when the editor is not editable, these commands do not execute. But, since the plugin binds this event by default, we either have to resort to capturing these events ourselves, or have the plugin not call the command in the first place.

We wrote a plugin that captures the event, and while it does produce our desired behavior, it feels hacky to include.

  addProseMirrorPlugins() { 
    return [ 
      new Plugin({
        key: new PluginKey('uneditableHistory'),
        props: {
          handleDOMEvents: {
            beforeinput: (view, e: Event) => {
              let inputType = (e as InputEvent).inputType
              if (inputType == 'historyUndo' || inputType == 'historyRedo') {
                if (!this.editor.isEditable) {
                  e.preventDefault()
                }
              }
            }
          }
        }
      })
    ]
  }

None of the other commands make such a check.

The desire here is not that the command consults with the EditorView.editable property, but that the plugin does since it handles these events when keymaps do not apply.

> How are the commands being called, in this case? Through a keymap? Or by some other means? The prosemirror-history plugin is binding a default`beforeinput` handler which is called even when there is no keymap. Our preference is that when the editor is not editable, these commands do not execute. But, since the plugin binds this event by default, we either have to resort to capturing these events ourselves, or have the plugin not call the command in the first place. We wrote a plugin that captures the event, and while it does produce our desired behavior, it feels hacky to include. ```ts addProseMirrorPlugins() { return [ new Plugin({ key: new PluginKey('uneditableHistory'), props: { handleDOMEvents: { beforeinput: (view, e: Event) => { let inputType = (e as InputEvent).inputType if (inputType == 'historyUndo' || inputType == 'historyRedo') { if (!this.editor.isEditable) { e.preventDefault() } } } } } }) ] } ``` > None of the other commands make such a check. The desire here is not that the command consults with the `EditorView.editable` property, but that the plugin does since it handles these events when keymaps do not apply.
marijnh commented 2024-10-24 12:19:31 +02:00 (Migrated from github.com)

Oh, I see, you're modifying the beforeinput handler, not the commands. That does seem reasonable!

Oh, I see, you're modifying the beforeinput handler, not the commands. That does seem reasonable!
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
prosemirror/prosemirror-history!10
No description provided.