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

Can't delete embed blots on android #1985

Open
EthanRutherford opened this issue Feb 28, 2018 · 13 comments · May be fixed by #4476
Open

Can't delete embed blots on android #1985

EthanRutherford opened this issue Feb 28, 2018 · 13 comments · May be fixed by #4476

Comments

@EthanRutherford
Copy link

Steps for Reproduction

  1. Visit https://codepen.io/EthanRutherford/pen/LQqbRE on android
  2. focus the editor
  3. hit delete

Expected behavior:
the embedded blot is deleted

Actual behavior:
the editor is defocused, and the blot is not deleted

Platforms:
android O, chrome

Version:
1.3.5

@charrondev
Copy link

charrondev commented Mar 1, 2018

I did a little bit of testing myself and can confirm this on a few different platforms as well.

I was able to reproduce on Android versions 6->8.1 using Chrome 64 and the google keyboard. I was unable to reproduce on Samsung devices however (Chrome 64 and android 7.1, although these devices were not using the Google keyboard).

I also able to reproduce this issue on the same devices on the quill homepage https://quilljs.com. The same behaviour occurs with the formula embed in the main example.

@EthanRutherford
Copy link
Author

I can also reproduce with the formula embed on the homepage

@ivanalejandro0
Copy link

I've stumbled upon this problem as well.
It seems that the Gboard keyboard behaves differently than others.
To workaround this problem I redefined the update method on my custom blot that extends Embed to handle the childList case where a node is removed.

You can see a working example here: https://codepen.io/ivanalejandro0/pen/GdOBjQ

Here's a copy/paste of the code that does the trick:

const Base = Quill.import('blots/embed');
class MentionBlot extends Base {
  // ... more code here ...
  /**
   * Redefine the `update` method to handle the `childList` case.
   * This is necessary to correctly handle "backspace" on Android using Gboard.
   * It behaves differently than other cases and we need to handle the node
   * removal instead of the `characterData`.
   */
  update(mutations, context) {
    // `childList` mutations are not handled on Quill
    // see `update` implementation on:
    // https://github.com/quilljs/quill/blob/master/blots/embed.js

    mutations.forEach(mutation => {
      if (mutation.type != 'childList') return;
      if (mutation.removedNodes.length == 0) return;

      setTimeout(() => this._remove(), 0);
    });

    const unhandledMutations = mutations.filter(m => m.type != 'childList')
    super.update(unhandledMutations, context);
  }
  
  _remove() {
    // NOTE: call this function as:
    // setTimeout(() => this._remove(), 0);
    // otherwise you'll get the error: "The given range isn't in document."
    const cursorPosition = quill.getSelection().index - 1;

    // see `remove` implementation on:
    // https://github.com/quilljs/parchment/blob/master/src/blot/abstract/shadow.ts
    this.remove();
    
    // schedule cursor positioning after quill is done with whatever has scheduled
    setTimeout(() => quill.setSelection(cursorPosition, Quill.sources.API), 0);
  }
  // ... more code here ...
}

I've tried this with latest quill (1.3.6 as today), on Android 7.0, Chrome 66.0.3359.158. I've tested it with both Gboard and Samsung keyboards.

I think that this fix can be ported to the update method here https://github.com/quilljs/quill/blob/master/blots/embed.js#L67 but I didn't have time to go there.
Hopefully my workaround and example can give you some insight of the problem and how to workaround/fix it.

@lixiaoyan
Copy link
Contributor

lixiaoyan commented Jun 15, 2018

