Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions src/drawing/DrawingAnnotations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type Props = {
location: number;
redoDrawingPathGroup: () => void;
resetDrawing: () => void;
rotation: number;
setActiveAnnotationId: (annotationId: string | null) => void;
setReferenceId: (uuid: string) => void;
setStaged: (staged: CreatorItemDrawing | null) => void;
Expand All @@ -47,6 +48,7 @@ const DrawingAnnotations = (props: Props): JSX.Element => {
location,
redoDrawingPathGroup,
resetDrawing,
rotation,
setActiveAnnotationId,
setReferenceId,
setStaged,
Expand Down Expand Up @@ -170,6 +172,7 @@ const DrawingAnnotations = (props: Props): JSX.Element => {
color={color}
onStart={handleStart}
onStop={handleStop}
rotation={rotation}
/>
)}

Expand Down
3 changes: 3 additions & 0 deletions src/drawing/DrawingAnnotationsContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
getAnnotationsForLocation,
getColor,
getCreatorStatus,
getRotation,
Mode,
setActiveAnnotationIdAction,
setReferenceIdAction,
Expand All @@ -34,6 +35,7 @@ export type Props = {
canShowPopupToolbar: boolean;
drawnPathGroups: Array<PathGroup>;
isCreating: boolean;
rotation: number;
stashedPathGroups: Array<PathGroup>;
};

Expand All @@ -47,6 +49,7 @@ export const mapStateToProps = (state: AppState, { location }: { location: numbe
canShowPopupToolbar: creatorStatus === CreatorStatus.started,
drawnPathGroups: getDrawingDrawnPathGroupsForLocation(state, location),
isCreating: getAnnotationMode(state) === Mode.DRAWING && creatorStatus !== CreatorStatus.pending,
rotation: getRotation(state),
stashedPathGroups: getStashedDrawnPathGroupsForLocation(state, location),
};
};
Expand Down
25 changes: 9 additions & 16 deletions src/drawing/DrawingCreator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import DrawingSVG, { DrawingSVGRef } from './DrawingSVG';
import PointerCapture, { PointerCaptureRef, Status as DrawingStatus } from '../components/PointerCapture';
import { getDrawingCursor } from './DrawingCursor';
import { getPathCommands } from './drawingUtil';
import { getElementLocalPosition } from '../utils/rotate';
import { PathGroup, Position } from '../@types';
import './DrawingCreator.scss';

Expand All @@ -15,6 +16,7 @@ export type Props = {
color?: string;
onStart: () => void;
onStop: (pathGroup: PathGroup) => void;
rotation?: number;
size?: number;
};

Expand All @@ -26,6 +28,7 @@ export default function DrawingCreator({
color = defaultStrokeColor,
onStart,
onStop,
rotation = 0,
size = defaultStrokeSize,
}: Props): JSX.Element {
const [drawingStatus, setDrawingStatus] = React.useState<DrawingStatus>(DrawingStatus.init);
Expand All @@ -45,7 +48,9 @@ export default function DrawingCreator({
return [];
}

const { height, width } = creatorEl.getBoundingClientRect();
// Get the element's dimensions (before any rotation is applied)
const width = creatorEl.offsetWidth;
const height = creatorEl.offsetHeight;
const { size: minSize } = stroke;
const MAX_X = width - minSize;
const MAX_Y = height - minSize;
Expand All @@ -56,21 +61,9 @@ export default function DrawingCreator({
}));
}, [stroke]);

const getPosition = (x: number, y: number): [number, number] => {
const { current: creatorEl } = creatorElRef;

if (!creatorEl) {
return [x, y];
}

// Calculate the new position based on the mouse position less the page offset
const { left, top } = creatorEl.getBoundingClientRect();
return [x - left, y - top];
};

