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

Cursor disappear when calling updateContents twice on Chrome #1057

Closed
benbro opened this issue Oct 16, 2016 · 14 comments
Closed

Cursor disappear when calling updateContents twice on Chrome #1057

benbro opened this issue Oct 16, 2016 · 14 comments

Comments

@benbro
Copy link
Contributor

benbro commented Oct 16, 2016

Cursor disappear when calling quill.updateContents(newDelta, Quill.sources.SILENT) twice on Chrome
Firefox 49 on Windows/Ubuntu doesn't have this issue.

The code is a simplified version of the authorship module.
#1049 (comment)

Steps for Reproduction

  1. Visit http://codepen.io/anon/pen/ALPRqy
  2. Type 'a'
  3. Type 'b'

Expected behavior:
Editor should stay in focus

Actual behavior:
Editor lose focus after the second character.

Platforms:
Chrome 54 on Windows 7

Version:

1.0.6

@sferoze
Copy link
Contributor

sferoze commented Oct 16, 2016

@benbro

I figured it out!

Based upon this stackoverflow: http://stackoverflow.com/questions/25897883/edit-cursor-not-displayed-on-chrome-in-contenteditable

It is because the authorship module nests the text inside a span element. In chrome this makes it hide the cursor

I fixed it in my app and your codepen by adding the css

.ql-editor span {
  display: inline-block;
  min-width: 1px;
}

The cursor now appears like normal.

I am not sure of any unwanted side effects from adding this css. Please let me know if you see any side effects. Otherwise this could be the solution.

@benbro benbro changed the title updateContents lose focus on Chrome Cursor disappear when calling updateContents twice on Chrome Oct 16, 2016
@jhchen
Copy link
Member

jhchen commented Oct 21, 2016

display: inline-block unfortunately has undesirable interactions with text wrapping: http://codepen.io/quill/pen/GjPqrp

@sferoze
Copy link
Contributor

sferoze commented Oct 21, 2016

@jhchen you are right. I noticed this issue.

I found a way to fix it. Here is my current fix, and I have not found any issues or side effects with it so far

t.editor.on 'text-change', (delta, oldDelta, source) ->

    if source is 'user'

      # hack to fix invisible cursor in span element due to authorship module
      node = document.getSelection().anchorNode
      if node? 
        sel = document.getSelection()
        sel?.collapse(node, 1)

@sferoze
Copy link
Contributor

sferoze commented Oct 27, 2016

Here is my latest update to the fix

t.editor.on 'text-change', (delta, oldDelta, source) ->

    if source is 'user'

      # hack to fix invisible cursor in span element due to authorship module

    sel = rangy.getSelection()
    node = sel?.nativeSelection.anchorNode

    if sel.isCollapsed
      if node? and !$(node).hasClass('ql-clipboard')
        if sel.rangeCount and sel.anchorOffset > 0
          range = sel.getRangeAt(0)
          range.collapse false
          range.collapseAfter(node)
          sel.setSingleRange range

The previous fix had some issues when pasting in text. This one works as far as I can tell

@sferoze
Copy link
Contributor

sferoze commented Nov 26, 2016

So I have been using the fix above for sometime now as it is needed if you are using the authorship module but today I noticed issues with the hack fix and android mobile.

The toolbar keeps flashing if using the bubble theme, and at the beggining of a new line it capitalizes and autoselects itself when typing on android mobile keyboard. Just has funky effects.

So I disabled authorship on mobile devices. but on desktop I still use the module and the fix appears to work fine.

I wonder what a longterm solution to this problem? Any idea @jhchen

@sferoze
Copy link
Contributor

sferoze commented Nov 26, 2016

Basically, should the authorship module still works by adding the authors class to the content the author has added? And if so should we add it using a span element?

Then if the way the authors edits are being tracked remains the same, we will have to figure out a solution to the cursor issue. Because the cursor disappears in a contenteditable element if you start typing in a element because the element collapses because it has no height. I remember I tried using a css fix before like min-height, or making span element block with no success. Most of what I tried had issues.

So then I tried the javascript solution.

It works on desktop as far as I can tell, but I am not sure what to do about mobile.

I really wish there was a solution for this, and I wonder what others think

@sferoze
Copy link
Contributor

sferoze commented Nov 26, 2016

Actually the weird issues on Android Mobile was not due to authorship and cursor hack fix.

It is actually an issue with Quill itself. I disabled bubble theme on mobile so this fixed the issue.

The other issue about autoselecting the first capitalized letter of a sentence I am still not sure about.

So there is a chance that the cursor fix

t.editor.on 'text-change', (delta, oldDelta, source) ->

  if source is 'user'

    # hack to fix invisible cursor in span element due to authorship module

    sel = rangy.getSelection()
    node = sel?.nativeSelection.anchorNode

    if sel.isCollapsed
      if node? and !$(node).hasClass('ql-clipboard')
        if sel.rangeCount and sel.anchorOffset > 0
          range = sel.getRangeAt(0)
          range.collapse false
          range.collapseAfter(node)
          sel.setSingleRange range

Is still a working solution for both mobile and desktop. I need to do more testing on mobile. It seems like iOS does not have issues but I have to check more on android.

@danielschwartz
Copy link

@sferoze i'm trying to apply the cursor fix, however am having trouble actually making your code work. You use rangy in rangy.getSelection() however rangyis not a variable anywhere else in your code. I've tried replacing it with document.getSelection() or editor.getSelection() both of which fail because it's unable to get nativeSelection.anchorNode.

Any chance you can look at the code you posted and make sure it actually works and/or post a generalized version?

@sferoze
Copy link
Contributor

sferoze commented Dec 8, 2016

@danielschwartz

Rangy is a popular library used for manipulating selections and cursor https://github.com/timdown/rangy/wiki/Rangy-Selection.

But might not be needed.

you could probably replace sel = rangy.getSelection() with sel = window.getSelection()

then also update node = sel?.nativeSelection.anchorNode with node = sel.anchorNode

I can't quite remember but I think I needed the rangy to get it to work accurately, but I could be wrong. Let me know what you find... thanks!

@danielschwartz
Copy link

@sferoze

Awesome, will try this with rangy. One last question for you: Does the full cursor fix include both the CSS (making spans inline-block) and JS fix, or just the JS fix?

@sferoze
Copy link
Contributor

sferoze commented Dec 8, 2016

@danielschwartz Just the JS fix that I posted above.

The css fix of making span inline-block had undesired side effects which caused issues with formatting in the editor. So css is not a good fix, but I have found this JS fix to work well. Let me know if you find any side effects from the JS fix, but so far I have been using it for sometime and have not noticed any issues.

@sferoze
Copy link
Contributor

sferoze commented Dec 31, 2016

@jhchen @danielschwartz @benbro

With the newest version of quill 1.1.8 the cursor does not disappear anymore with the authorship module added.

I am not sure why? Do you know why? I checked and it is still using span element with class to define which author added the text.

So I removed the cursor fix I posted above because it doesn't work with the newest quill 1.1.8 but it is not even needed with it. Because the cursor does not disappear. I have no idea what fixed it.

@benbro
Copy link
Contributor Author

benbro commented Jan 1, 2017

Confirmed that the issue is fixed in 1.1.8
http://codepen.io/anon/pen/ZLzKwg

@benbro benbro closed this as completed Jan 1, 2017
@sandro-pasquali
Copy link

Just wondering if it can be said that now Quill supports multiple cursors? Just confirming -- lots of conversations over the last few months. If so, is there a reference implementation available? Thanks!

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

No branches or pull requests

5 participants