feat(drawing): Support drawing colors (#653)

Mingze · web-flow · commit f445cef33d73 · 2020-12-17T18:20:04.000Z
* feat(drawing): Support drawing colors

* feat(drawing): Address comments
diff --git a/src/@types/events.ts b/src/@types/events.ts
@@ -8,6 +8,7 @@ enum Event {
     ANNOTATION_REMOVE = 'annotations_remove',
     ANNOTATIONS_INITIALIZED = 'annotations_initialized',
     ANNOTATIONS_MODE_CHANGE = 'annotations_mode_change',
+    COLOR_SET = 'annotations_color_set',
     VISIBLE_SET = 'annotations_visible_set',
 }
 
diff --git a/src/common/BaseAnnotator.ts b/src/common/BaseAnnotator.ts
@@ -34,6 +34,7 @@ export type Options = {
     };
     fileOptions?: FileOptions;
     hasTouch?: boolean;
+    initialColor?: string;
     initialMode?: store.Mode;
     intl: IntlOptions;
     locale?: string;
@@ -60,7 +61,17 @@ export default class BaseAnnotator extends EventEmitter {
 
     store: store.AppStore;
 
-    constructor({ apiHost, container, features = {}, file, fileOptions, initialMode, intl, token }: Options) {
+    constructor({
+        apiHost,
+        container,
+        features = {},
+        file,
+        fileOptions,
+        initialColor,
+        initialMode,
+        intl,
+        token,
+    }: Options) {
         super();
 
         const fileOptionsValue = fileOptions?.[file.id];
@@ -72,7 +83,7 @@ export default class BaseAnnotator extends EventEmitter {
             annotations: {
                 activeId: fileOptionsValue?.annotations?.activeId ?? null,
             },
-            common: { mode: initialMode },
+            common: { color: initialColor, mode: initialMode },
             options: {
                 features,
                 fileId: file.id,
@@ -93,6 +104,7 @@ export default class BaseAnnotator extends EventEmitter {
         this.addListener(LegacyEvent.SCALE, this.handleScale);
         this.addListener(Event.ACTIVE_SET, this.handleSetActive);
         this.addListener(Event.ANNOTATION_REMOVE, this.handleRemove);
+        this.addListener(Event.COLOR_SET, this.handleColorSet);
         this.addListener(Event.VISIBLE_SET, this.handleSetVisible);
 
         // Load any required data at startup
@@ -155,6 +167,10 @@ export default class BaseAnnotator extends EventEmitter {
         this.store.dispatch(store.setActiveAnnotationIdAction(annotationId));
     }
 
+    public setColor(color: string): void {
+        this.store.dispatch(store.setColorAction(color));
+    }
+
     public setVisibility(visibility: boolean): void {
         if (!this.containerEl) {
             return;
@@ -195,6 +211,10 @@ export default class BaseAnnotator extends EventEmitter {
         this.setVisibility(visibility);
     };
 
+    protected handleColorSet = (color: string): void => {
+        this.setColor(color);
+    };
+
     protected hydrate(): void {
         // Redux dispatch method signature doesn't seem to like async actions
         this.store.dispatch<any>(store.fetchAnnotationsAction()); // eslint-disable-line @typescript-eslint/no-explicit-any
diff --git a/src/common/__tests__/BaseAnnotator-test.ts b/src/common/__tests__/BaseAnnotator-test.ts
@@ -152,11 +152,11 @@ describe('BaseAnnotator', () => {
             expect(annotator.features).toEqual(features);
         });
 
-        test('should set initial mode', () => {
-            initAnnotator({ initialMode: Mode.REGION });
+        test('should set initial mode and color', () => {
+            initAnnotator({ initialColor: '#000', initialMode: Mode.REGION });
 
             expect(store.createStore).toHaveBeenLastCalledWith(
-                expect.objectContaining({ common: { mode: Mode.REGION } }),
+                expect.objectContaining({ common: { color: '#000', mode: Mode.REGION } }),
                 expect.any(Object),
             );
         });
@@ -229,6 +229,7 @@ describe('BaseAnnotator', () => {
             jest.spyOn(annotator, 'init');
             jest.spyOn(annotator, 'removeAnnotation');
             jest.spyOn(annotator, 'setActiveId');
+            jest.spyOn(annotator, 'setColor');
             jest.spyOn(annotator, 'setVisibility');
         });
 
@@ -244,6 +245,9 @@ describe('BaseAnnotator', () => {
 
             annotator.emit(Event.VISIBLE_SET, true);
             expect(annotator.setVisibility).toHaveBeenCalledWith(true);
+
+            annotator.emit(Event.COLOR_SET, '#000');
+            expect(annotator.setColor).toHaveBeenCalledWith('#000');
         });
     });
 
diff --git a/src/drawing/DrawingAnnotations.tsx b/src/drawing/DrawingAnnotations.tsx
@@ -16,6 +16,7 @@ export type Props = {
     addDrawingPathGroup: (pathGroup: PathGroup) => void;
     annotations: AnnotationDrawing[];
     canShowPopupToolbar: boolean;
+    color: string;
     drawnPathGroups: Array<PathGroup>;
     isCreating: boolean;
     location: number;
@@ -36,6 +37,7 @@ const DrawingAnnotations = (props: Props): JSX.Element => {
         addDrawingPathGroup,
         annotations,
         canShowPopupToolbar,
+        color,
         drawnPathGroups,
         isCreating,
         location,
@@ -119,7 +121,12 @@ const DrawingAnnotations = (props: Props): JSX.Element => {
             )}
 
             {isCreating && (
-                <DrawingCreator className="ba-DrawingAnnotations-creator" onStart={handleStart} onStop={handleStop} />
+                <DrawingCreator
+                    className="ba-DrawingAnnotations-creator"
+                    color={color}
+                    onStart={handleStart}
+                    onStop={handleStop}
+                />
             )}
 
             {isCreating && hasPathGroups && drawingSVGGroup && canShowPopupToolbar && (
diff --git a/src/drawing/DrawingAnnotationsContainer.tsx b/src/drawing/DrawingAnnotationsContainer.tsx
@@ -16,6 +16,7 @@ import {
     getActiveAnnotationId,
     getAnnotationMode,
     getAnnotationsForLocation,
+    getColor,
     getCreatorStatus,
     Mode,
     setActiveAnnotationIdAction,
@@ -29,6 +30,7 @@ import { setupDrawingAction } from './actions';
 export type Props = {
     activeAnnotationId: string | null;
     annotations: AnnotationDrawing[];
+    color: string;
     canShowPopupToolbar: boolean;
     drawnPathGroups: Array<PathGroup>;
     isCreating: boolean;
@@ -41,6 +43,7 @@ export const mapStateToProps = (state: AppState, { location }: { location: numbe
     return {
         activeAnnotationId: getActiveAnnotationId(state),
         annotations: getAnnotationsForLocation(state, location).filter(isDrawing),
+        color: getColor(state),
         canShowPopupToolbar: creatorStatus === CreatorStatus.started,
         drawnPathGroups: getDrawingDrawnPathGroupsForLocation(state, location),
         isCreating: getAnnotationMode(state) === Mode.DRAWING && creatorStatus !== CreatorStatus.pending,
diff --git a/src/drawing/DrawingCreator.tsx b/src/drawing/DrawingCreator.tsx
@@ -6,29 +6,35 @@ import DrawingPathGroup from './DrawingPathGroup';
 import DrawingSVG, { DrawingSVGRef } from './DrawingSVG';
 import PointerCapture, { PointerCaptureRef, Status as DrawingStatus } from '../components/PointerCapture';
 import { getPathCommands } from './drawingUtil';
-import { PathGroup, Position, Stroke } from '../@types';
+import { PathGroup, Position } from '../@types';
 import './DrawingCreator.scss';
 
 export type Props = {
     className?: string;
+    color?: string;
     onStart: () => void;
     onStop: (pathGroup: PathGroup) => void;
-    stroke?: Stroke;
+    size?: number;
 };
 
-export const defaultStroke = {
-    color: bdlBoxBlue,
-    size: 4,
-};
+export const defaultStrokeColor = bdlBoxBlue;
+export const defaultStrokeSize = 4;
 
-export default function DrawingCreator({ className, onStart, onStop, stroke = defaultStroke }: Props): JSX.Element {
+export default function DrawingCreator({
+    className,
+    color = defaultStrokeColor,
+    onStart,
+    onStop,
+    size = defaultStrokeSize,
+}: Props): JSX.Element {
     const [drawingStatus, setDrawingStatus] = React.useState<DrawingStatus>(DrawingStatus.init);
     const capturedPointsRef = React.useRef<Array<Position>>([]);
     const creatorElRef = React.useRef<PointerCaptureRef>(null);
     const drawingDirtyRef = React.useRef<boolean>(false);
     const drawingPathRef = React.useRef<DrawingPathRef>(null);
     const drawingSVGRef = React.useRef<DrawingSVGRef>(null);
     const renderHandleRef = React.useRef<number | null>(null);
+    const stroke = { color, size };
 
     const getPoints = React.useCallback((): Array<Position> => {
         const { current: creatorEl } = creatorElRef;
diff --git a/src/drawing/__tests__/DrawingAnnotations-test.tsx b/src/drawing/__tests__/DrawingAnnotations-test.tsx
@@ -18,6 +18,7 @@ describe('DrawingAnnotations', () => {
         addDrawingPathGroup: jest.fn(),
         annotations: [],
         canShowPopupToolbar: false,
+        color: '#000',
         drawnPathGroups: [],
         isCreating: false,
         location: 0,
@@ -211,6 +212,12 @@ describe('DrawingAnnotations', () => {
             expect(wrapper.find(PopupDrawingToolbar).hasClass('ba-is-drawing')).toBe(true);
         });
 
+        test('should apply selected color', () => {
+            const wrapper = getWrapper({ color: '#111', isCreating: true });
+
+            expect(wrapper.find(DrawingCreator).prop('color')).toBe('#111');
+        });
+
         test.each`
             drawn         | stashed       | canComment | canRedo  | canUndo
             ${pathGroups} | ${[]}         | ${true}    | ${false} | ${true}
diff --git a/src/drawing/__tests__/DrawingAnnotationsContainer-test.tsx b/src/drawing/__tests__/DrawingAnnotationsContainer-test.tsx
@@ -9,14 +9,18 @@ import { createStore, CreatorStatus, Mode } from '../../store';
 jest.mock('../../common/withProviders');
 
 describe('DrawingAnnotationsContainer', () => {
+    const initialState = {
+        common: { color: '#000' },
+    };
+
     const getDefaults = (): {
         intl: IntlShape;
         location: number;
         store: Store;
     } => ({
         intl: {} as IntlShape,
         location: 1,
-        store: createStore(),
+        store: createStore(initialState),
     });
     const getWrapper = (props = {}): ReactWrapper<Props> =>
         mount(<DrawingAnnotationsContainer {...getDefaults()} {...props} />);
@@ -31,6 +35,7 @@ describe('DrawingAnnotationsContainer', () => {
                 addDrawingPathGroup: expect.any(Function),
                 annotations: [],
                 canShowPopupToolbar: false,
+                color: '#000',
                 drawnPathGroups: [],
                 isCreating: false,
                 location: 1,
diff --git a/src/drawing/__tests__/DrawingCreator-test.tsx b/src/drawing/__tests__/DrawingCreator-test.tsx
@@ -1,8 +1,9 @@
 import React from 'react';
 import { act } from 'react-dom/test-utils';
 import { mount, ReactWrapper } from 'enzyme';
-import DrawingCreator, { defaultStroke, Props } from '../DrawingCreator';
+import DrawingCreator, { defaultStrokeColor, defaultStrokeSize, Props } from '../DrawingCreator';
 import DrawingPath from '../DrawingPath';
+import DrawingPathGroup from '../DrawingPathGroup';
 import DrawingSVG from '../DrawingSVG';
 import PointerCapture, { Props as PointerCaptureProps } from '../../components/PointerCapture';
 
@@ -81,6 +82,14 @@ describe('DrawingCreator', () => {
             expect(wrapper.find(PointerCapture).exists()).toBe(true);
             expect(wrapper.find(DrawingSVG).exists()).toBe(true);
         });
+
+        test('should render DrawingPathGroup with right stroke color', () => {
+            const wrapper = getWrapper({ color: '#111' });
+
+            simulateDrawStart(wrapper.find(PointerCapture));
+
+            expect(wrapper.find(DrawingPathGroup).prop('stroke')).toMatchObject({ color: '#111' });
+        });
     });
 
     describe('onStart()', () => {
@@ -127,7 +136,10 @@ describe('DrawingCreator', () => {
                         ],
                     },
                 ],
-                stroke: defaultStroke,
+                stroke: {
+                    color: defaultStrokeColor,
+                    size: defaultStrokeSize,
+                },
             });
         });
 
@@ -160,15 +172,18 @@ describe('DrawingCreator', () => {
                             ],
                         },
                     ],
-                    stroke: defaultStroke,
+                    stroke: {
+                        color: defaultStrokeColor,
+                        size: defaultStrokeSize,
+                    },
                 });
             },
         );
 
         test('should bound the drawn points by the boundary of the PointerCapture element based on the provided stroke size', () => {
             const customStroke = { color: '#000', size: 10 };
             const onStop = jest.fn();
-            const wrapper = getWrapper({ onStop, stroke: customStroke });
+            const wrapper = getWrapper({ onStop, ...customStroke });
 
             simulateDrawStart(wrapper.find(PointerCapture));
 
diff --git a/src/store/common/__mocks__/commonState.ts b/src/store/common/__mocks__/commonState.ts
@@ -1,5 +1,6 @@
 import { Mode } from '../types';
 
 export default {
+    color: '#000',
     mode: Mode.NONE,
 };
diff --git a/src/store/common/__tests__/reducer-test.ts b/src/store/common/__tests__/reducer-test.ts
@@ -1,9 +1,17 @@
 import reducer from '../reducer';
 import state from '../__mocks__/commonState';
 import { Mode } from '../types';
-import { toggleAnnotationModeAction } from '../actions';
+import { setColorAction, toggleAnnotationModeAction } from '../actions';
 
 describe('store/common/reducer', () => {
+    describe('setColorAction', () => {
+        test('should set the color in state', () => {
+            const newState = reducer(state, setColorAction('#111'));
+
+            expect(newState.color).toEqual('#111');
+        });
+    });
+
     describe('toggleAnnotationModeAction', () => {
         const { NONE, REGION } = Mode;
         test.each`
diff --git a/src/store/common/__tests__/selectors-test.ts b/src/store/common/__tests__/selectors-test.ts
@@ -1,10 +1,18 @@
 import commonState from '../__mocks__/commonState';
-import { getAnnotationMode } from '../selectors';
+import { getAnnotationMode, getColor } from '../selectors';
 
 describe('store/common/selectors', () => {
+    const state = { common: commonState };
+
     describe('getAnnotationMode', () => {
         test('should return annotation mode', () => {
-            expect(getAnnotationMode({ common: commonState })).toBe('none');
+            expect(getAnnotationMode(state)).toBe('none');
+        });
+    });
+
+    describe('getColor', () => {
+        test('should return the current creator color', () => {
+            expect(getColor(state)).toBe('#000');
         });
     });
 });
diff --git a/src/store/common/actions.ts b/src/store/common/actions.ts
@@ -1,4 +1,5 @@
 import { createAction } from '@reduxjs/toolkit';
 import { Mode } from './types';
 
+export const setColorAction = createAction<string>('SET_COLOR');
 export const toggleAnnotationModeAction = createAction<Mode>('TOGGLE_ANNOTATION_MODE');
diff --git a/src/store/common/reducer.ts b/src/store/common/reducer.ts
@@ -1,11 +1,19 @@
-import { createReducer, combineReducers } from '@reduxjs/toolkit';
+import { bdlBoxBlue } from 'box-ui-elements/es/styles/variables';
+import { createReducer } from '@reduxjs/toolkit';
 import { CommonState, Mode } from './types';
-import { toggleAnnotationModeAction } from './actions';
+import { setColorAction, toggleAnnotationModeAction } from './actions';
 
-const modeReducer = createReducer<CommonState['mode']>(Mode.NONE, builder =>
-    builder.addCase(toggleAnnotationModeAction, (state, { payload: mode }: { payload: Mode }) => mode),
-);
+export const initialState = {
+    color: bdlBoxBlue,
+    mode: Mode.NONE,
+};
 
-export default combineReducers({
-    mode: modeReducer,
-});
+export default createReducer<CommonState>(initialState, builder =>
+    builder
+        .addCase(setColorAction, (state, { payload }) => {
+            state.color = payload;
+        })
+        .addCase(toggleAnnotationModeAction, (state, { payload }) => {
+            state.mode = payload;
+        }),
+);
diff --git a/src/store/common/selectors.ts b/src/store/common/selectors.ts
@@ -4,3 +4,4 @@ import { Mode } from './types';
 type State = Pick<AppState, 'common'>;
 
 export const getAnnotationMode = ({ common }: State): Mode => common.mode;
+export const getColor = (state: State): string => state.common.color;
diff --git a/src/store/common/types.ts b/src/store/common/types.ts

Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@ enum Event {`
`8`	`8`	`ANNOTATION_REMOVE = 'annotations_remove',`
`9`	`9`	`ANNOTATIONS_INITIALIZED = 'annotations_initialized',`
`10`	`10`	`ANNOTATIONS_MODE_CHANGE = 'annotations_mode_change',`
	`11`	`+ COLOR_SET = 'annotations_color_set',`
`11`	`12`	`VISIBLE_SET = 'annotations_visible_set',`
`12`	`13`	`}`
`13`	`14`