// Drawing Lifecycle Callbacks
const startDraw = (x: number, y: number): void => {
const [x1, y1] = getPosition(x, y);
const [x1, y1] = getElementLocalPosition(x, y, creatorElRef.current, rotation);

setDrawingStatus(DrawingStatus.dragging);

Expand Down Expand Up @@ -100,7 +93,7 @@ export default function DrawingCreator({

const updateDraw = React.useCallback(
(x: number, y: number): void => {
const [x2, y2] = getPosition(x, y);
const [x2, y2] = getElementLocalPosition(x, y, creatorElRef.current, rotation);
const { current: points } = capturedPointsRef;

points.push({ x: x2, y: y2 });
Expand All @@ -111,7 +104,7 @@ export default function DrawingCreator({
onStart();
}
},
[drawingStatus, onStart, setDrawingStatus],
[drawingStatus, onStart, rotation, setDrawingStatus],
);

// Event Handlers
Expand Down
1 change: 1 addition & 0 deletions src/drawing/__tests__/DrawingAnnotations-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('DrawingAnnotations', () => {
location: 0,
redoDrawingPathGroup: jest.fn(),
resetDrawing: jest.fn(),
rotation: 0,
setActiveAnnotationId: jest.fn(),
setReferenceId: jest.fn(),
setStaged: jest.fn(),
Expand Down
13 changes: 13 additions & 0 deletions src/drawing/__tests__/DrawingCreator-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,26 @@ describe('DrawingCreator', () => {
const getWrapper = (props?: Partial<Props>): ReactWrapper<PointerCaptureProps, NonNullable<unknown>> =>
mount(<DrawingCreator {...getDefaults()} {...props} />);

let origOffsetWidth: PropertyDescriptor | undefined;
let origOffsetHeight: PropertyDescriptor | undefined;

beforeEach(() => {
jest.useFakeTimers();

jest.spyOn(window, 'cancelAnimationFrame');
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(cb => setTimeout(cb, 100)); // 10 fps;
jest.spyOn(Element.prototype, 'getBoundingClientRect').mockImplementation(() => getDOMRect());
jest.spyOn(Element.prototype, 'setAttribute');

origOffsetWidth = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetWidth');
origOffsetHeight = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetHeight');
Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { configurable: true, get: () => 100 });
Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { configurable: true, get: () => 100 });
Comment thread
kduncanhsu marked this conversation as resolved.
Comment thread
kduncanhsu marked this conversation as resolved.
});

afterEach(() => {
if (origOffsetWidth) Object.defineProperty(HTMLElement.prototype, 'offsetWidth', origOffsetWidth);
if (origOffsetHeight) Object.defineProperty(HTMLElement.prototype, 'offsetHeight', origOffsetHeight);
});

const simulateDrawStart = (wrapper: ReactWrapper<PointerCaptureProps, NonNullable<unknown>>, clientX = 10, clientY = 10): void => {
Expand Down
11 changes: 4 additions & 7 deletions src/region/RegionCreation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import { getVideoCurrentTimeInMilliseconds } from '../utils/useVideoTiming';

type Props = {
isCreating: boolean;
isRotated: boolean;
location: number;
resetCreator: () => void;
rotation: number;
setReferenceId: (uuid: string) => void;
setStaged: (staged: CreatorItemRegion | null) => void;
setStatus: (status: CreatorStatus) => void;
Expand All @@ -27,7 +27,7 @@ type State = {
export default class RegionCreation extends React.PureComponent<Props, State> {
static defaultProps = {
isCreating: false,
isRotated: false,
rotation: 0,
};

state: State = {};
Expand Down Expand Up @@ -57,11 +57,7 @@ export default class RegionCreation extends React.PureComponent<Props, State> {
};

render(): JSX.Element | null {
const { isCreating, isRotated, staged } = this.props;

if (isRotated) {
return null;
}
const { isCreating, rotation, staged } = this.props;

return (
<>
Expand All @@ -71,6 +67,7 @@ export default class RegionCreation extends React.PureComponent<Props, State> {
onAbort={this.handleAbort}
onStart={this.handleStart}
onStop={this.handleStop}
rotation={rotation}
/>
)}

Expand Down
4 changes: 2 additions & 2 deletions src/region/RegionCreationContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import withProviders from '../common/withProviders';

export type Props = {
isCreating: boolean;
isRotated: boolean;
rotation: number;
staged: CreatorItemRegion | null;
};

Expand All @@ -28,7 +28,7 @@ export const mapStateToProps = (state: AppState, { location }: { location: numbe

return {
isCreating: getAnnotationMode(state) === Mode.REGION && getCreatorStatus(state) !== CreatorStatus.pending,
isRotated: !!getRotation(state),
rotation: getRotation(state),
staged: isCreatorStagedRegion(staged) ? staged : null,
};
};
Expand Down
26 changes: 9 additions & 17 deletions src/region/RegionCreator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PointerCapture, { Status as DrawingStatus } from '../components/PointerCa
import PopupCursor from '../components/Popups/PopupCursor';
import RegionRect, { RegionRectRef } from './RegionRect';
import useAutoScroll from '../common/useAutoScroll';
import { getElementLocalPosition } from '../utils/rotate';
import { Rect } from '../@types';
import { styleShape } from './regionUtil';
import './RegionCreator.scss';
Expand All @@ -13,6 +14,7 @@ type Props = {
onAbort: () => void;
onStart: () => void;
onStop: (shape: Rect) => void;
rotation?: number;
};

const MIN_X = 0; // Minimum region x position must be positive
Expand All @@ -22,7 +24,7 @@ const MIN_SIZE = 10; // Minimum region size must be large enough to be clickable
const isValid = (x1: number, y1: number, x2: number, y2: number): boolean =>
Math.abs(x2 - x1) >= MIN_SIZE || Math.abs(y2 - y1) >= MIN_SIZE;

export default function RegionCreator({ className, onAbort, onStart, onStop }: Props): JSX.Element {
export default function RegionCreator({ className, onAbort, onStart, onStop, rotation = 0 }: Props): JSX.Element {
const [drawingStatus, setDrawingStatus] = React.useState<DrawingStatus>(DrawingStatus.init);
const [isHovering, setIsHovering] = React.useState<boolean>(false);
const creatorElRef = React.useRef<HTMLDivElement>(null);
Expand All @@ -34,18 +36,6 @@ export default function RegionCreator({ className, onAbort, onStart, onStop }: P
const regionRectRef = React.useRef<RegionRectRef>(null);
const renderHandleRef = React.useRef<number | null>(null);

// Drawing Helpers
const getPosition = (x: number, y: number): [number, number] => {
const { current: creatorEl } = creatorElRef;

if (!creatorEl) {
return [x, y];
}

// Calculate the new position based on the mouse position less the page offset
const { left, top } = creatorEl.getBoundingClientRect();
return [x - left, y - top];
};
const getShape = (): Rect | null => {
const { current: creatorEl } = creatorElRef;
const { current: x1 } = positionX1Ref;
Expand All @@ -57,7 +47,9 @@ export default function RegionCreator({ className, onAbort, onStart, onStop }: P
return null;
}

const { height, width } = creatorEl.getBoundingClientRect();
// Get the element's dimensions (before any rotation is applied)
const width = creatorEl.offsetWidth;
const height = creatorEl.offsetHeight;
const MAX_HEIGHT = height - MIN_SIZE;
const MAX_WIDTH = width - MIN_SIZE;
const MAX_X = Math.max(0, width - MIN_X);
Expand Down Expand Up @@ -88,7 +80,7 @@ export default function RegionCreator({ className, onAbort, onStart, onStop }: P

// Drawing Lifecycle Callbacks
const startDraw = (x: number, y: number): void => {
const [x1, y1] = getPosition(x, y);
const [x1, y1] = getElementLocalPosition(x, y, creatorElRef.current, rotation);

setDrawingStatus(DrawingStatus.dragging);

Expand Down Expand Up @@ -119,7 +111,7 @@ export default function RegionCreator({ className, onAbort, onStart, onStop }: P

const updateDraw = React.useCallback(
(x: number, y: number): void => {
const [x2, y2] = getPosition(x, y);
const [x2, y2] = getElementLocalPosition(x, y, creatorElRef.current, rotation);
const { current: x1 } = positionX1Ref;
const { current: y1 } = positionY1Ref;
const { current: prevX2 } = positionX2Ref;
Expand All @@ -138,7 +130,7 @@ export default function RegionCreator({ className, onAbort, onStart, onStop }: P
onStart();
}
},
[drawingStatus, onStart, setDrawingStatus],
[drawingStatus, onStart, rotation, setDrawingStatus],
);

// Event Handlers
Expand Down
10 changes: 5 additions & 5 deletions src/region/__tests__/RegionCreation-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,15 @@ describe('RegionCreation', () => {
expect(wrapper.exists(RegionRect)).toBe(false);
});

test('should not render creation components if file is rotated', () => {
test('should render creation components when file is rotated', () => {
const wrapper = getWrapper({
isCreating: true,
isRotated: true,
staged: {},
rotation: -90,
staged: getStaged(),
});

expect(wrapper.exists(RegionCreator)).toBe(false);
expect(wrapper.exists(RegionRect)).toBe(false);
expect(wrapper.exists(RegionCreator)).toBe(true);
expect(wrapper.exists(RegionRect)).toBe(true);
});
});
});
24 changes: 13 additions & 11 deletions src/region/__tests__/RegionCreationContainer-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,22 @@ describe('RegionCreationContainer', () => {
);

test.each`
rotation | isRotated
${null} | ${false}
${undefined} | ${false}
${0} | ${false}
${90} | ${true}
${360} | ${true}
${-360} | ${true}
${-90} | ${true}
${-0} | ${false}
`('should set the isRotated prop based on the rotation angle value', ({ isRotated, rotation }) => {
rotation | expected
${null} | ${null}
${undefined} | ${0}
${0} | ${0}
${-0} | ${-0}
${90} | ${90}
${-90} | ${-90}
${-180} | ${-180}
${-270} | ${-270}
${360} | ${360}
${-360} | ${-360}
`('should pass rotation=$rotation as $expected to the underlying component', ({ rotation, expected }) => {
const store = createStore({ options: { rotation } });
const wrapper = getWrapper({ store });

expect(wrapper.find(RegionCreation).prop('isRotated')).toEqual(isRotated);
expect(wrapper.find(RegionCreation).prop('rotation')).toEqual(expected);
});
});
});
13 changes: 13 additions & 0 deletions src/region/__tests__/RegionCreator-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ describe('RegionCreator', () => {
// Render helpers
const getWrapper = (props = {}): ReactWrapper => mount(<RegionCreator {...defaults} {...props} />);

let origOffsetWidth: PropertyDescriptor | undefined;
let origOffsetHeight: PropertyDescriptor | undefined;

beforeEach(() => {
jest.useFakeTimers();

Expand All @@ -42,6 +45,16 @@ describe('RegionCreator', () => {
jest.spyOn(document, 'removeEventListener');
jest.spyOn(window, 'cancelAnimationFrame');
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(cb => setTimeout(cb, 100)); // 10 fps

origOffsetWidth = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetWidth');
origOffsetHeight = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetHeight');
Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { configurable: true, get: () => 1000 });
Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { configurable: true, get: () => 1000 });
});

afterEach(() => {
if (origOffsetWidth) Object.defineProperty(HTMLElement.prototype, 'offsetWidth', origOffsetWidth);
if (origOffsetHeight) Object.defineProperty(HTMLElement.prototype, 'offsetHeight', origOffsetHeight);
});

describe('mouse events', () => {
Expand Down
Loading