Implement ignoreMutation for Widget Decorations

[jljorgenson18]

We have a widget that can take a selection and the recent change to not ignore selections in the base ViewDesc class has caused a few issues. We have a terrible hack to fix our problem, but I figured I would create this PR because it is an easy extension and this is already being done in CustomNodeViewDesc. If other prosemirror users have a widget that they want to select, this would allow users to ignore selection changes. This would also help make Widgets more future proof if the ViewDesc class continues to change.

[marijnh]

I don't think DOM mutations inside widgets are ever interesting, so exposing a way for client code to implement this method seems like unjustified complexity.

Could you describe the issues the change caused more precisely, so that we can maybe find a default implementation for WidgetViewDesc.ignoreMutation that solves them?

[jljorgenson18]

Really this is due to selections as opposed to actual mutations so it would make more sense to just call it "ignore selections" but to me, it made sense to keep the API consistent. We have a widget where we need to be able to set the selection and that currently isn't being ignored by the base ViewDesc. We could add a flag called "allowSelectionChanges" or something along those lines to revert widgets to what they were before the ViewDesc ignoreMutation change.

Does that answer your question?

[marijnh]

What happens when you set a selection inside the widget? (And does this selectable thing take focus, or is focus still in the outer editor when the user selects in the widget?)

[jljorgenson18]

Are you asking what happened before the ignoreMutation change was introduced or what happens now that ViewDesc does not ignore selection changes? The focus is still on the editor and the widget does not take focus, it only takes a selection.

[jljorgenson18]

By the way, this is our current hack

    const widgetViewDesc = (view as any).docView.nearestDesc(widgetElement);
    if (widgetViewDesc) widgetViewDesc.ignoreMutation = mut => true;

[marijnh]

Okay, I see the problem now I think—the selection in the widget gets reset immediately by ProseMirror syncing its selection state with the DOM selection. But this will happen on any transaction anyway—when you create a selection inside the widget and, for example, a collaborative editing step comes in, your selection will be reset. Is giving the widget a tabindex, so that it becomes focusable on its own and doesn't share focus with the editor, something you can do? Because that seems like a more robust solution (when the editor doesn't have focus, it won't mess with the DOM selection).

[jljorgenson18]

In this case, we can't change the focus and we want the focus to remain in the editor. If a focus change fires then we lose certain focus states and focus changes can cause weird keyboard behavior on Android. Focus also doesn't happen on mouse down and only happens on click. That means that if you want to seamlessly select text inside of the editor that is not "editable" (like in a widget or a custom nodeview), you have to focus on mousedown. Otherwise, you can't do a click and drag selection. This also causes problems because text and paragraphs themselves are not typically supposed to be focused.

I could see a situation where collaborative editing or other transactions would reset the selection, but we haven't had that problem just yet. We've dealt with similar situations with our "nested" selection system but we were able to work around it by just restoring the selection synchronously if Prosemirror overwrites the selection.

I think it could be beneficial to create an attribute or some other system that does not rely on focus. I'm not suggesting this should be the answer, but imagine having something like this

<div class="widget" data-allow-selections="true"><p>I am selectable text</p></div>

Where Prosemirror could check if an ancestor had that attribute before deciding to overwrite the native selection in selectionToDom unless "force" is set to true. Here is a a quick, untested implementation.

const shouldIgnoreSelectedElement = () => {
  const sel = window.getSelection()
  if(sel.rangeCount === 0) return false
  const range = sel.getRangeAt(0)
  const container = range.commonAncestorContainer instance of Text ? range.commonAncestoreContainer.parentElement : range.commonAncestoreContainer
  return !!container.closet('[data-allow-selections="true"]')
}

export function selectionToDOM(view, force) {
  let sel = view.state.selection
  syncNodeSelection(view, sel)

  if (view.editable ? !view.hasFocus() || (shouldIgnoreSelectedElement() && !force) : !(hasSelection(view) && document.activeElement.contains(view.dom))) return

  view.domObserver.disconnectSelection()

[marijnh]

I think the widget options are a better place for this than the DOM. Attached patch introduces an ignoreSelection option to widgets.

[jljorgenson18]

Yep I think this makes sense! If I think of anything else on the focus + selection issue, I can try to create an RFC. I'll close the PR.