carta icon indicating copy to clipboard operation
carta copied to clipboard

synchronizeScroll isn't in sync when there are large images

Open colinrobinsonuib opened this issue 8 months ago • 8 comments

https://github.com/user-attachments/assets/d4c822fe-77bb-4c50-8643-0bde6188e8d0

I think this is just a limitation of using percentage based scrolling. I'm working on an alternative scroll method based on line numbers.

colinrobinsonuib avatar May 07 '25 12:05 colinrobinsonuib

I'm working on it in this branch here and it seems to be working well. I'll add a video later.

https://github.com/colinrobinsonuib/carta/tree/betterscroll

The idea is that the scroll doesn't update until you click on the textarea. So this is something in between sync and async, it's a one directional sync on click. I called it click scroll.

We get the current line number that we clicked on from carta. We then find the corresponding element in the renderer that has a data-line value close to the line number in the textarea.

The problem is that all the elements in the rendered output need to have the data-line number. This is very easy to do with rehype, and I have an example plugin here:

https://github.com/colinrobinsonuib/carta/blob/betterscroll/packages/carta-md/src/routes/%2Bpage.svelte

function rehypeLineNumbers() {
		return (tree: Root) => {
			visit(tree, 'element', (node: Element, index, parent) => {
				// Only apply to elements that are direct children of the root
				if (parent === tree && node.position?.start?.line) {
					if (!node.properties) {
						node.properties = {};
					}
					node.properties['data-line'] = node.position.start.line;
				}
			});
		};
	}

But whats the best way to include that into core? I'm not sure because the scroll behavior is driven by MarkdownEditor and by that point Carter has already been initialized.

colinrobinsonuib avatar May 07 '25 12:05 colinrobinsonuib

I ended up creating a rehype package (rehype-line-numbers) and added it in carta.ts at the end of the processing stack:

asyncProcessor.use(rehypeLineNumbers);
asyncProcessor.use(rehypeStringify);

The scroll modes are now 'sync' | 'async' | 'cursor'

<MarkdownEditor scroll="cursor" value={sampleText} mode="split" {carta} />

The mode cursor triggers on the selectionchange event which fires every time the cursor moves. This can be: clicking, selecting text, or moving the cursor with the arrow keys.

https://github.com/user-attachments/assets/6329ebbb-4f68-4e07-b05a-d064fa6c4561

colinrobinsonuib avatar May 08 '25 08:05 colinrobinsonuib

This is great! I originally hoped to achieve something like this, but I didn't figure out a way to it. This would be much better compared to the standard behaviour, which doesn't work well with large texts or with images, as you've shown. I'm not really sure about the new 'cursor' mode: I think it would be better to just replace the current 'sync' mode, so that it works on scroll events, and not cursor one. I guess you've done it this way so that you can get the corresponding line element...

Maybe the same could be achieved by calculating the scroll percentage on the input, and using that determine approximaly the index of the line:

line_index = floor(input_scroll_percentage * number_of_lines)

BearToCode avatar May 09 '25 07:05 BearToCode

Okay, what do you think about this method for finding the line number on scroll event instead of on click/selectionchange event?

https://github.com/colinrobinsonuib/carta/commits/betterscroll-onscroll/ https://github.com/colinrobinsonuib/carta/commit/9233a421bbb38f906aedf05a799ff2346d8da841

I'm picking a point on the top left and detecting what <span class='line'> is under it. I then get the child index of that span, and that is the line number. I then scroll the Renderer so the top element is the one with the closest corresponding data-line number.

Image

Image

Scroll is currently only from Input -> Renderer, but the same technique could be used to scroll in the other direction.

colinrobinsonuib avatar May 09 '25 10:05 colinrobinsonuib

Also worth noting that we still use rehype-line-numbers to add the data-line attr. We can't just count the number of elements in the Renderer and assume that the 2nd element in the output matches the 2nd element in the input, because rehype can move things around. For example:

This markdown:

Banana[^1] Apple[^2] Cabbage[^3]

[^1]: A fruit

[^2]: Also a fruit

[^3]: This one is a vegetable

Have a nice day

Is rendered like:

Banana1 Apple2 Cabbage3

Have a nice day

Footnotes:

  1. A fruit
  2. Also a fruit
  3. This one is a vegetable

So Have a nice day is the 9th <span class='line'> element on the left, but the 2nd HTML element on the right.

colinrobinsonuib avatar May 09 '25 10:05 colinrobinsonuib

I don't really like using document.elementsFromPoint. It seems very prone to breaking in specific use cases. Building from my previous answer, which I understand won't work as lines (as elements with class.line) don't all have the same height, maybe we could try the following:

for line in lines
    is_visible = line.offsetHeight > container.scrollTop
    if is_visible
        top_line = line
        break
    end
end

I might have forgotten something, but this seems a more robust approach. I would clean the code a little bit(even my own!), but it looks very good!

BearToCode avatar May 09 '25 19:05 BearToCode

Instead of checking is_visible like that, I implemented a solution with the Intersection Observer API. Checkout this branch here:

https://github.com/colinrobinsonuib/carta/tree/betterscroll-IntersectionObserver

Overview

The gist of it is that you set an IntersectionObserver on inputElem > .carta-highlight > pre > code to watch the list of span.line elements. You do the same with renderElem and the HTML elements. This triggers a callback function whenever one of the child elements intersects with the top of the root element. This also means we don't need to use the onscroll event anymore.

We also setup MutationObservers on the root elements so that whenever there is a change, we stop watching the child elements and then re-observe the new children.

Functions

setupScrollSync()

This sets up everything described above.

customSmoothScroll()

When you scroll using something like

isSyncingHtml = true;
htmlElem.scrollIntoView({ block: 'start', behavior: 'instant' });
setTimeout(() => { isSyncingHtml = false; }, SYNC_DELAY);

The main problem is stacking/cancelling prior scrolls. We want to start scrolling as soon as possible. You scroll past line 3 in the input and that begins a scroll to line 3 in the renderer, but before you get there, you scroll past line 4 in the input which triggers another scroll action to line 4 in the renderer. This causes "rollback" as the elements in renderElem trigger the observer. For me, the easiest thing was to create a custom scroll function so we can cancel prior scroll animations as we add new ones. There's probably a way to use scrollIntoView but I also like that we can set the duration and easing function.

setScrollPosition

When the windowMode is tabs, and we switch tabs, instantly scroll to whatever line number you were on for the other tab.

findElementByLineOrBefore

Helper function for setScrollPosition. Not every line number in the Input exists in the Renderer, so when we switch from write tab to preview tab, we find whatever output line number is closest to the input line we were on.

Performance

I noticed when I type, there is quite a lot of lag in large documents (none in small documents). At first I thought this was because of rebuilding the observers on every keypress, so I added a debounce to the mutation callbacks, but then I noticed that the lag is happening in the main branch of Carta as well.

Video here:

https://github.com/BearToCode/carta/issues/157#issuecomment-2871205079

I don't think there are any performance issues with my code, and when I set const USE_HIGHLIGHTER = false; everything is super fast. But I can't fully assess the impact when editing large documents until the Carta core also stops lagging when editing large documents.

colinrobinsonuib avatar May 12 '25 08:05 colinrobinsonuib

Nice work! 🚀 I like this approach more. If you create a PR, I'll check it out in more details as soon as I got some time.

BearToCode avatar May 14 '25 19:05 BearToCode