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

#127 Made annotatingEnabled property reactive #128

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
edd2e4a
Added `annotatingEnabled` flag updating support
oleksandr-danylchenko Aug 1, 2024
2c5b4da
Added `annotatingEnabled` update from the react wrapper
oleksandr-danylchenko Aug 1, 2024
2c9fb93
Added undefined `annotatingEnabled` prop handling
oleksandr-danylchenko Aug 28, 2024
e6ef046
Added cleanable `debounce` function
oleksandr-danylchenko Aug 28, 2024
da40d2c
Added selection state cleanup on annotating disabling
oleksandr-danylchenko Aug 28, 2024
5baebe7
Decreased debounce timeout to 10ms
oleksandr-danylchenko Aug 28, 2024
6b19a83
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Aug 30, 2024
3d7e09c
Added strict context binding to the debounced function
oleksandr-danylchenko Aug 30, 2024
c0dc156
Added context-unaware `debounce` wrapper
oleksandr-danylchenko Aug 30, 2024
822ee71
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Sep 2, 2024
0b4ebc9
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Sep 3, 2024
21249d7
Replaced custom `debounce` method with the library one
oleksandr-danylchenko Sep 10, 2024
ffd4564
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Sep 10, 2024
a4079b0
Decreased debounce timeout to 10ms
oleksandr-danylchenko Sep 10, 2024
06adc93
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Sep 26, 2024
eb65af5
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Sep 30, 2024
ac26a81
Merged `main` into `#127-annotating-enabled-reactive`
oleksandr-danylchenko Sep 30, 2024
befcf51
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Oct 7, 2024
8084534
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Oct 29, 2024
b3e9a54
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Nov 4, 2024
2ee7384
Removed `isContextMenuOpen` usage
oleksandr-danylchenko Nov 4, 2024
7a44635
Added `isLeftClick` reset on annotating disabling
oleksandr-danylchenko Nov 4, 2024
843c284
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Nov 11, 2024
4249cc1
Added `annotatingEnabled` in opts handling
oleksandr-danylchenko Nov 11, 2024
3ca9ab6
Merge branch 'main' into #127-annotating-enabled-reactive
oleksandr-danylchenko Dec 16, 2024
b2bbebc
Comment formatting
oleksandr-danylchenko Dec 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions packages/text-annotator-react/src/TextAnnotator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ export const TextAnnotator = <E extends unknown>(props: TextAnnotatorProps<E>) =
const el = useRef<HTMLDivElement>(null);

const { className, children, ...opts } = props;
const { style, filter, user } = opts;

const { style, filter, user, annotatingEnabled } = opts;

const { anno, setAnno } = useContext(AnnotoriousContext);

Expand All @@ -48,10 +48,12 @@ export const TextAnnotator = <E extends unknown>(props: TextAnnotatorProps<E>) =

useEffect(() => anno?.setUser(user), [anno, user]);

useEffect(() => anno?.setAnnotatingEnabled(annotatingEnabled), [anno, annotatingEnabled]);

return (
<div ref={el} className={className}>
{children}
</div>
)
);

}
3 changes: 2 additions & 1 deletion packages/text-annotator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@
"dependencies": {
"@annotorious/core": "^3.0.0",
"colord": "^2.9.3",
"debounce": "^2.1.0",
"dequal": "^2.0.3",
"rbush": "^4.0.1",
"uuid": "^10.0.0"
}
}
}
45 changes: 33 additions & 12 deletions packages/text-annotator/src/SelectionHandler.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Filter, Origin, type User } from '@annotorious/core';
import { v4 as uuidv4 } from 'uuid';

