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

Improvement: handle when editor not styled with pre-wrap #981

Closed
3 tasks
hitsthings opened this issue Oct 3, 2019 · 3 comments
Closed
3 tasks

Improvement: handle when editor not styled with pre-wrap #981

hitsthings opened this issue Oct 3, 2019 · 3 comments

Comments

@hitsthings
Copy link

hitsthings commented Oct 3, 2019

Issue details

Either in code, or in documentation as a warning, it might be good to handle the need for white-space: pre-wrap to avoid issues with ' ' vs ' '

Steps to reproduce

When 'b' was selected in 'a b c', and I typed a space, handleTextInput would be called with text=' ', from=1 and to=4 (if I've got my numbers right). That is, it was treated like I had pasted three spaces while the selection was ' b '.

It turns out this was because my content container was not white-space: pre-wrap. The browser was changing the HTML to <mark>a</mark>&nbsp;<font><b>&nbsp;</b></font>&nbsp;<mark>c</mark>.

https://glitch.com/edit/#!/sandy-constellation

https://sandy-constellation.glitch.me/

ProseMirror version

Latest

Affected platforms

  • [/ ] Chrome
  • Firefox
  • Internet Explorer
  • Other

Screenshots / Screencast (Optional)

@hitsthings
Copy link
Author

One way to handle this would be to relax the text comparison in findDiffStart.

if (childA.isText && childA.text != childB.text) {
      for (var j = 0; childA.text[j] == childB.text[j]; j++) {
        pos++;
      }

      return pos;
    }

childA.text[j] == childB.text[j] || (spaceChars.includes(childA.text[j]) && spaceChars.includes(childB.text[j])

That's probably going to cause some knock on problem though.

Another option is to warn if getComputedStyle().whiteSpace isn't pre/pre-wrap.
Third option would be documentation about required styling.

Fourth option is do nothing, but I know how much pain this caused me and it'd be nice to save others. :)

@marijnh
Copy link
Member

marijnh commented Oct 3, 2019

The need to include prosemirror.css is documented, but it seems that the editor mostly works well enough without it and people don't notice it. I guess doing a check (maybe a second after the first time the editor is focused, to avoid false positives due to deferred css file loading) and emitting a console warning would be reasonable. Want to submit a PR?

@hitsthings
Copy link
Author

Definitely my bad for missing that line in the docs. I added the warning in ProseMirror/prosemirror-view#62 . After doing it, feels like it's as much of a loss for maintainability as it is a win for newbies though. 🤷‍♂

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

2 participants