#130 Added recalculation completion listener to the spatialTree

[oleksandr-danylchenko]

Issue - #130

Changes Made

Following the UndoStack example from A9S, I added the nanoevents emitter to the spatialTree.
Currently, we're interested in just a single event type - recalculated. Which fires upon the completion of the recalculate function

[rsimon]

Recalculation is a sync operation, though. Might be cleaner to listen to the actual causes than adding this?

[oleksandr-danylchenko]

The recalculate is invoked as a reaction to the resize of the window or the content within the baseRenderer:

text-annotator-js/packages/text-annotator/src/highlight/baseRenderer.ts

Lines 134 to 147 in e6d3f33

	// Refresh on resize
	const onResize = debounce(() => {
	store.recalculatePositions();
	if (currentPainter)
	currentPainter.reset();
	redraw();
	});
	window.addEventListener('resize', onResize);
	const resizeObserver = new ResizeObserver(onResize);
	resizeObserver.observe(container);

An initial assumption can be that we should add the same listeners to the consuming app and on their firing the spatial tree will already be recalcuted.
However, that's not true, because we cannot know that the consumer app's listeners will get triggered after the recalculation.

Therefore, the listener I added in the PR is aimed to provide a method that will allow reacting to a definite completion of the recalculate function execution, triggered by the resize observer.

[oleksandr-danylchenko]

Might be cleaner to listen to the actual causes than adding this?

I have a Note component that gets positioned next to the highlight. I use FloatingUI to position it, and its virtual element feature. The reference then is the highlight bounds:

refs.setPositionReference({
	getBoundingClientRect: () => {
		const bounds = r.state.store.getAnnotationBounds(annotation.id);
		const denormalizedBounds = bounds
			? denormalizeRectWithOffset(bounds, r.element.getBoundingClientRect())
			: new DOMRect();

		const { left: containerLeft } = notesContainer.getBoundingClientRect();
		const { top: annotationTop, height: annotationHeight } = denormalizedBounds;

		return new DOMRect(containerLeft, annotationTop, 0, annotationHeight);
	}
});

Floating UI cannot automatically track the changes on screen when the reference is virtual. So I need to manually call the update function when the content resizes.

useContentContainerResize({ onResize: () => update() });

Unfortunately, when I call it immediately upon the resize, the r.state.store.getAnnotationBounds, used for the virtual element, still returns an obsolete position. That's because the spatial tree hasn't finished its own recalculate call triggered by the resize

[rsimon]

I'm afraid I don't have the time to look into this more closely right now (and I'm only on mobile). But might be similar problem to this change I made recently?

0cd547d

[oleksandr-danylchenko]

I'm afraid I don't have the time to look into this more closely right now

No problem, that's a Friday evening first of all 😅. We have a separate fork to proceed with the development, but I'm glad to keep you in a loop of changes that might be beneficial for the annotator in general.

[oleksandr-danylchenko]

I'm afraid I don't have the time to look into this more closely right now (and I'm only on mobile). But might be similar problem to this change I made recently?

0cd547d

I had the same code before in my app and it works perfectly when the virtual reference element is consulting the native browser's ranges 👌🏻

---

However, we couldn't use them anymore, due to the 3rd party integration in our app that wildly mutates the underlying DOM, making the captured ranges collapse. That's why I switched to the spatial tree and its cached boundaries. (#123). So the issue this PR solves only exists when the popup/note/etc. consults the spatial tree.

[rsimon]

Either way: IMO a component shouldn't trigger an async-like event after completing a sync operation. At least it should be the component that triggers the sync op. (Might already be the case in your PR. As I said: can't look into it right now yet :-)

[oleksandr-danylchenko]

IMO a component shouldn't trigger an async-like event after completing a sync operation. At least it should be the component that triggers the sync op

That's fair, I like this suggestion 👍🏻
I'll see how I can tweak the emitter to match it, thanks!

[rsimon]

I had the same code before in my app and it works perfectly when the virtual reference element is consulting the native browser's ranges 👌🏻

Got it, thanks!

[oleksandr-danylchenko]

IMO a component shouldn't trigger an async-like event after completing a sync operation. At least it should be the component that triggers the sync op

I think that having the emission from the spatialTree fits the usage patterns we already have in the A9S Store and the UndoStack 🤔

Although those components don't have asynchronous methods, they still emit the events as a reaction to their functions's invocation. Example:

Or another one:

So I can envision the spatial tree as the "store"-like module from which we can receive events about recalculation / update / etc.

---

I also thought about moving the event emission either to the Store's recalculatePositions:

text-annotator-js/packages/text-annotator/src/state/TextAnnotatorState.ts

Line 131 in e6d3f33

const recalculatePositions = () => tree.recalculate();

or next to the recalculatePositions call from the baseRenderer:

text-annotator-js/packages/text-annotator/src/highlight/baseRenderer.ts

Lines 134 to 142 in e6d3f33

	// Refresh on resize
	const onResize = debounce(() => {
	store.recalculatePositions();
	if (currentPainter)
	currentPainter.reset();
	redraw();
	});

But I'm not extremely fond of them. Because then we can have a scenario when the recalculate gets invoked directly on the spatial tree instance, but none of the onRecalculatePositions listeners won't be notified 🤔

Therefore, I would recommend (but not insist yet 😅) leaving the emitter.emit('recalculate') within the spatialTree.