Skip to content

Commit 775d12c

Browse files
jstoffanjstoffan
authored
fix(regions): Use relative position and size to support dynamic reps (#500)
1 parent f4553f7 commit 775d12c

24 files changed

+227
-251
lines changed

‎src/common/BaseAnnotator.scss‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@
2020
@import '~box-ui-elements/es/styles/common/forms';
2121
@import '~box-ui-elements/es/styles/common/buttons';
2222

23-
.ba-Layer {
24-
@include box-sizing;
25-
@include common-typography;
26-
}
27-
2823
&.is-hidden {
2924
.ba-Layer {
3025
display: none;
3126
}
3227
}
3328
}
29+
30+
.ba-Layer {
31+
@include box-sizing;
32+
@include common-typography;
33+
}

‎src/common/BaseManager.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ export type Options = {
99

1010
export type Props = {
1111
intl: IntlShape;
12-
scale: number;
1312
store: Store;
1413
};
1514

‎src/common/__tests__/BaseAnnotator-test.ts‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import * as store from '../../store';
2+
import APIFactory from '../../api';
23
import BaseAnnotator from '../BaseAnnotator';
34
import { ANNOTATOR_EVENT } from '../../constants';
45
import { Event, LegacyEvent } from '../../@types';
56
import { Mode } from '../../store/common';
6-
import APIFactory from '../../api';
77

8-
jest.mock('../EventEmitter');
98
jest.mock('../../api');
109
jest.mock('../../store', () => ({
1110
createStore: jest.fn(() => ({ dispatch: jest.fn() })),
@@ -128,6 +127,29 @@ describe('BaseAnnotator', () => {
128127
});
129128
});
130129

130+
describe('event handlers', () => {
131+
beforeEach(() => {
132+
jest.spyOn(annotator, 'init');
133+
jest.spyOn(annotator, 'removeAnnotation');
134+
jest.spyOn(annotator, 'setActiveId');
135+
jest.spyOn(annotator, 'setVisibility');
136+
});
137+
138+
test('should call their underlying methods', () => {
139+
annotator.emit(LegacyEvent.SCALE, { scale: 1 });
140+
expect(annotator.init).toHaveBeenCalledWith(1);
141+
142+
annotator.emit(Event.ACTIVE_SET, 12345);
143+
expect(annotator.setActiveId).toHaveBeenCalledWith(12345);
144+
145+
annotator.emit(Event.ANNOTATION_REMOVE, 12345);
146+
expect(annotator.removeAnnotation).toHaveBeenCalledWith(12345);
147+
148+
annotator.emit(Event.VISIBLE_SET, true);
149+
expect(annotator.setVisibility).toHaveBeenCalledWith(true);
150+
});
151+
});
152+
131153
describe('scrollToAnnotation()', () => {
132154
test('should exist', () => {
133155
expect(annotator.scrollToAnnotation).toBeTruthy();

‎src/document/DocumentAnnotator.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ export default class DocumentAnnotator extends BaseAnnotator {
7979
pageManagers.forEach(manager =>
8080
manager.render({
8181
intl: this.intl,
82-
scale: this.scale,
8382
store: this.store,
8483
}),
8584
);
@@ -112,10 +111,10 @@ export default class DocumentAnnotator extends BaseAnnotator {
112111
}
113112

114113
const canSmoothScroll = 'scrollBehavior' in this.annotatedEl.style;
115-
const parentCenterX = this.annotatedEl.clientWidth / 2;
116-
const parentCenterY = this.annotatedEl.clientHeight / 2;
117-
const offsetCenterX = Math.round(offsetX * this.scale);
118-
const offsetCenterY = Math.round(offsetY * this.scale);
114+
const parentCenterX = Math.round(this.annotatedEl.clientWidth / 2);
115+
const parentCenterY = Math.round(this.annotatedEl.clientHeight / 2);
116+
const offsetCenterX = Math.round(pageEl.clientWidth * (offsetX / 100));
117+
const offsetCenterY = Math.round(pageEl.clientHeight * (offsetY / 100));
119118
const offsetScrollLeft = pageEl.offsetLeft - parentCenterX + offsetCenterX;
120119
const offsetScrollTop = pageEl.offsetTop - parentCenterY + offsetCenterY;
121120
const scrollLeft = Math.max(0, Math.min(offsetScrollLeft, this.annotatedEl.scrollWidth));

‎src/document/__tests__/DocumentAnnotator-test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,6 @@ describe('DocumentAnnotator', () => {
164164
expect(annotator.getPageNumber).toHaveBeenCalledWith(pageEl);
165165
expect(mockManager.render).toHaveBeenCalledWith({
166166
intl: annotator.intl,
167-
scale: 1,
168167
store: expect.any(Object),
169168
});
170169
});
@@ -201,6 +200,8 @@ describe('DocumentAnnotator', () => {
201200
const getPageMock = (pageNumber = 1): HTMLElement => {
202201
const page = document.createElement('div');
203202

203+
Object.defineProperty(page, 'clientHeight', { configurable: true, value: 100 });
204+
Object.defineProperty(page, 'clientWidth', { configurable: true, value: 100 });
204205
Object.defineProperty(page, 'offsetLeft', { configurable: true, value: 0 });
205206
Object.defineProperty(page, 'offsetTop', { configurable: true, value: (pageNumber - 1) * 100 }); // 100 pixels per page
206207

‎src/region/RegionAnnotation.scss‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
.ba-RegionAnnotation {
55
@include ba-RegionRect;
66

7-
margin: 0;
8-
padding: 0;
97
background: none;
108
border: none;
119
box-shadow: none;

‎src/region/RegionAnnotations.tsx‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import RegionRect, { RegionRectRef } from './RegionRect';
66
import { AnnotationRegion, Rect } from '../@types';
77
import { CreateArg } from './actions';
88
import { CreatorItem, CreatorStatus } from '../store/creator';
9-
import { scaleShape } from './regionUtil';
109
import './RegionAnnotations.scss';
1110

1211
type Props = {
@@ -16,7 +15,6 @@ type Props = {
1615
isCreating: boolean;
1716
message: string;
1817
page: number;
19-
scale: number;
2018
setActiveAnnotationId: (annotationId: string | null) => void;
2119
setMessage: (message: string) => void;
2220
setStaged: (staged: CreatorItem | null) => void;
@@ -33,27 +31,21 @@ export default class RegionAnnotations extends React.PureComponent<Props, State>
3331
static defaultProps = {
3432
annotations: [],
3533
isCreating: false,
36-
scale: 1,
3734
};
3835

3936
state: State = {};
4037

41-
setStatus(status: CreatorStatus): void {
42-
const { setStatus } = this.props;
43-
setStatus(status);
44-
}
45-
4638
handleAnnotationActive = (annotationId: string | null): void => {
4739
const { setActiveAnnotationId } = this.props;
4840

4941
setActiveAnnotationId(annotationId);
5042
};
5143

5244
handleCancel = (): void => {
53-
const { setMessage, setStaged } = this.props;
45+
const { setMessage, setStaged, setStatus } = this.props;
5446
setMessage('');
5547
setStaged(null);
56-
this.setStatus(CreatorStatus.init);
48+
setStatus(CreatorStatus.init);
5749
};
5850

5951
handleChange = (text?: string): void => {
@@ -62,15 +54,15 @@ export default class RegionAnnotations extends React.PureComponent<Props, State>
6254
};
6355

6456
handleStart = (): void => {
65-
const { setStaged } = this.props;
57+
const { setStaged, setStatus } = this.props;
6658
setStaged(null);
67-
this.setStatus(CreatorStatus.init);
59+
setStatus(CreatorStatus.init);
6860
};
6961

7062
handleStop = (shape: Rect): void => {
71-
const { page, scale, setStaged } = this.props;
72-
setStaged({ location: page, shape: scaleShape(shape, scale, true) });
73-
this.setStatus(CreatorStatus.staged);
63+
const { page, setStaged, setStatus } = this.props;
64+
setStaged({ location: page, shape });
65+
setStatus(CreatorStatus.staged);
7466
};
7567

7668
handleSubmit = (): void => {
@@ -88,7 +80,7 @@ export default class RegionAnnotations extends React.PureComponent<Props, State>
8880
};
8981

9082
render(): JSX.Element {
91-
const { activeAnnotationId, annotations, isCreating, message, scale, staged, status } = this.props;
83+
const { activeAnnotationId, annotations, isCreating, message, staged, status } = this.props;
9284
const { rectRef } = this.state;
9385
const canReply = status !== CreatorStatus.init;
9486
const isPending = status === CreatorStatus.pending;
@@ -101,7 +93,6 @@ export default class RegionAnnotations extends React.PureComponent<Props, State>
10193
annotations={annotations}
10294
className="ba-RegionAnnotations-list"
10395
onSelect={this.handleAnnotationActive}
104-
scale={scale}
10596
/>
10697

10798
{/* Layer 2: Drawn (unsaved) incomplete annotation target, if any */}
@@ -116,7 +107,7 @@ export default class RegionAnnotations extends React.PureComponent<Props, State>
116107
{/* Layer 3a: Staged (unsaved) annotation target, if any */}
117108
{isCreating && staged && (
118109
<div className="ba-RegionAnnotations-target">
119-
<RegionRect ref={this.setRectRef} isActive shape={scaleShape(staged.shape, scale)} />
110+
<RegionRect ref={this.setRectRef} isActive shape={staged.shape} />
120111
</div>
121112
)}
122113

‎src/region/RegionCreator.scss‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ $region_cursor_32_3x: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGAAAABgCAY
88
}
99

1010
.ba-RegionCreator-rect {
11-
will-change: height, transform, width;
11+
transform: translateZ(0);
1212
}

‎src/region/RegionCreator.tsx‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export default function RegionCreator({ className, onStart, onStop }: Props): JS
4949
const { current: x2 } = positionX2Ref;
5050
const { current: y2 } = positionY2Ref;
5151

52-
if (!creatorEl || !x1 || !x2 || !y1 || !y2) {
52+
if (!creatorEl || x1 === null || x2 === null || y1 === null || y2 === null) {
5353
return null;
5454
}
5555

@@ -63,12 +63,16 @@ export default function RegionCreator({ className, onStart, onStop }: Props): JS
6363
const targetX = Math.min(Math.max(MIN_X, x2 > x1 ? x2 : x1), MAX_X);
6464
const targetY = Math.min(Math.max(MIN_Y, y2 > y1 ? y2 : y1), MAX_Y);
6565

66+
// Derive the shape height/width from the two coordinates
67+
const shapeHeight = Math.max(MIN_SIZE, targetY - originY);
68+
const shapeWidth = Math.max(MIN_SIZE, targetX - originX);
69+
6670
return {
67-
height: Math.max(MIN_SIZE, targetY - originY),
71+
height: (shapeHeight / height) * 100,
6872
type: 'rect',
69-
width: Math.max(MIN_SIZE, targetX - originX),
70-
x: originX,
71-
y: originY,
73+
width: (shapeWidth / width) * 100,
74+
x: (originX / width) * 100,
75+
y: (originY / height) * 100,
7276
};
7377
};
7478

@@ -164,7 +168,7 @@ export default function RegionCreator({ className, onStart, onStop }: Props): JS
164168

165169
if (isDirty && regionRect && shape) {
166170
// Directly set the style attribute
167-
Object.assign(regionRect.style, styleShape(shape, true));
171+
Object.assign(regionRect.style, styleShape(shape));
168172

169173
// Reset the dirty state to avoid multiple renders
170174
regionDirtyRef.current = false;

‎src/region/RegionList.tsx‎

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,36 @@ import noop from 'lodash/noop';
44
import RegionAnnotation from './RegionAnnotation';
55
import useOutsideEvent from '../common/useOutsideEvent';
66
import { AnnotationRegion } from '../@types';
7-
import { scaleShape } from './regionUtil';
87

98
export type Props = {
109
activeId?: string | null;
1110
annotations: AnnotationRegion[];
1211
className?: string;
1312
onSelect?: (annotationId: string | null) => void;
14-
scale: number;
1513
};
1614

17-
export function RegionList({ activeId, annotations, className, onSelect = noop, scale }: Props): JSX.Element {
15+
export function checkValue(value: number): boolean {
16+
return value >= 0 && value <= 100; // Values cannot be negative or larger than 100%
17+
}
18+
19+
export function filterRegion({ target }: AnnotationRegion): boolean {
20+
const { shape } = target;
21+
const { height, width, x, y } = shape;
22+
23+
return checkValue(height) && checkValue(width) && checkValue(x) && checkValue(y);
24+
}
25+
26+
export function sortRegion({ target: targetA }: AnnotationRegion, { target: targetB }: AnnotationRegion): number {
27+
const { shape: shapeA } = targetA;
28+
const { shape: shapeB } = targetB;
29+
30+
// Render the smallest annotations last to ensure they are always clickable
31+
return shapeA.height * shapeA.width > shapeB.height * shapeB.width ? -1 : 1;
32+
}
33+
34+
export function RegionList({ activeId, annotations, className, onSelect = noop }: Props): JSX.Element {
1835
const [isListening, setIsListening] = React.useState(true);
1936
const rootElRef = React.createRef<HTMLDivElement>();
20-
const sortedAnnotations = annotations.sort(({ target: targetA }, { target: targetB }) => {
21-
const { shape: shapeA } = targetA;
22-
const { shape: shapeB } = targetB;
23-
24-
// Render the smallest annotations last to ensure they are always clickable
25-
return shapeA.height * shapeA.width > shapeB.height * shapeB.width ? -1 : 1;
26-
});
2737

2838
// Document-level event handlers for focus and pointer control
2939
useOutsideEvent('click', rootElRef, (): void => onSelect(null));
@@ -32,15 +42,18 @@ export function RegionList({ activeId, annotations, className, onSelect = noop,
3242

3343
return (
3444
<div ref={rootElRef} className={classNames(className, { 'is-listening': isListening })}>
35-
{sortedAnnotations.map(({ id, target }) => (
36-
<RegionAnnotation
37-
key={id}
38-
annotationId={id}
39-
isActive={activeId === id}
40-
onSelect={onSelect}
41-
shape={scaleShape(target.shape, scale)}
42-
/>
43-
))}
45+
{annotations
46+
.filter(filterRegion)
47+
.sort(sortRegion)
48+
.map(({ id, target }) => (
49+
<RegionAnnotation
50+
key={id}
51+
annotationId={id}
52+
isActive={activeId === id}
53+
onSelect={onSelect}
54+
shape={target.shape}
55+
/>
56+
))}
4457
</div>
4558
);
4659
}

0 commit comments

Comments
 (0)