import type { TextAnnotatorState } from './state';
import type { TextAnnotationTarget } from './model';
import {
Expand All @@ -11,10 +12,9 @@ import {
NOT_ANNOTATABLE_SELECTOR
} from './utils';

export const SelectionHandler = (
export const createSelectionHandler = (
container: HTMLElement,
state: TextAnnotatorState,
annotatingEnabled: boolean,
offsetReferenceSelector?: string
) => {

Expand All @@ -26,6 +26,18 @@ export const SelectionHandler = (

const setFilter = (filter?: Filter) => currentFilter = filter;

let currentAnnotatingEnabled = true;

const setAnnotatingEnabled = (enabled: boolean) => {
currentAnnotatingEnabled = enabled;
onSelectionChange.clear();

if (!enabled) {
currentTarget = undefined;
lastPointerDown = undefined;
}
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must clean the onSelectionChange debounced call to prevent it from running on obsolete values from the closure. Otherwise, the selectionchange event will still get processed in the deferred call, even though the currentAnnotatingEnabled became false.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how crucial that is though. Would annotatingEnabled get called during a selection in practice? The normal use cases I would see is:

  • Setting it once on page init (because the user is getting a page served in "read-only mode")
  • "Locking a page" interactively, by the user clicking a "Lock" button. (In which case there wouldn't be a selection.)


const { store, selection } = state;

let currentTarget: TextAnnotationTarget | undefined;
Expand All @@ -35,6 +47,8 @@ export const SelectionHandler = (
let lastPointerDown: PointerEvent | undefined;

const onSelectStart = (evt: PointerEvent) => {
if (!currentAnnotatingEnabled) return;

if (!isLeftClick) return;

// Make sure we don't listen to selection changes that were
Expand All @@ -54,10 +68,11 @@ export const SelectionHandler = (
}
}

if (annotatingEnabled)
container.addEventListener('selectstart', onSelectStart);
container.addEventListener('selectstart', onSelectStart);

const onSelectionChange = debounce((evt: PointerEvent) => {
if (!currentAnnotatingEnabled) return;

const sel = document.getSelection();

// This is to handle cases where the selection is "hijacked" by another element
Expand All @@ -78,7 +93,7 @@ export const SelectionHandler = (

const selectionRange = sel.getRangeAt(0);

// The selection should be captured only within the annotatable container
// The selection should be captured only within the annotatable container
const containedRange = trimRangeToContainer(selectionRange, container);
if (isWhitespaceOrEmpty(containedRange)) return;

Expand All @@ -87,7 +102,7 @@ export const SelectionHandler = (
const hasChanged =
annotatableRanges.length !== currentTarget.selector.length ||
annotatableRanges.some((r, i) => r.toString() !== currentTarget.selector[i]?.quote);

if (!hasChanged) return;

currentTarget = {
Expand All @@ -101,7 +116,7 @@ export const SelectionHandler = (
} else {
// Proper lifecycle management: clear selection first...
selection.clear();

// ...then add annotation to store...
store.addAnnotation({
id: currentTarget.annotation,
Expand All @@ -113,10 +128,9 @@ export const SelectionHandler = (
// select events don't have offsetX/offsetY - reuse last up/down)
selection.userSelect(currentTarget.annotation, lastPointerDown);
}
})
});

if (annotatingEnabled)
document.addEventListener('selectionchange', onSelectionChange);
document.addEventListener('selectionchange', onSelectionChange);

// Select events don't carry information about the mouse button
// Therefore, to prevent right-click selection, we need to listen
Expand Down Expand Up @@ -169,16 +183,23 @@ export const SelectionHandler = (
document.addEventListener('pointerup', onPointerUp);

const destroy = () => {
currentTarget = undefined;
lastPointerDown = undefined;

onSelectionChange.clear();

container.removeEventListener('selectstart', onSelectStart);
document.removeEventListener('selectionchange', onSelectionChange);
document.removeEventListener('selectionchange', onSelectionChange);

document.removeEventListener('pointerdown', onPointerDown);
document.removeEventListener('pointerup', onPointerUp);
}

return {
destroy,
setFilter,
setUser
setUser,
setAnnotatingEnabled
}

}
33 changes: 27 additions & 6 deletions packages/text-annotator/src/TextAnnotator.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
import { createAnonymousGuest, createLifecycleObserver, createBaseAnnotator, Filter, createUndoStack } from '@annotorious/core';
import {
createAnonymousGuest,
createLifecycleObserver,
createBaseAnnotator,
Filter,
createUndoStack
} from '@annotorious/core';
import type { Annotator, User, PresenceProvider } from '@annotorious/core';
import { createCanvasRenderer, createHighlightsRenderer, createSpansRenderer, type HighlightStyleExpression } from './highlight';
import {
createCanvasRenderer,
createHighlightsRenderer,
createSpansRenderer,
type HighlightStyleExpression
} from './highlight';
import { createPresencePainter } from './presence';
import { scrollIntoView } from './api';
import { TextAnnotationStore, TextAnnotatorState, createTextAnnotatorState } from './state';
import type { TextAnnotation } from './model';
import { cancelSingleClickEvents } from './utils';
import { fillDefaults, type RendererType, type TextAnnotatorOptions } from './TextAnnotatorOptions';
import { SelectionHandler } from './SelectionHandler';
import { createSelectionHandler } from './SelectionHandler';

import './TextAnnotator.css';

Expand All @@ -22,6 +33,8 @@ export interface TextAnnotator<E extends unknown = TextAnnotation> extends Annot
// Returns true if successful (or false if the annotation is not currently rendered)
scrollIntoView(annotation: TextAnnotation): boolean;

setAnnotatingEnabled: (enabled: boolean) => void;

state: TextAnnotatorState;

}
Expand Down Expand Up @@ -60,8 +73,8 @@ export const createTextAnnotator = <E extends unknown = TextAnnotation>(

const highlightRenderer =
useRenderer === 'SPANS' ? createSpansRenderer(container, state, viewport) :
useRenderer === 'CSS_HIGHLIGHTS' ? createHighlightsRenderer(container, state, viewport) :
useRenderer === 'CANVAS' ? createCanvasRenderer(container, state, viewport) : undefined;
useRenderer === 'CSS_HIGHLIGHTS' ? createHighlightsRenderer(container, state, viewport) :
useRenderer === 'CANVAS' ? createCanvasRenderer(container, state, viewport) : undefined;

if (!highlightRenderer)
throw `Unknown renderer implementation: ${useRenderer}`;
Expand All @@ -71,8 +84,9 @@ export const createTextAnnotator = <E extends unknown = TextAnnotation>(
if (opts.style)
highlightRenderer.setStyle(opts.style);

const selectionHandler = SelectionHandler(container, state, opts.annotatingEnabled, opts.offsetReferenceSelector);
const selectionHandler = createSelectionHandler(container, state, opts.offsetReferenceSelector);
selectionHandler.setUser(currentUser);
selectionHandler.setAnnotatingEnabled(opts.annotatingEnabled);

/*************************/
/* External API */
Expand All @@ -83,6 +97,12 @@ export const createTextAnnotator = <E extends unknown = TextAnnotation>(

const getUser = () => currentUser;

const setAnnotatingEnabled = (enabled?: boolean) => {
selectionHandler.setAnnotatingEnabled(
enabled === undefined ? true : enabled
);
};

const setFilter = (filter?: Filter) => {
highlightRenderer.setFilter(filter);
selectionHandler.setFilter(filter);
Expand Down Expand Up @@ -127,6 +147,7 @@ export const createTextAnnotator = <E extends unknown = TextAnnotation>(
destroy,
element: container,
getUser,
setAnnotatingEnabled,
setFilter,
setStyle,
setUser,
Expand Down
7 changes: 4 additions & 3 deletions packages/text-annotator/src/highlight/baseRenderer.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import type { Filter, ViewportState } from '@annotorious/core';

import type { TextAnnotatorState } from '../state';
import { debounce } from '../utils';
import { ViewportBounds, getViewportBounds, trackViewport } from './viewport';
import type { HighlightPainter } from './HighlightPainter';
import type { Highlight } from './Highlight';
import type { HighlightStyleExpression } from './HighlightStyle';
import { debounce } from '../utils';

export interface RendererImplementation {

Expand Down Expand Up @@ -135,8 +136,7 @@ export const createBaseRenderer = (
const onResize = debounce(() => {
store.recalculatePositions();

if (currentPainter)
currentPainter.reset();
currentPainter?.reset();

redraw();
});
Expand Down Expand Up @@ -169,6 +169,7 @@ export const createBaseRenderer = (

document.removeEventListener('scroll', onScroll);

onResize.clear();
window.removeEventListener('resize', onResize);
resizeObserver.disconnect();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import type { ViewportState } from '@annotorious/core';

import type { TextAnnotatorState } from '../../state';
import { debounce } from '../../utils';
import type { ViewportBounds } from '../viewport';
import type { HighlightStyle } from '../HighlightStyle';
import { DEFAULT_SELECTED_STYLE, DEFAULT_STYLE, HighlightStyleExpression } from '../HighlightStyle';
import type { HighlightPainter } from '../HighlightPainter';
import { createBaseRenderer, type RendererImplementation } from '../baseRenderer';
import type { Highlight } from '../Highlight';
import { debounce } from '../../utils';

import './canvasRenderer.css';

Expand Down Expand Up @@ -116,9 +117,7 @@ const createRenderer = (container: HTMLElement): RendererImplementation => {
});
});

const onResize = debounce(() => {
resetCanvas(canvas);
});
const onResize = debounce(() => resetCanvas(canvas));

window.addEventListener('resize', onResize);

Expand All @@ -129,6 +128,7 @@ const createRenderer = (container: HTMLElement): RendererImplementation => {
const destroy = () => {
canvas.remove();

onResize.clear();
window.removeEventListener('resize', onResize);
}

Expand Down
26 changes: 20 additions & 6 deletions packages/text-annotator/src/utils/debounce.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
export const debounce = <T extends (...args: any[]) => void>(func: T, delay = 10): T => {
let timeoutId: ReturnType<typeof setTimeout>;
import libDebounce from 'debounce';

return ((...args: any[]) => {
clearTimeout(timeoutId);
timeoutId = setTimeout(() => func.apply(this, args), delay);
}) as T;
/**
* Wraps the `debounce` function from the `debounce` package
* to make it context-agnostic.
* Otherwise, we won't be able to use it in multiple event listeners simultaneously,
* like `window.onresize` and `ResizeObserver`.
* @see https://github.com/sindresorhus/debounce/issues/8#issuecomment-2321341074
*/
export const debounce: typeof libDebounce = (function_, wait = 10, options) => {
oleksandr-danylchenko marked this conversation as resolved.
Show resolved Hide resolved
const fn = libDebounce(function_, wait, options);

const boundFn = fn.bind(undefined);

Object.getOwnPropertyNames(fn).forEach(
prop => Object.defineProperty(boundFn, prop, Object.getOwnPropertyDescriptor(fn, prop))
);
const proto = Object.getPrototypeOf(fn);
Object.setPrototypeOf(boundFn, proto);

return boundFn;
}
2 changes: 1 addition & 1 deletion packages/text-annotator/src/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
export * from './cancelSingleClickEvents';
export * from './debounce';
export * from './getAnnotatableFragment';
export * from './getClientRectsPonyfill';
export * from './getQuoteContext';
Expand All @@ -12,4 +11,5 @@ export * from './reviveSelector';
export * from './reviveTarget';
export * from './splitAnnotatableRanges';
export * from './trimRangeToContainer';
export * from './debounce';