From 1dc2c67bbe31f86884829f55cbd2fd4b60c46f06 Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 26 Aug 2020 10:30:04 -0700 Subject: [PATCH 1/3] feat(highlights): Scroll to highlight annotation --- src/document/DocumentAnnotator.ts | 5 ++ .../__tests__/DocumentAnnotator-test.ts | 11 ++++- src/highlight/__tests__/highlightUtil-test.ts | 44 +++++++++++++++++ src/highlight/highlightUtil.ts | 49 ++++++++++++++++++- src/region/RegionAnnotation.tsx | 3 +- src/region/RegionRect.tsx | 3 +- src/region/regionUtil.ts | 8 +-- src/utils/util.ts | 18 +++++++ 8 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 src/highlight/__tests__/highlightUtil-test.ts diff --git a/src/document/DocumentAnnotator.ts b/src/document/DocumentAnnotator.ts index dbde2ed9e..4e472077a 100644 --- a/src/document/DocumentAnnotator.ts +++ b/src/document/DocumentAnnotator.ts @@ -1,5 +1,6 @@ import BaseAnnotator, { Options } from '../common/BaseAnnotator'; import BaseManager from '../common/BaseManager'; +import { centerHighlight, isHighlight } from '../highlight/highlightUtil'; import { centerRegion, isRegion, RegionManager } from '../region'; import { Event } from '../@types'; import { getAnnotation } from '../store/annotations'; @@ -145,6 +146,10 @@ export default class DocumentAnnotator extends BaseAnnotator { scrollToLocation(this.annotatedEl, annotationPageEl, { offsets: centerRegion(annotation.target.shape), }); + } else if (isHighlight(annotation)) { + scrollToLocation(this.annotatedEl, annotationPageEl, { + offsets: centerHighlight(annotation.target.shapes), + }); } } } diff --git a/src/document/__tests__/DocumentAnnotator-test.ts b/src/document/__tests__/DocumentAnnotator-test.ts index 56958be5b..2da1bcffa 100644 --- a/src/document/__tests__/DocumentAnnotator-test.ts +++ b/src/document/__tests__/DocumentAnnotator-test.ts @@ -3,6 +3,7 @@ import DocumentAnnotator from '../DocumentAnnotator'; import HighlightListener from '../../highlight/HighlightListener'; import RegionManager from '../../region/RegionManager'; import { Annotation, Event } from '../../@types'; +import { annotation as highlight } from '../../highlight/__mocks__/data'; import { annotations as regions } from '../../region/__mocks__/data'; import { fetchAnnotationsAction } from '../../store'; import { HighlightManager } from '../../highlight'; @@ -39,7 +40,7 @@ describe('DocumentAnnotator', () => { }; const payload = { - entries: regions as Annotation[], + entries: [...regions, highlight] as Annotation[], limit: 1000, next_marker: null, previous_marker: null, @@ -263,5 +264,13 @@ describe('DocumentAnnotator', () => { annotator.scrollToAnnotation(null); expect(scrollToLocation).not.toHaveBeenCalled(); }); + + test('should call scrollToLocation for highlight annotations', () => { + const parentEl = annotator.annotatedEl as HTMLElement; + const referenceEl = parentEl.querySelector('[data-page-number="1"]'); + + annotator.scrollToAnnotation('223'); + expect(scrollToLocation).toHaveBeenCalledWith(parentEl, referenceEl, { offsets: { x: 15, y: 10 } }); + }); }); }); diff --git a/src/highlight/__tests__/highlightUtil-test.ts b/src/highlight/__tests__/highlightUtil-test.ts new file mode 100644 index 000000000..de230dd17 --- /dev/null +++ b/src/highlight/__tests__/highlightUtil-test.ts @@ -0,0 +1,44 @@ +import { centerHighlight, getBoundingRect } from '../highlightUtil'; +import { Shape } from '../../utils/util'; + +const shape1: Shape = { + height: 10, + width: 10, + x: 0, + y: 0, +}; + +const shape2: Shape = { + height: 10, + width: 20, + x: 10, + y: 10, +}; + +describe('highlightUtil', () => { + describe('getBoundingRect()', () => { + test('should be the same rect for a single shape', () => { + expect(getBoundingRect([shape1])).toEqual(shape1); + }); + + test('should get the bounding rect for multiple shapes', () => { + expect(getBoundingRect([shape1, shape2])).toEqual({ + height: 20, + width: 30, + x: 0, + y: 0, + }); + }); + }); + + describe('centerHighlight()', () => { + test.each` + shapes | expectedCenter + ${[shape1]} | ${{ x: 5, y: 5 }} + ${[shape2]} | ${{ x: 20, y: 15 }} + ${[shape1, shape2]} | ${{ x: 15, y: 10 }} + `('should get the center of the highlight to be $expectedCenter', ({ shapes, expectedCenter }) => { + expect(centerHighlight(shapes)).toEqual(expectedCenter); + }); + }); +}); diff --git a/src/highlight/highlightUtil.ts b/src/highlight/highlightUtil.ts index 47e4528bb..568d93dc2 100644 --- a/src/highlight/highlightUtil.ts +++ b/src/highlight/highlightUtil.ts @@ -1,4 +1,51 @@ -import { Annotation, AnnotationHighlight, Type } from '../@types'; +import { Annotation, AnnotationHighlight, Position, Type } from '../@types'; +import { centerShape, Shape } from '../utils/util'; + +export const getBoundingRect = (shapes: Shape[]): Shape => { + let minX = Number.MAX_VALUE; + let minY = Number.MAX_VALUE; + let maxX = Number.MIN_VALUE; + let maxY = Number.MIN_VALUE; + + shapes.forEach(({ height, width, x, y }) => { + const x2 = x + width; + const y2 = y + height; + + if (x < minX) { + minX = x; + } + + if (y < minY) { + minY = y; + } + + if (x2 > maxX) { + maxX = x2; + } + + if (y2 > maxY) { + maxY = y2; + } + }); + + return { + x: minX, + y: minY, + width: maxX - minX, + height: maxY - minY, + }; +}; + +export const centerHighlight = (shapes: Shape[]): Position => { + const boundingShape = getBoundingRect(shapes); + const { x: shapeX, y: shapeY } = boundingShape; + const { x: centerX, y: centerY } = centerShape(boundingShape); + + return { + x: centerX + shapeX, + y: centerY + shapeY, + }; +}; export function isHighlight(annotation: Annotation): annotation is AnnotationHighlight { return annotation?.target?.type === Type.highlight; diff --git a/src/region/RegionAnnotation.tsx b/src/region/RegionAnnotation.tsx index e059d6f1a..f457bb709 100644 --- a/src/region/RegionAnnotation.tsx +++ b/src/region/RegionAnnotation.tsx @@ -4,7 +4,8 @@ import classNames from 'classnames'; import noop from 'lodash/noop'; import { getIsCurrentFileVersion } from '../store'; import { MOUSE_PRIMARY } from '../constants'; -import { Shape, styleShape } from './regionUtil'; +import { Shape } from '../utils/util'; +import { styleShape } from './regionUtil'; import './RegionAnnotation.scss'; type Props = { diff --git a/src/region/RegionRect.tsx b/src/region/RegionRect.tsx index 20de63394..3628b0b33 100644 --- a/src/region/RegionRect.tsx +++ b/src/region/RegionRect.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import classNames from 'classnames'; -import { Shape, styleShape } from './regionUtil'; +import { Shape } from '../utils/util'; +import { styleShape } from './regionUtil'; import './RegionRect.scss'; type Props = { diff --git a/src/region/regionUtil.ts b/src/region/regionUtil.ts index acec523ad..4f89558f6 100644 --- a/src/region/regionUtil.ts +++ b/src/region/regionUtil.ts @@ -1,6 +1,7 @@ import * as React from 'react'; import { Annotation, AnnotationRegion, Position } from '../@types'; import { invertYCoordinate, Point, rotatePoint, translatePoint } from './transformUtil'; +import { Shape } from '../utils/util'; // Possible rotation values as supplied by box-content-preview const ROTATION_ONCE_DEG = -90; @@ -15,13 +16,6 @@ export type Dimensions = { width: number; }; -export type Shape = { - height: number; - width: number; - x: number; - y: number; -}; - export type Translation = { dx?: number; dy?: number; diff --git a/src/utils/util.ts b/src/utils/util.ts index 31bfea3d3..b37a9ebc7 100644 --- a/src/utils/util.ts +++ b/src/utils/util.ts @@ -1,3 +1,21 @@ +import { Position } from '../@types'; + +export type Shape = { + height: number; + width: number; + x: number; + y: number; +}; + +export const centerShape = (shape: Shape): Position => { + const { height, width } = shape; + + return { + x: width / 2, + y: height / 2, + }; +}; + export function checkValue(value: number): boolean { return value >= 0 && value <= 100; // Values cannot be negative or larger than 100% } From 0ac729ca3c6761a1263be652f0701c0d403662ce Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 26 Aug 2020 12:31:33 -0700 Subject: [PATCH 2/3] chore: pr comments --- src/highlight/__tests__/highlightUtil-test.ts | 5 ++--- src/highlight/highlightUtil.ts | 12 +++++++++++- src/region/RegionAnnotation.tsx | 3 +-- src/region/RegionRect.tsx | 3 +-- src/region/regionUtil.ts | 3 ++- src/utils/util.ts | 18 ------------------ 6 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/highlight/__tests__/highlightUtil-test.ts b/src/highlight/__tests__/highlightUtil-test.ts index de230dd17..6911e80fc 100644 --- a/src/highlight/__tests__/highlightUtil-test.ts +++ b/src/highlight/__tests__/highlightUtil-test.ts @@ -1,14 +1,13 @@ import { centerHighlight, getBoundingRect } from '../highlightUtil'; -import { Shape } from '../../utils/util'; -const shape1: Shape = { +const shape1: Required = { height: 10, width: 10, x: 0, y: 0, }; -const shape2: Shape = { +const shape2: Required = { height: 10, width: 20, x: 10, diff --git a/src/highlight/highlightUtil.ts b/src/highlight/highlightUtil.ts index 568d93dc2..5ff43ef1a 100644 --- a/src/highlight/highlightUtil.ts +++ b/src/highlight/highlightUtil.ts @@ -1,5 +1,6 @@ import { Annotation, AnnotationHighlight, Position, Type } from '../@types'; -import { centerShape, Shape } from '../utils/util'; + +type Shape = Required; export const getBoundingRect = (shapes: Shape[]): Shape => { let minX = Number.MAX_VALUE; @@ -36,6 +37,15 @@ export const getBoundingRect = (shapes: Shape[]): Shape => { }; }; +const centerShape = (shape: Required): Position => { + const { height, width } = shape; + + return { + x: width / 2, + y: height / 2, + }; +}; + export const centerHighlight = (shapes: Shape[]): Position => { const boundingShape = getBoundingRect(shapes); const { x: shapeX, y: shapeY } = boundingShape; diff --git a/src/region/RegionAnnotation.tsx b/src/region/RegionAnnotation.tsx index f457bb709..058d45c5e 100644 --- a/src/region/RegionAnnotation.tsx +++ b/src/region/RegionAnnotation.tsx @@ -4,7 +4,6 @@ import classNames from 'classnames'; import noop from 'lodash/noop'; import { getIsCurrentFileVersion } from '../store'; import { MOUSE_PRIMARY } from '../constants'; -import { Shape } from '../utils/util'; import { styleShape } from './regionUtil'; import './RegionAnnotation.scss'; @@ -13,7 +12,7 @@ type Props = { className?: string; isActive?: boolean; onSelect?: (annotationId: string) => void; - shape: Shape; + shape: Required; }; export type RegionAnnotationRef = HTMLButtonElement; diff --git a/src/region/RegionRect.tsx b/src/region/RegionRect.tsx index 3628b0b33..a544b5ee1 100644 --- a/src/region/RegionRect.tsx +++ b/src/region/RegionRect.tsx @@ -1,13 +1,12 @@ import * as React from 'react'; import classNames from 'classnames'; -import { Shape } from '../utils/util'; import { styleShape } from './regionUtil'; import './RegionRect.scss'; type Props = { className?: string; isActive?: boolean; - shape?: Shape; + shape?: Required; }; export type RegionRectRef = HTMLDivElement; diff --git a/src/region/regionUtil.ts b/src/region/regionUtil.ts index 4f89558f6..056b49c28 100644 --- a/src/region/regionUtil.ts +++ b/src/region/regionUtil.ts @@ -1,7 +1,6 @@ import * as React from 'react'; import { Annotation, AnnotationRegion, Position } from '../@types'; import { invertYCoordinate, Point, rotatePoint, translatePoint } from './transformUtil'; -import { Shape } from '../utils/util'; // Possible rotation values as supplied by box-content-preview const ROTATION_ONCE_DEG = -90; @@ -16,6 +15,8 @@ export type Dimensions = { width: number; }; +type Shape = Required; + export type Translation = { dx?: number; dy?: number; diff --git a/src/utils/util.ts b/src/utils/util.ts index b37a9ebc7..31bfea3d3 100644 --- a/src/utils/util.ts +++ b/src/utils/util.ts @@ -1,21 +1,3 @@ -import { Position } from '../@types'; - -export type Shape = { - height: number; - width: number; - x: number; - y: number; -}; - -export const centerShape = (shape: Shape): Position => { - const { height, width } = shape; - - return { - x: width / 2, - y: height / 2, - }; -}; - export function checkValue(value: number): boolean { return value >= 0 && value <= 100; // Values cannot be negative or larger than 100% } From a4b4e095cb586b966a6dd6d110304c7584dc7aab Mon Sep 17 00:00:00 2001 From: Conrad Chan Date: Wed, 26 Aug 2020 14:35:47 -0700 Subject: [PATCH 3/3] chore: top level Shape type --- src/@types/model.ts | 2 ++ src/highlight/__tests__/highlightUtil-test.ts | 5 +++-- src/highlight/highlightUtil.ts | 6 ++---- src/region/RegionAnnotation.tsx | 3 ++- src/region/RegionRect.tsx | 3 ++- src/region/regionUtil.ts | 4 +--- src/store/selection/actions.ts | 9 +++++---- src/store/selection/types.ts | 6 +++--- 8 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/@types/model.ts b/src/@types/model.ts index 13ae24a76..d6742c03c 100644 --- a/src/@types/model.ts +++ b/src/@types/model.ts @@ -41,6 +41,8 @@ export interface Position { y: number; } +export type Shape = Required; + export interface User { id: string; login: string; diff --git a/src/highlight/__tests__/highlightUtil-test.ts b/src/highlight/__tests__/highlightUtil-test.ts index 6911e80fc..a7aea29ec 100644 --- a/src/highlight/__tests__/highlightUtil-test.ts +++ b/src/highlight/__tests__/highlightUtil-test.ts @@ -1,13 +1,14 @@ import { centerHighlight, getBoundingRect } from '../highlightUtil'; +import { Shape } from '../../@types'; -const shape1: Required = { +const shape1: Shape = { height: 10, width: 10, x: 0, y: 0, }; -const shape2: Required = { +const shape2: Shape = { height: 10, width: 20, x: 10, diff --git a/src/highlight/highlightUtil.ts b/src/highlight/highlightUtil.ts index 5ff43ef1a..d66b94bfb 100644 --- a/src/highlight/highlightUtil.ts +++ b/src/highlight/highlightUtil.ts @@ -1,6 +1,4 @@ -import { Annotation, AnnotationHighlight, Position, Type } from '../@types'; - -type Shape = Required; +import { Annotation, AnnotationHighlight, Position, Shape, Type } from '../@types'; export const getBoundingRect = (shapes: Shape[]): Shape => { let minX = Number.MAX_VALUE; @@ -37,7 +35,7 @@ export const getBoundingRect = (shapes: Shape[]): Shape => { }; }; -const centerShape = (shape: Required): Position => { +const centerShape = (shape: Shape): Position => { const { height, width } = shape; return { diff --git a/src/region/RegionAnnotation.tsx b/src/region/RegionAnnotation.tsx index 058d45c5e..0144fb00b 100644 --- a/src/region/RegionAnnotation.tsx +++ b/src/region/RegionAnnotation.tsx @@ -4,6 +4,7 @@ import classNames from 'classnames'; import noop from 'lodash/noop'; import { getIsCurrentFileVersion } from '../store'; import { MOUSE_PRIMARY } from '../constants'; +import { Shape } from '../@types'; import { styleShape } from './regionUtil'; import './RegionAnnotation.scss'; @@ -12,7 +13,7 @@ type Props = { className?: string; isActive?: boolean; onSelect?: (annotationId: string) => void; - shape: Required; + shape: Shape; }; export type RegionAnnotationRef = HTMLButtonElement; diff --git a/src/region/RegionRect.tsx b/src/region/RegionRect.tsx index a544b5ee1..1c1d2dde2 100644 --- a/src/region/RegionRect.tsx +++ b/src/region/RegionRect.tsx @@ -1,12 +1,13 @@ import * as React from 'react'; import classNames from 'classnames'; +import { Shape } from '../@types'; import { styleShape } from './regionUtil'; import './RegionRect.scss'; type Props = { className?: string; isActive?: boolean; - shape?: Required; + shape?: Shape; }; export type RegionRectRef = HTMLDivElement; diff --git a/src/region/regionUtil.ts b/src/region/regionUtil.ts index 056b49c28..65243a11e 100644 --- a/src/region/regionUtil.ts +++ b/src/region/regionUtil.ts @@ -1,5 +1,5 @@ import * as React from 'react'; -import { Annotation, AnnotationRegion, Position } from '../@types'; +import { Annotation, AnnotationRegion, Position, Shape } from '../@types'; import { invertYCoordinate, Point, rotatePoint, translatePoint } from './transformUtil'; // Possible rotation values as supplied by box-content-preview @@ -15,8 +15,6 @@ export type Dimensions = { width: number; }; -type Shape = Required; - export type Translation = { dx?: number; dy?: number; diff --git a/src/store/selection/actions.ts b/src/store/selection/actions.ts index 672a345cb..fe7a41218 100644 --- a/src/store/selection/actions.ts +++ b/src/store/selection/actions.ts @@ -1,5 +1,6 @@ import { createAction } from '@reduxjs/toolkit'; -import { DOMRectMini, SelectionItem } from './types'; +import { SelectionItem } from './types'; +import { Shape } from '../../@types'; export type SelectionArg = { location: number; @@ -10,7 +11,7 @@ type Payload = { payload: SelectionItem | null; }; -export const getDOMRectMini = ({ height, width, x, y }: DOMRect): DOMRectMini => ({ +export const getShape = ({ height, width, x, y }: DOMRect): Shape => ({ height, width, x, @@ -30,9 +31,9 @@ export const setSelectionAction = createAction( return { payload: { - boundingRect: getDOMRectMini(range.getBoundingClientRect()), + boundingRect: getShape(range.getBoundingClientRect()), location, - rects: Array.from(range.getClientRects()).map(getDOMRectMini), + rects: Array.from(range.getClientRects()).map(getShape), }, }; }, diff --git a/src/store/selection/types.ts b/src/store/selection/types.ts index 4250818bf..34fb59df3 100644 --- a/src/store/selection/types.ts +++ b/src/store/selection/types.ts @@ -1,11 +1,11 @@ -export type DOMRectMini = Required; +import { Shape } from '../../@types'; export type SelectionState = { selection: SelectionItem | null; }; export type SelectionItem = { - boundingRect: DOMRectMini; + boundingRect: Shape; location: number; - rects: Array; + rects: Array; };