feat(discoverability): Reorder RegionCreator before RegionList (#597)

Author: Conrad Chan

src/common/BaseAnnotator.ts

@@ -60,7 +60,7 @@ 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, initialMode, intl, token }: Options) {
         super();
 
         const fileOptionsValue = fileOptions?.[file.id];
@@ -74,6 +74,7 @@ export default class BaseAnnotator extends EventEmitter {
             },
             common: { mode: initialMode },
             options: {
+                features,
                 fileId: file.id,
                 fileVersionId: fileOptionsVersionId ?? fileVersionId,
                 isCurrentFileVersion: !fileOptionsVersionId || fileOptionsVersionId === currentFileVersionId,
@@ -82,7 +83,7 @@ export default class BaseAnnotator extends EventEmitter {
         };
 
         this.container = container;
-        this.features = features || {};
+        this.features = features;
         this.intl = i18n.createIntlProvider(intl);
         this.store = store.createStore(initialState, {
             api: new API({ apiHost, token }),

src/common/__tests__/BaseAnnotator-test.ts

@@ -70,6 +70,7 @@ describe('BaseAnnotator', () => {
                     expect.objectContaining({
                         annotations: { activeId: expectedActiveId },
                         options: {
+                            features: { enabledFeature: true },
                             fileId: '12345',
                             fileVersionId: '98765',
                             isCurrentFileVersion: true,
@@ -97,6 +98,7 @@ describe('BaseAnnotator', () => {
                 expect.objectContaining({
                     annotations: { activeId: null },
                     options: {
+                        features: { enabledFeature: true },
                         fileId: '12345',
                         fileVersionId: '456',
                         isCurrentFileVersion: false,

src/region/RegionAnnotations.tsx

@@ -13,6 +13,7 @@ type Props = {
     annotations: AnnotationRegion[];
     createRegion: (arg: CreateArg) => void;
     isCreating: boolean;
+    isDiscoverabilityEnabled: boolean;
     isRotated: boolean;
     location: number;
     message: string;
@@ -33,6 +34,7 @@ export default class RegionAnnotations extends React.PureComponent<Props, State>
     static defaultProps = {
         annotations: [],
         isCreating: false,
+        isDiscoverabilityEnabled: false,
         isRotated: false,
     };
 
@@ -76,34 +78,60 @@ export default class RegionAnnotations extends React.PureComponent<Props, State>
         createRegion({ ...staged, message });
     };
 
+    renderCreator = (): JSX.Element => {
+        const { isCreating, isRotated } = this.props;
+        const canCreate = isCreating && !isRotated;
+
+        return (
+            <>
+                {canCreate && (
+                    <RegionCreator
+                        className="ba-RegionAnnotations-creator"
+                        onStart={this.handleStart}
+                        onStop={this.handleStop}
+                    />
+                )}
+            </>
+        );
+    };
+
+    renderList = (): JSX.Element => {
+        const { activeAnnotationId, annotations } = this.props;
+
+        return (
+            <RegionList
+                activeId={activeAnnotationId}
+                annotations={annotations}
+                className="ba-RegionAnnotations-list"
+                onSelect={this.handleAnnotationActive}
+            />
+        );
+    };
+
     setRectRef = (rectRef: RegionRectRef): void => {
         this.setState({ rectRef });
     };
 
     render(): JSX.Element {
-        const { activeAnnotationId, annotations, isCreating, isRotated, message, staged, status } = this.props;
+        const { isCreating, isDiscoverabilityEnabled, isRotated, message, staged, status } = this.props;
         const { rectRef } = this.state;
         const canCreate = isCreating && !isRotated;
         const canReply = status !== CreatorStatus.started && status !== CreatorStatus.init;
         const isPending = status === CreatorStatus.pending;
 
         return (
             <>
-                {/* Layer 1: Saved annotations */}
-                <RegionList
-                    activeId={activeAnnotationId}
-                    annotations={annotations}
-                    className="ba-RegionAnnotations-list"
-                    onSelect={this.handleAnnotationActive}
-                />
-
-                {/* Layer 2: Drawn (unsaved) incomplete annotation target, if any */}
-                {canCreate && (
-                    <RegionCreator
-                        className="ba-RegionAnnotations-creator"
-                        onStart={this.handleStart}
-                        onStop={this.handleStop}
-                    />
+                {isDiscoverabilityEnabled ? (
+                    <>
+                        {/* With discoverability, put the RegionCreator below the RegionList so that existing regions can be clicked */}
+                        {this.renderCreator()}
+                        {this.renderList()}
+                    </>
+                ) : (
+                    <>
+                        {this.renderList()}
+                        {this.renderCreator()}
+                    </>
                 )}
 
                 {/* Layer 3a: Staged (unsaved) annotation target, if any */}

src/region/RegionContainer.tsx

@@ -12,6 +12,7 @@ import {
     getCreatorStatus,
     getRotation,
     isCreatorStagedRegion,
+    isFeatureEnabled,
     Mode,
     resetCreatorAction,
     setActiveAnnotationIdAction,
@@ -28,6 +29,7 @@ export type Props = {
     activeAnnotationId: string | null;
     annotations: AnnotationRegion[];
     isCreating: boolean;
+    isDiscoverabilityEnabled: boolean;
     isRotated: boolean;
     message: string;
     staged: CreatorItemRegion | null;
@@ -41,6 +43,7 @@ export const mapStateToProps = (state: AppState, { location }: { location: numbe
         activeAnnotationId: getActiveAnnotationId(state),
         annotations: getAnnotationsForLocation(state, location).filter(isRegion),
         isCreating: getAnnotationMode(state) === Mode.REGION,
+        isDiscoverabilityEnabled: isFeatureEnabled(state, 'discoverability'),
         isRotated: !!getRotation(state),
         message: getCreatorMessage(state),
         staged: isCreatorStagedRegion(staged) ? staged : null,

src/region/__tests__/RegionAnnotations-test.tsx

@@ -231,5 +231,43 @@ describe('RegionAnnotations', () => {
             expect(wrapper.exists(RegionList)).toBe(true);
             expect(wrapper.exists(RegionRect)).toBe(false);
         });
+
+        test('should render RegionList before RegionCreator if discoverability is disabled', () => {
+            const wrapper = getWrapper({ isCreating: true, isDiscoverabilityEnabled: false });
+            expect(wrapper).toMatchInlineSnapshot(`
+                <Fragment>
+                  <Memo(RegionList)
+                    activeId={null}
+                    annotations={Array []}
+                    className="ba-RegionAnnotations-list"
+                    onSelect={[Function]}
+                  />
+                  <RegionCreator
+                    className="ba-RegionAnnotations-creator"
+                    onStart={[Function]}
+                    onStop={[Function]}
+                  />
+                </Fragment>
+            `);
+        });
+
+        test('should render RegionCreator before RegionList if discoverability is enabled', () => {
+            const wrapper = getWrapper({ isCreating: true, isDiscoverabilityEnabled: true });
+            expect(wrapper).toMatchInlineSnapshot(`
+                <Fragment>
+                  <RegionCreator
+                    className="ba-RegionAnnotations-creator"
+                    onStart={[Function]}
+                    onStop={[Function]}
+                  />
+                  <Memo(RegionList)
+                    activeId={null}
+                    annotations={Array []}
+                    className="ba-RegionAnnotations-list"
+                    onSelect={[Function]}
+                  />
+                </Fragment>
+            `);
+        });
     });
 });

src/region/__tests__/RegionContainer-test.tsx

@@ -26,6 +26,7 @@ describe('RegionContainer', () => {
                 annotations: [],
                 createRegion: expect.any(Function),
                 isCreating: false,
+                isDiscoverabilityEnabled: false,
                 location: 1,
                 message: '',
                 staged: null,
@@ -54,5 +55,12 @@ describe('RegionContainer', () => {
 
             expect(wrapper.find(RegionAnnotations).prop('isRotated')).toEqual(isRotated);
         });
+
+        test('should read the discoverability feature from the store', () => {
+            const store = createStore({ options: { features: { discoverability: true } } });
+            const wrapper = getWrapper({ store });
+
+            expect(wrapper.find(RegionAnnotations).prop('isDiscoverabilityEnabled')).toBe(true);
+        });
     });
 });

src/store/options/__tests__/selectors-test.ts

@@ -1,7 +1,16 @@
-import { getFileId, getFileVersionId, getPermissions, getRotation, getScale } from '../selectors';
+import {
+    getFeatures,
+    getFileId,
+    getFileVersionId,
+    getPermissions,
+    getRotation,
+    getScale,
+    isFeatureEnabled,
+} from '../selectors';
 
 describe('store/options/selectors', () => {
     const optionsState = {
+        features: { enabledFeature: true },
         fileId: '12345',
         fileVersionId: '67890',
         isCurrentFileVersion: true,
@@ -13,6 +22,12 @@ describe('store/options/selectors', () => {
         scale: 1,
     };
 
+    describe('getFeatures', () => {
+        test('should return the features', () => {
+            expect(getFeatures({ options: optionsState })).toEqual({ enabledFeature: true });
+        });
+    });
+
     describe('getFileVersionId', () => {
         test('should return the file id', () => {
             expect(getFileId({ options: optionsState })).toBe('12345');
@@ -45,4 +60,14 @@ describe('store/options/selectors', () => {
             expect(getScale({ options: optionsState })).toBe(1);
         });
     });
+
+    describe('isFeatureEnabled', () => {
+        test('should return value for feature in the features object', () => {
+            expect(isFeatureEnabled({ options: optionsState }, 'enabledFeature')).toBe(true);
+        });
+
+        test('should return false for feature not in the features object', () => {
+            expect(isFeatureEnabled({ options: optionsState }, 'nonExistentFeature')).toBe(false);
+        });
+    });
 });

src/store/options/reducer.ts

@@ -9,6 +9,7 @@ import {
 } from './actions';
 
 export const initialState = {
+    features: {},
     fileId: null,
     fileVersionId: null,
     isCurrentFileVersion: true,

src/store/options/selectors.ts

@@ -1,11 +1,16 @@
+import getProp from 'lodash/get';
 import { AppState } from '../types';
 import { Permissions } from '../../@types';
+import { Features } from '../../BoxAnnotations';
 
 type State = Pick<AppState, 'options'>;
 
+export const getFeatures = (state: State): Features => state.options.features;
 export const getFileId = (state: State): string | null => state.options.fileId;
 export const getFileVersionId = (state: State): string | null => state.options.fileVersionId;
 export const getIsCurrentFileVersion = (state: State): boolean => state.options.isCurrentFileVersion;
 export const getPermissions = (state: State): Permissions => state.options.permissions;
 export const getRotation = (state: State): number => state.options.rotation;
 export const getScale = (state: State): number => state.options.scale;
+export const isFeatureEnabled = (state: State, featurename: string): boolean =>
+    getProp(getFeatures(state), featurename, false);

src/store/options/types.ts

@@ -1,6 +1,8 @@
+import { Features } from '../../BoxAnnotations';
 import { Permissions } from '../../@types';
 
 export type OptionsState = {
+    features: Features;
     fileId: string | null;
     fileVersionId: string | null;
     isCurrentFileVersion: boolean;