bustle / mobiledoc-kit

A toolkit for building WYSIWYG editors with Mobiledoc
https://bustle.github.io/mobiledoc-kit/demo/
MIT License
1.55k stars 156 forks source link

Multiple space → nbsp translation has poor line wrapping behavior #748

Open andersk opened 3 years ago

andersk commented 3 years ago

mobiledoc-kit currently renders multiple consecutive spaces by replacing each space-space pair with space-nbsp (#209). Depending on the text width, this results in the left margin being misaligned when a line is wrapped at that point. Emphasizing nbsp with _ to illustrate the problem:

This is a sentence that is followed by two
spaces. _This is a sentence that is followed
by three spaces. _ This is another sentence.

This is a sentence that is
followed by two spaces.
_This is a sentence that is
followed by three spaces.
_ This is another sentence.

To fix this, it would be better to replace each sequence of n spaces with n − 1 nbsps followed by 1 space. This can be done with regexp lookahead:

text.replace(/ (?= )/g, "\u00A0")
This is a sentence that is followed by two
spaces._ This is a sentence that is followed
by three spaces.__ This is another sentence.

This is a sentence that is
followed by two spaces._
This is a sentence that is
followed by three spaces.__
This is another sentence.

It might be better to disable the nbsp translation entirely and rely on CSS white-space: pre-wrap, but that’d be a bigger change.

(The same issue is present in other repositories like mobiledoc-dom-renderer, mobiledoc-html-renderer, mobiledoc-apple-news-renderer; I assume I don’t need to file separate issues for each one.)

gpoitch commented 1 year ago

I believe this was fixed somewhere along the way. Can't reproduce in the demo.

andersk commented 1 year ago

This was not fixed. It still reproduces in the demo:

Screenshot from 2022-08-31 11-23-40

And the incorrect code is still here:

https://github.com/bustle/mobiledoc-kit/blob/4b40e5127877c5bb3da138e5076a1b68a4698552/src/js/renderers/editor-dom.ts#L60

and in other repositories:

https://github.com/bustle/mobiledoc-apple-news-renderer/blob/357dd068e45f218983cdbc55d73b420ceeedced8/lib/utils/dom.js#L3 https://github.com/bustle/mobiledoc-dom-renderer/blob/0e6d290024d9d277ee64acf2df66928b60806af6/lib/utils/dom.js#L5 https://github.com/bustle/mobiledoc-html-renderer/blob/54ac2b5b1e8d4e6e758ebd6b6374a2d01aaa6705/lib/utils/dom.js#L43

Please reopen.

gpoitch commented 1 year ago

Looks good in my browser

Screen Shot 2022-08-31 at 2 44 07 PM
andersk commented 1 year ago

Yeah, looks like WebKit handles the wrapping differently. Try it in Chromium or Firefox.