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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

delete fail on mobile #40

Open
wxgeorge opened this issue Oct 12, 2018 · 7 comments 路 May be fixed by #275
Open

delete fail on mobile #40

wxgeorge opened this issue Oct 12, 2018 · 7 comments 路 May be fixed by #275

Comments

@wxgeorge
Copy link

wxgeorge commented Oct 12, 2018

First 馃檶 for an incredible plugin.

Deleting mentions doesn't work with the current release on mobile handsets I've tested (Nexus 5X + OnePlus6). I'm seeing distinct issues in Firefox and Chrome on Androids 8.1 and 9. gifs follow.

Is this a regression? As the recordings show, most of the functionality seems present (i.e. '@' triggering user menu, typing sub-selecting menu, being able to select a mention all seem to work ...)

on Chrome: (v69.0.3491.100 + v61.0.3163.98)
chrome v69 0 3497 100 on oneplus6 running oxygenos v9

on Firefox (v62.0.3)
firefox v62 0 3 on oneplus6 running oxygenos v9

Recordings done with OnePlus 6, running OxygenOS v9 (+ zoom.us for screensharing + gifox).
Behaviours reproduced on Nexus 5X running Android 8.1

@chanlito
Copy link

chanlito commented Dec 5, 2018

Yep just ran into this issue as well, delete doesn't work on mobile.

@keligijus
Copy link

It seems it's a bug with Android and/or Chrome on Android. Here's a suggestion to fix it with zero-width space ProseMirror/prosemirror#565 (comment)

@wxgeorge and @chanlito have you managed to fix this?

@keligijus
Copy link

Any updates on this?

keligijus added a commit to keligijus/quill-mention that referenced this issue Jul 17, 2019
@equilerex
Copy link

equilerex commented Oct 27, 2020

@MadSpindel this affects every single android phone, there are multiple open issues / pr's in this repository and the firstthing that comes up when you google "quill mention android" is this thread. its obviously an issue, so maybe reconsider?
Personally, looking into forking off or writing a new implemention since this is a pretty heavy bug for android users.

@MadSpindel
Copy link
Member

@equilerex Sorry, I will reopen the issue and feel free to make a pull request to this repo.

@MadSpindel MadSpindel reopened this Oct 27, 2020
@equilerex
Copy link

equilerex commented Oct 28, 2020

Been poking around with it for more than a day... its a real frustrating freak of an issue.
For anyone else looking for a solution, heres a pretty reliable one i came up with.

the only caveat is that since contenteditable is not applied to the whole blob anymore, rather its child elements, it is somewhat possible to capture focus / set the cursor onto the actual "blob" on any device now (if you click really, really accurately exactly in the beginning or end of the mention text) - as a graceful fallback, any attempt to trigger changes from that position will result in the blob being removed. (not 100% ideal but it works and it does not leave the user with a broken interface as is currently the case)

(if anyone wants the full minified package, ive uploaded it here: https://github.com/equilerex/quill-mention )

import Quill from "quill";

const Embed = Quill.import("blots/embed");

class MentionBlot extends Embed {
  static create(data) {
    const node = super.create();
    // prefix character
    const denotationChar = document.createElement("span");
    denotationChar.className = "ql-mention-denotation-char";
    denotationChar.innerHTML = data.denotationChar;
    denotationChar.setAttribute("contenteditable", false);

    // Content
    const dataContainer = document.createElement("span");
    dataContainer.innerHTML = data.value;
    dataContainer.setAttribute("contenteditable", false);

    // when android keyboard reaches a `contenteditable=false` block, it automatically closes.
    // avoid that by adding a buffer "space" without the attribute.
    const AndroidBackspaceFix = document.createElement("span");
    AndroidBackspaceFix.innerHTML = " ";
    // it needs to be "visible" in order to work - so limit to minimal size.
    AndroidBackspaceFix.setAttribute("style", "display: inline-block; height: 1px; width: 1px; overflow: hidden; ");

    node.appendChild(denotationChar);
    node.appendChild(dataContainer);
    node.appendChild(AndroidBackspaceFix);

    return MentionBlot.setDataValues(node, data);
  }

  static setDataValues(element, data) {
    // the extended Embed constructor has added contenteditable=false to the outermost span,
    // we want to override that in favour of ones applied to the child elements inside create()
    setTimeout(() => {
      element.getElementsByTagName("span")[0].setAttribute("contenteditable", "inherit");
    }, 0);

    const domNode = element;
    Object.keys(data).forEach(key => {
      domNode.dataset[key] = data[key];
    });
    return domNode;
  }

  static value(domNode) {
    return domNode.dataset;
  }

  // android Gboard backspace does not fire onkeypress events, resulting in the caret
  // breaking into the read-only blot element. - so we need to handle edit events inside the blot child elements as well
  update(mutations, context) {
    // `childList` mutations are not handled on Quill
    // see `update` implementation on:
    // https://github.com/quilljs/quill/blob/master/blots/embed.js

    // any attempt at modifying the inner content will just remove it
    // (since we cant block any modifiications completely, this is the "lesser evil" / graceful fallback)
    for (const mutation of mutations) {
      if (mutation.type === "attributes" && mutation.attributeName === "contenteditable") continue;
      setTimeout(() => this.remove(), 0);
      return;
    }
  }
}

MentionBlot.blotName = "mention";
MentionBlot.tagName = "span";
MentionBlot.className = "mention";

Quill.register(MentionBlot);

@hpierre74
Copy link

Hello @MadSpindel 馃憢 Is there any fix incoming for this issue ? It's quite a huge bug 馃槥
I'd be pleased to help and make some PR if you point me in the right direction. It seems that @equilerex solution is not working with the last releases

@omega1996 omega1996 linked a pull request Apr 21, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants