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

Use a span instead of image to prevent word-breaks #60

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jeffrafter
Copy link

Fixes codemirror/dev#989

Currently the <img> tag used for the cm-WidgetBuffer causes a word break. This is especially noticeable with collaborative cursors (i.e., from YJS and y-codemirror.next). The breaking behavior is weird. Joining with a &NoBreak; in the text causes content changes in the contenteditable so this doesn't work reliably:

      let dom = document.createElement("img");
      dom.insertAdjacentText("beforebegin", "\u2060");
      dom.insertAdjacentText("afterend", "\u2060");
      dom.className = "cm-widgetBuffer";
      dom.setAttribute("aria-hidden", "true");

This comment from @marijnh was insightful:

codemirror/dev#989 (comment)

It turns out there is such a mysterious element: <nobr>. It was never in a spec but was implemented. But it has been deprecated. Changing the buffer it a <nobr> worked in Safari and Firefox (even with obscure CSS problems and no CSS) but didn't work in all cases with Chrome (3D transforms seemed to still break, which might not be important).

The attached fix just uses a <span style="white-space:nowrap"> and it works in all of the cases we want.

In my application CSS I also have (not sure if it is required):

.ͼ1 .cm-widgetBuffer {
  display: inline;
}

.ͼ1 .cm-widgetBuffer::before, .ͼ1 .cm-widgetBuffer::after {
  content: '\002060' !important; // this is a unicode non-breaking space / empty word-joiner char
  display: inline;
  white-space: nowrap;
}

@jeffrafter
Copy link
Author

I'll note that while this solves the word-wrap / word-break problem, it doesn't solve content editable spellchecking being split by the cursor:

image

@marijnh
Copy link
Member

marijnh commented Nov 9, 2023

Have you considered using a layer instead of a widget for your remote cursors?

@jeffrafter
Copy link
Author

👋 yes, and am still thinking it might be a good path. I looked into LayerMarker and RectangleMarker and can give it a shot. Even pursuing that, this still feels like a valuable change. Without it, word-breaks are not controllable.

@marijnh
Copy link
Member

marijnh commented Nov 13, 2023

For many widgets, them breaking words is the appropriate behavior.

In the early days of the library I tried a bunch of different elements, including spans and <wbr>, and all of them had some issues that empty images did not have. I didn't keep notes on it, but this is a very messy, subtle hack that I'm not really willing to overhaul again, as I expect there'll be a ton of regressions if we do.

@jeffrafter
Copy link
Author

That's fair. I don't have those notes so I will trust you. Would you consider a patch that exports this class (so I can use patchMethod on it) or possibly a signature that would allow a callback on the sync method so it could be controlled by individual widgets?

@marijnh
Copy link
Member

marijnh commented Nov 14, 2023

This example works for me.

@marijnh
Copy link
Member

marijnh commented Nov 16, 2023

I don't really see a non-messy non-footgun way in which such a thing could be exported. If you want to do dodgy monkey-patching, you can already create an editor with a widget, get the widget buffer DOM node, and access node.cmView.constructor to get a path to WidgetBufferView.

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

Successfully merging this pull request may close these issues.

WidgetBuffer creates wrap opportunity in chrome
2 participants