V4

[rsimon]

The architecture has grown and IMO the code would benefit from refactoring and cleanup. I started drafting a "version 4" in a separate branch. So far, I have only worked on the text-annotator package, not the TEI or React packages yet.

Key Changes

• Removed the canvas renderer. I don't think anyone has been using it, and compared to the (default) SPANs renderer it IMO has no advantages.
• Removed the presence utility. I don't think anyone (except ourselves in Recogito Studio) is using this either, and I'd like to move that into a separate plugin package from v4 onwards.
• Sub-divided the utils folder. Introduced a (hopefully) reasonable structure, for better readability.
• Cleaned up project structure. Structure and naming has become a bit chaotic. I made an attempt at cleaning this up. Files are also renamed to a common lowercase+hyphen naming convention.

The project structure is now reduced to:

- model/              # core and w3c data model classes (no change)
- rendering/          # rendering code (previously "highlight")
  - renderer-css-highlight/
  - renderer-spans/
  - base-renderer.ts
  - color.ts          # color utilities - used to be inside /spans
  - highlight-style.ts
  - highlight.ts
  - renderer.ts       # essential types
  - viewport.ts
- state/ # no change
- utils/
  - annotation/       # annotation-related helpers
  - dom/              # DOM element and range helpers
  - events/           # event handling helpers
  - highlight/        # helper related to highlight rendering
- selection-handler.ts
- text-annotator.ts
- text-annotator-options.ts
- text-annotator.css

Next Steps

• Use CSS modules for the stylesheets
• Extend the setStyle(style) method to setStyle(style, id?), as a helper to easily set specific styles on a particular annotation. (Mostly needed to externalize the presence package, but should be generally useful.)

[rsimon]

CC'ing @oleksandr-danylchenko for info & feedback!

[oleksandr-danylchenko]

Thanks for the mention! The key changes look promising, and the repo would definitely benefit from some standardization.

---

I have a few questions:

1. renderer.ts and the base-renderer.ts are pretty close names. Especially, if the renderer.ts doesn't contain a specific implementation.

2. The color.ts can be placed under utils/highlight or even highlight-style.ts. Its single getBackgroundColor operates with the HighlightStyle.

3. Extend the setStyle(style) method to setStyle(style, id?)

   Wouldn't it be superfluous, as the HighlightStyleExpression exposes the annotation entirely? Then, the consumer can apply the annotation-specific styles within the callback.

[rsimon]

Thanks for the quick feedback, @oleksandr-danylchenko - all great points!

1. renderer.ts includes only the types, while base-renderer.ts has the key utility function that implements the boilerplate that's usually common among renderers. But I take your point. Perhaps just pack everything in one file, named renderer.ts?
2. Fully agreed about color.ts. Given this is (probably) exclusive to the rendering package, I think it makes sense to pack it into highlight-style, as you say!
3. I tried that, specifically for the use case of temporarily swapping the highlight color in our multi-user system. (Remote user selects an annotation -> color changes for other users.) It's actually tricky... essentially, you end up building a "wrapping system" that maintains the current base style, plus adds/removes temporary override styles, as users select and deselect. It felt useful enough to build that once and add it as a feature, rather than having people rebuild themselves outside. (Especially since it could be a nice feature, having a quick way to programmatically emphasize annotations temporarily, without having to replace the whole styling function.)

[oleksandr-danylchenko]

1. But I take your point. Perhaps just pack everything in one file, named renderer.ts?

   I like the idea of packing everything into a single file. Although I prefer the base-renderer name a bit more. As it signifies that it can't act on its own, and requires a concrete implementation injection.

[rsimon]

I like the idea of packing everything into a single file. Although I prefer the base-renderer name a bit more.

Makes sense - done!

[oleksandr-danylchenko]

3. I tried that, specifically for the use case of temporarily swapping the highlight color in our multi-user system. (Remote user selects an annotation -> color changes for other users.) It's actually tricky...

Yeah, I can agree with that... The callbacks API is super explicit, but could be more verbose.

I tried to address this issue internally by using a cascade of HighlightStyleMiddleware:

type HighlightStyleMiddleware = (
	annotation: WebtextsAnnotation,
	state: AnnotationState,
	style: HighlightStyle
) => HighlightStyle;

