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

Improving edge detection for blocks #3014

Closed
ephox-mogran opened this issue Oct 13, 2017 · 1 comment
Closed

Improving edge detection for blocks #3014

ephox-mogran opened this issue Oct 13, 2017 · 1 comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion...

Comments

@ephox-mogran
Copy link
Contributor

ephox-mogran commented Oct 13, 2017

Issue Overview

At the moment, detecting if you are at the end of the block is incomplete. The existing approach of using textContent only really works when the current node is a text node. It is difficult to get non-text nodes in the content at the moment, so this issue hasn't been very visible.

A text node has valid cursor positions at 0 all the way of up to its textContent.length (or nodeValue.length)
An element has valid cursor positions from 0 all the way up to the number of children (childNodes.length). Its textContent is largely irrelevant for cursor position.

See the Range API section of this blog post I helped with earlier this year for details: https://go.tinymce.com/blog/a-quick-guide-to-browser-selection-models/

There are also a few extra considerations just to make things fun :)

  1. tags like <img> have a 0 for their start, and a 1 for their end
  2. editors tend to use zero width characters etc. to allow cursor positions that content editable doesn't respect. You may start finding yourself needing to filter these out when checking the real 'length' of a text node. The inline link boundaries will be using these characters
  3. if unusual node types like comment tags appear in your nodes, this might also affect your offsets.

Typically, you'll often get a text node, so it won't matter most of the time. But if someone puts an element at the end of their block (e.g. image), then the last cursor position in that block is going to be (<p>, n) or (<img>, 1). Therefore, textContent won't help you identify that you're at the edge.

Steps to Reproduce (for bugs)

  1. Open a block that has an image at the end (may be difficult ... I just injected it)
  2. Go to the end of the block and try and move to the block underneath it with

Possible Solution

The solution is to create a few more methods in dom.js that determine what the end of a particular node is (or the start), and use those methods when doing edge detection.

@iseulde , I'll have a PR on this shortly. Let me know what your thoughts are. I don't expect this to cover all of the edge cases, but it will be a start.

PR: #3015

@mcsf
Copy link
Contributor

mcsf commented Jul 9, 2018

Seems like we can close this. Regardless of implementation, writing flow has come a long way. There are certainly micro-optimizations to be had, to be addressed in focused issues.

@mcsf mcsf closed this as completed Jul 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion...
Projects
None yet
Development

No branches or pull requests

3 participants