Use <br> for zero-width white spaces

[mattkrick]

Do you want to request a feature or report a bug?

bug

What's the current behavior?

Using an extra span with a zero-width space character for line breaks is pretty nasty.

• It prevents performance improvements Reduce number of component updates #1938
• it prevents support for android (browsers that do not support input events level 2 do not allow compositionUpdate or beforeInput events to be canceled so it requires DOM-native behavior. See https://w3c.github.io/uievents/#event-type-compositionstart and https://www.w3.org/TR/input-events-1/)
• It is the root cause of Empty lines don't show a selected space on Mac #838 Get selected text of first inline node in a block [Bug] #910 Empty Blocks don't show a selection when selected by highlight #1181 New lines are removed on copy #1932 Clicking on a zero-width node and dragging does not select text #1969 to name a few

What's the expected behavior?

Using line breaks to preserve whitespace.

• it's how prosemirror does it
• it's how draftjs does it
• it's how quill does it
• it's how the DOM does it for vanilla contenteditable

I think it' safe to say it's how it should be done
I created this issue so we can first agree on the right way to do it, and then accept a suitable PR.

Here's the proposed acceptance criteria:

• Hitting enter adds a new block with a new leaf. inside that leaf is a <br>
• removing the contents of a line (eg backspace the only character on that line, highlight the entire line & hit delete, etc) turns the contents into a <br>

[ianstormtaylor]

Hey @mattkrick, those are good points.

For context, we actually used to have <br/> elements in the leaf nodes of blocks, just like Draft. I think it was removed ~1 year ago or so, I don't remember why exactly though. These kinds of things are often hard to keep track of, (hence the COMPAT: comments in the source code) since there are so many competing conflicts in browser support, IME support, copy-paste support, etc.

Quick searching... here's a handful of PRs in the area: #1644 #1388 #339 #1322

It sounds like it might have been an IME-related fix. But maybe there is another way to fix the IME-related issue that gets around the React re-rendering problem, and that means we can still preserve <br/> since they are helpful for other reasons.

I'm okay with reverting it if someone wants to make that PR, and having the IME issue fixed in another attempt afterwards, and not having them dependent on each other.

---

As a side note, could I ask that you tone down the condescension in your issues? These kinds of browser-related, IME-related things are very finicky, and a handful of people have super-helpfully contributed their time to try to get them right. No one is purposely adding "nasty" things to the codebase.

[zhujinxuan]

Hi, I cannot reproduce the problem with inspector turned on...

Can anyone help me find a way to reproduce the problem with inspector turned on?

[zhujinxuan]

Could anyone help me test this direty hack?

span[data-slate-zero-width="n"] {
  user-select: all;
  display: inline-block;
  min-width: 1em;
}

span[data-slate-zero-width="n"]:before {
  content:' ';
  user-select: all;
}

span[data-slate-zero-width="n"]::selection {
  background-color: cyan;
}

It seems working in my chrome. But I am not sure whether there are some IME issues.

[Nantris]

@ianstormtaylor I don't think @mattkrick meant at all to be condescending. I think only that he meant it has nasty (of course unintended) side effects, not that it's a nasty thing to do. I know for sure I've run into this type of thing in my own.

1. Find a problem
2. Create a solution
3. Use solution for weeks without issue
4. Discover unintended consequences of solution
5. Damn your ingenuity
6. Repeat