@ivanalejandro0
I have run into a problem with the code above, and after some digging, I found a way to resolve it.

      if (mutation.type === 'childList' && mutation.removedNodes.length === 1) {
        if (
          mutation.removedNodes[0] === this.leftGuard ||
          mutation.removedNodes[0] === this.rightGuard
        ) {

If you modified the dom inside embeds (specifically, removeChild), the embed may be removed unexpectedly. We should check whether the dom removed is one of the guard nodes first.

@sgtpepper43
Copy link

I'm running into this problem, and while the above code allows me to delete the embed, it also dismisses the keyboard.

@mahdiG
Copy link

mahdiG commented Nov 10, 2019

I used @ivanalejandro0 's solution and it worked. I had to blur and focus the element to reopen the keyboard.

@laukaichung
Copy link

laukaichung commented Sep 19, 2020

I have to add a zero-width no break space character (\uFEFF) next to my non-contenteditable embed block (not 'blots/embed' but blots/block/embed) along with @ivanalejandro0 solution in order to get it work on Android devices.

Custom Blot:

import React from "react"
import { render } from "react-dom";

export class TextBlot extends EmbedPatch {
  static blotName = "textBlot";
  static tagName = "div";
  static create(values) {
    const domNode = super.create(values);
    render(
      <React.Fragment>
        <div contenteditable={false}>
            {values.text}
        <div>
        {"\uFEFF"}
      </React.Fragment>,
      domNode
    );
    domNode.dataset = values;
    return domNode;
  }

}
import { Quill } from "react-quill";
const Embed = Quill.import("blots/block/embed");

export default class EmbedPatch extends Embed {
   // ivanalejandro0's methods 
}

The extended EmbedPatch class is basically the solution by @ivanalejandro0 (#1985 (comment)).

@Alejopdp
Copy link

Alejopdp commented Oct 2, 2020

Hi @laukaichung , do you have a full code example? I'm trying to fix this on my React-Quill functional component but i'm not getting it to work. I'm configuring a modules object inside my component to pass it to the real Quill js editor component. Using a blot class kinda crashes with my implementation.

If you could pass me a full example I'd really appreciate it.

Thanks!

@jullicer
Copy link

I've stumbled upon this problem as well.
It seems that the Gboard keyboard behaves differently than others.
To workaround this problem I redefined the update method on my custom blot that extends Embed to handle the childList case where a node is removed.

You can see a working example here: https://codepen.io/ivanalejandro0/pen/GdOBjQ

Here's a copy/paste of the code that does the trick:

const Base = Quill.import('blots/embed');
class MentionBlot extends Base {
  // ... more code here ...
  /**
   * Redefine the `update` method to handle the `childList` case.
   * This is necessary to correctly handle "backspace" on Android using Gboard.
   * It behaves differently than other cases and we need to handle the node
   * removal instead of the `characterData`.
   */
  update(mutations, context) {
    // `childList` mutations are not handled on Quill
    // see `update` implementation on:
    // https://github.com/quilljs/quill/blob/master/blots/embed.js

    mutations.forEach(mutation => {
      if (mutation.type != 'childList') return;
      if (mutation.removedNodes.length == 0) return;

      setTimeout(() => this._remove(), 0);
    });

    const unhandledMutations = mutations.filter(m => m.type != 'childList')
    super.update(unhandledMutations, context);
  }
  
  _remove() {
    // NOTE: call this function as:
    // setTimeout(() => this._remove(), 0);
    // otherwise you'll get the error: "The given range isn't in document."
    const cursorPosition = quill.getSelection().index - 1;

    // see `remove` implementation on:
    // https://github.com/quilljs/parchment/blob/master/src/blot/abstract/shadow.ts
    this.remove();
    
    // schedule cursor positioning after quill is done with whatever has scheduled
    setTimeout(() => quill.setSelection(cursorPosition, Quill.sources.API), 0);
  }
  // ... more code here ...
}

I've tried this with latest quill (1.3.6 as today), on Android 7.0, Chrome 66.0.3359.158. I've tested it with both Gboard and Samsung keyboards.

I think that this fix can be ported to the update method here https://github.com/quilljs/quill/blob/master/blots/embed.js#L67 but I didn't have time to go there.
Hopefully my workaround and example can give you some insight of the problem and how to workaround/fix it.

I tried putting your code either on quill/blots/embed or quill-mention/blots/mention but error stating quill is undefined.

@jullicer
Copy link

image
i updated the blot file and added update method

if user is removing the guard i reset the blot to text so the mention plugin will trigger again.

@oller
Copy link

oller commented Jan 29, 2021

image
i updated the blot file and added update method

if user is removing the guard i reset the blot to text so the mention plugin will trigger again.

    update(mutations, context) {
        mutations.forEach((mutation) => {
            if (
                mutation.type === "childList" &&
                (Array.from(mutation.removedNodes).includes(this.leftGuard) ||
                    Array.from(mutation.removedNodes).includes(this.rightGuard))
            ) {
                let tag;
                if (mutation.previousSibling) {
                    tag = mutation.previousSibling.innerText;
                } else if (mutation.nextSibling) {
                    tag = mutation.nextSibling.innerText;
                }
                if (tag) {
                    super.replaceWith("text", tag);
                }
            }
        });

        super.update(mutations, context);
    }

Thanks for the solution @jullicer, just typing it up, might save someone else the trouble!

@xuergo
Copy link

xuergo commented Oct 30, 2021

image i updated the blot file and added update method

if user is removing the guard i reset the blot to text so the mention plugin will trigger again.

Hello brother, where should I put it?

@wpj-wpj-wpj
Copy link

I've oppened PR #4476

the reason of this issue

the cursor focus between the contentNode of the embed and the rightGuard of the embed which is a zero-width space

in this case, the native selection range is like

{
 start:{
   node: rightGuard,
   offset: 0,
 }
}

until now, everything is fine

but when quill normalize this range, it goes the wrong way, because offset is 0.

image

the final range we get is the position before our embed

so when you backspace

you can't delete the embed

you just delete the previous sibling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.