const TRANSPARENT_HIGHLIGHT_STYLE: HighlightStyle = {
	fill: 'rgba(0, 0, 0, 0)',
	fillOpacity: 0
};

// Emulates the default browser's highlight style
const DEFAULT_HIGHLIGHT_STYLE: HighlightStyle = {
	fill: 'rgb(217, 233, 255)',
	fillOpacity: 0.7
};

...

const highlightStyle = useCallback<HighlightStyleMiddleware>(
	(annotation, state, style) => {
		...
		const highlightStyle = state.selected ? DEFAULT_HIGHLIGHT_STYLE : style;
		...
		return shouldOverride
			? {
					...highlightStyle,
					fill: ...
					fillOpacity: ...
				}
			: highlightStyle;
	},
	[...]
);

const sharedStyle = useCallback<HighlightStyleMiddleware>(
	(annotation, state, style) => {
		...

		if (state.selected && (...special_case...)) {
			return {
				...style,
				fill: ...,
				fillOpacity: ...
			};
		}

		const showSharedUnderline = ...
		if (showSharedUnderline) {
			return {
				...style,
				underlineColor: ...,
				underlineOffset: 2,
				underlineThickness: 2
			};
		}

		return style;
	},
	[...]
);

useEffect(() => {
	if (!r) return;

	const styleMiddlewares = [highlightStyle, sharedStyle];

	const style: HighlightStyleExpression = (annotation, state) =>
		styleMiddlewares.reduce(
			(style, middleware) => (middleware ? middleware(annotation, state, style) : style),
			TRANSPARENT_HIGHLIGHT_STYLE
		);

	const setStyleFrameId = requestAnimationFrame(() => r.setStyle(style));
	return () => cancelAnimationFrame(setStyleFrameId);
}, [highlightStyle, sharedStyle, r]);

Each middleware acts on the previously applied style, modifying it based on the internal state of an annotation, or an external state of the consumer.

[rsimon]

Yep, that's pretty much going into the direction I mean. (And in your case you don't even toggle for specific annotations, if I see correctly). Having a setStyle(style, id) function to toggle specific annotations wouldn't eliminate the need for such a middleware approach, obviously, but might help in some specific selection/emphasis cases I guess.

[oleksandr-danylchenko]

Do I understand correctly that the style argument still will be the HighlightStyleExpression, but it'll only receive arguments related to a single annotation?

[rsimon]

Yes, exactly. The style argument is still an expression - fixed style or full style function. It would simply override whatever style is currently applied globally, for that specific annotation.

Once you call .setStyle(undefined, 'my-id'); you'd clear the override, and the annotation would go back to the global style.

[oleksandr-danylchenko]

Got it, thanks for the clarification. I believe it poses not that much of a risk as long as each .setStyle(style, id) will override the previously specified annotation-specific style. Anyway, it'll require a bit of the persistence trickery to preserve the "global" and the specific styles.

[rsimon]

Yep, any new call would override any previous ID-specific style for this ID.

Actually, my plan is to keep it simple internally. The global style would always be maintained separately, along with a Map<string, HighlightStyleExpression> to cache the currently applied custom styles.

[rsimon]

Another small change upcoming:

@oleksandr-danylchenko: you exposed the redraw function from the renderer in PR #104. I refactored this minimally: instead of forwarding the function through the TextAnnotator, I'd like to expose the renderer directly instead.

The reason is that I'll need more interaction with the renderer for the realtime awareness plugin (and perhaps other use cases in the future.) Specifically, I need an event callback after rendering. Seems cleaner to have these functions/events on the renderer, rather than routing them both through the TextAnnotator.

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

Lines 33 to 39 in cc4774d

	export interface TextAnnotator<I extends TextAnnotation = TextAnnotation, E extends unknown = TextAnnotation> extends Annotator<I, E> {
	element: HTMLElement;
	renderer: Renderer;
	setStyle(style?: HighlightStyleExpression, id?: string): void;

text-annotator-js/packages/text-annotator/src/rendering/base-renderer.ts

Lines 223 to 230 in cc4774d

	return {
	destroy,
	on,
	redraw,
	setStyle,
	setFilter,
	setVisible: painter.setVisible.bind(painter)
	}

[oleksandr-danylchenko]

@rsimon, I like that! Also, we may benefit from the "event callback after rendering" too. Currently, we rely on the onRecalculatePositions callback for positioning the notes - #130.