Skip to content

Commit 98434cd

Browse files
mickrmickr
authored
feat(annotations): update url on annotation selection (#2079)
* feat(annotations): update url on annotation selection * Add history push to update url when clicking annotation activity item * Add proper activeFeedEntryType for focus * Fix focus state for activity item * Add messages for missing annotation * Add annotator to withAnnotations context to emit back to annotations. * Add the annotator emit handler to context
1 parent bb68144 commit 98434cd

File tree

22 files changed

+283
-52
lines changed

22 files changed

+283
-52
lines changed

‎i18n/en-US.properties‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ be.accessStatsPermissionsError = Sorry, you do not have permission to see the ac
44
be.activityFeed.fullDateTime = {time, date, full} at {time, time, short}
55
# Error message for feed item API errors
66
be.activityFeedItemApiError = There was a problem loading the activity feed. Please refresh the page or try again later.
7+
# Text to show when an annotation activity no longer exists
8+
be.activitySidebar.activityFeed.annotationMissingError = This comment no longer exists
79
# Text to show when comment no longer exists
810
be.activitySidebar.activityFeed.commentMissingError = This comment no longer exists
911
# Error title

‎src/common/types/feed.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { BoxItemPermission, BoxItemVersion, BoxItemVersionMini, User } from
44
import type { Target } from './annotations';
55

66
// Feed item types that can receive deeplinks inline in the feed
7-
type FocusableFeedItemType = 'task' | 'comment';
7+
type FocusableFeedItemType = 'task' | 'comment' | 'annotation';
88

99
type BoxCommentPermission = {
1010
can_delete?: boolean,

‎src/elements/common/annotator-context/AnnotatorContext.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
import * as React from 'react';
2-
import { AnnotatorState } from './types';
2+
import { AnnotatorContext } from './types';
33

4-
export default React.createContext({} as AnnotatorState);
4+
export default React.createContext({} as AnnotatorContext);

‎src/elements/common/annotator-context/__tests__/withAnnotations.test.tsx‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { shallow, ShallowWrapper } from 'enzyme';
33

44
import { AnnotatorContext, withAnnotations } from '../index';
55
import { WithAnnotationsProps, ComponentWithAnnotations } from '../withAnnotations';
6-
import { AnnotatorState, Action } from '../types';
6+
import { AnnotatorContext as AnnotatorContextType, Action } from '../types';
77

88
type ComponentProps = {
99
className: string;
@@ -12,15 +12,20 @@ type ComponentProps = {
1212
type WrappedComponentProps = ComponentProps & WithAnnotationsProps;
1313

1414
type ContextProviderProps = {
15-
value: AnnotatorState;
15+
value: AnnotatorContextType;
1616
};
1717

1818
describe('elements/common/annotator-context/withAnnotations', () => {
1919
const MockComponent = (props: ComponentProps) => <div {...props} />;
2020

2121
const WrappedComponent = withAnnotations(MockComponent);
2222

23-
const defaultProps = { className: 'foo', onAnnotatorEvent: jest.fn(), onPreviewDestroy: jest.fn() };
23+
const defaultProps = {
24+
className: 'foo',
25+
onAnnotator: jest.fn(),
26+
onAnnotatorEvent: jest.fn(),
27+
onPreviewDestroy: jest.fn(),
28+
};
2429

2530
const getWrapper = (
2631
props: WrappedComponentProps = defaultProps,
@@ -36,23 +41,43 @@ describe('elements/common/annotator-context/withAnnotations', () => {
3641

3742
const wrappedComponent = wrapper.find<WrappedComponentProps>(MockComponent);
3843
expect(wrappedComponent.exists()).toBeTruthy();
44+
expect(wrappedComponent.props().onAnnotator).toBeTruthy();
3945
expect(wrappedComponent.props().onAnnotatorEvent).toBeTruthy();
4046
expect(wrappedComponent.props().onPreviewDestroy).toBeTruthy();
4147
});
4248

43-
test('should pass the state on to the AnnotatorContext.Provider', () => {
49+
test('should pass the context on to the AnnotatorContext.Provider', () => {
4450
const wrapper = getWrapper();
45-
51+
const instance = wrapper.instance();
4652
const contextProvider = getContextProvider(wrapper);
53+
4754
expect(contextProvider.exists()).toBeTruthy();
48-
expect(contextProvider.prop('value')).toEqual({
55+
expect(contextProvider.prop('value').emitActiveChangeEvent).toEqual(instance.emitActiveChangeEvent);
56+
expect(contextProvider.prop('value').state).toEqual({
4957
action: null,
5058
activeAnnotationId: null,
5159
annotation: null,
5260
error: null,
5361
});
5462
});
5563

64+
describe('emitActiveChangeEvent', () => {
65+
test('should call annotator emit on action', () => {
66+
const mockAnnotator = {
67+
emit: jest.fn(),
68+
};
69+
const wrapper = getWrapper();
70+
const instance = wrapper.instance();
71+
72+
// Set the annotator on the withAnnotations instance
73+
instance.handleOnAnnotator(mockAnnotator);
74+
instance.emitActiveChangeEvent('123');
75+
76+
expect(mockAnnotator.emit).toBeCalled();
77+
expect(mockAnnotator.emit).toBeCalledWith('annotations_active_set', '123');
78+
});
79+
});
80+
5681
describe('handleAnnotationCreate()', () => {
5782
const mockAnnotation = { foo: 'bar' };
5883
const mockError = new Error('boo');
@@ -77,7 +102,7 @@ describe('elements/common/annotator-context/withAnnotations', () => {
77102
wrapper.instance().handleAnnotationCreate(eventData);
78103
const contextProvider = getContextProvider(wrapper);
79104
expect(contextProvider.exists()).toBeTruthy();
80-
expect(contextProvider.prop('value')).toEqual({
105+
expect(contextProvider.prop('value').state).toEqual({
81106
action: expectedAction,
82107
activeAnnotationId: null,
83108
annotation: expectedAnnotation,
@@ -98,7 +123,7 @@ describe('elements/common/annotator-context/withAnnotations', () => {
98123
wrapper.instance().handleActiveChange(annotationId);
99124
const contextProvider = getContextProvider(wrapper);
100125
expect(contextProvider.exists()).toBeTruthy();
101-
expect(contextProvider.prop('value').activeAnnotationId).toEqual(expected);
126+
expect(contextProvider.prop('value').state.activeAnnotationId).toEqual(expected);
102127
});
103128
});
104129

@@ -125,16 +150,16 @@ describe('elements/common/annotator-context/withAnnotations', () => {
125150
});
126151

127152
describe('handlePreviewDestroy()', () => {
128-
test('should reset state', () => {
153+
test('should reset state and annotator', () => {
129154
const wrapper = getWrapper();
130-
131-
wrapper.instance().handleActiveChange('123');
155+
const instance = wrapper.instance();
156+
instance.handleActiveChange('123');
132157
let contextProvider = getContextProvider(wrapper);
133-
expect(contextProvider.prop('value').activeAnnotationId).toEqual('123');
158+
expect(contextProvider.prop('value').state.activeAnnotationId).toEqual('123');
134159

135160
wrapper.instance().handlePreviewDestroy();
136161
contextProvider = getContextProvider(wrapper);
137-
expect(contextProvider.prop('value').activeAnnotationId).toEqual(null);
162+
expect(contextProvider.prop('value').state.activeAnnotationId).toEqual(null);
138163
});
139164
});
140165
});

‎src/elements/common/annotator-context/__tests__/withAnnotatorContext.test.tsx‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,22 @@ describe('elements/common/annotator-context/withAnnotatorContext', () => {
2828
annotation: { foo: 'bar' },
2929
action: Action.CREATE_START,
3030
};
31+
const mockEmitAnnotatorChangeEvent = jest.fn();
3132

32-
mockContext.mockReturnValue(annotatorState);
33+
mockContext.mockReturnValue({
34+
state: annotatorState,
35+
emitActiveChangeEvent: mockEmitAnnotatorChangeEvent,
36+
});
3337

3438
const wrapper = getWrapper();
3539
const wrappedComponent = wrapper.dive().find(Component);
40+
const props = wrappedComponent.props() as WrappedComponentProps<HTMLDivElement>;
41+
3642
expect(wrappedComponent.exists()).toBeTruthy();
37-
expect((wrappedComponent.props() as WrappedComponentProps<HTMLDivElement>).annotatorState).toEqual({
43+
expect(props.annotatorState).toEqual({
3844
annotation: { foo: 'bar' },
3945
action: Action.CREATE_START,
4046
});
47+
expect(props.emitAnnotatorActiveChangeEvent).toEqual(mockEmitAnnotatorChangeEvent);
4148
});
4249
});

‎src/elements/common/annotator-context/types.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,23 @@ export enum Action {
66
// Can extend to other actions: update_start, update_end, delete_start, delete_end
77
}
88

9+
export interface Annotator {
10+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
11+
emit: (event: string | symbol, ...args: any[]) => void;
12+
}
13+
914
export interface AnnotatorState {
1015
activeAnnotationId?: string | null;
1116
annotation?: object | null;
1217
action?: Action | null;
1318
error?: Error | null;
1419
}
1520

21+
export interface AnnotatorContext {
22+
state: AnnotatorState;
23+
emitActiveChangeEvent: (id: string) => void;
24+
}
25+
1626
export enum Status {
1727
ERROR = 'error',
1828
PENDING = 'pending',

‎src/elements/common/annotator-context/withAnnotations.tsx‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
import * as React from 'react';
22
import AnnotatorContext from './AnnotatorContext';
3-
import { Action, AnnotationActionEvent, AnnotatorState, Status } from './types';
3+
import { Action, Annotator, AnnotationActionEvent, AnnotatorState, Status } from './types';
44

55
export interface WithAnnotationsProps {
6+
onAnnotator: (annotator: Annotator) => void;
67
onAnnotatorEvent: ({ event, data }: { event: string; data: unknown }) => void;
78
onPreviewDestroy: () => void;
89
}
910

1011
export interface ComponentWithAnnotations {
12+
emitActiveChangeEvent: (id: string | null) => void;
1113
getAction: (eventData: AnnotationActionEvent) => Action;
1214
handleActiveChange: (annotationId: string | null) => void;
15+
handleAnnotationChangeEvent: (id: string | null) => void;
1316
handleAnnotationCreate: (eventData: AnnotationActionEvent) => void;
1417
handleAnnotatorEvent: ({ event, data }: { event: string; data?: unknown }) => void;
18+
handleOnAnnotator: (annotator: Annotator) => void;
1519
handlePreviewDestroy: () => void;
1620
}
1721

@@ -30,8 +34,20 @@ export default function withAnnotations<P extends object>(
3034
class ComponentWithAnnotations extends React.Component<P & WithAnnotationsProps, AnnotatorState> {
3135
static displayName: string;
3236

37+
annotator: Annotator | null = null;
38+
3339
state = defaultState;
3440

41+
emitActiveChangeEvent = (id: string | null) => {
42+
const { annotator } = this;
43+
44+
if (!annotator) {
45+
return;
46+
}
47+
48+
annotator.emit('annotations_active_set', id);
49+
};
50+
3551
getAction({ meta: { status }, error }: AnnotationActionEvent): Action {
3652
return status === Status.SUCCESS || error ? Action.CREATE_END : Action.CREATE_START;
3753
}
@@ -59,15 +75,23 @@ export default function withAnnotations<P extends object>(
5975
}
6076
};
6177

78+
handleOnAnnotator = (annotator: Annotator): void => {
79+
this.annotator = annotator;
80+
};
81+
6282
handlePreviewDestroy = (): void => {
6383
this.setState(defaultState);
84+
this.annotator = null;
6485
};
6586

6687
render(): JSX.Element {
6788
return (
68-
<AnnotatorContext.Provider value={this.state}>
89+
<AnnotatorContext.Provider
90+
value={{ emitActiveChangeEvent: this.emitActiveChangeEvent, state: this.state }}
91+
>
6992
<WrappedComponent
7093
{...this.props}
94+
onAnnotator={this.handleOnAnnotator}
7195
onAnnotatorEvent={this.handleAnnotatorEvent}
7296
onPreviewDestroy={this.handlePreviewDestroy}
7397
/>

‎src/elements/common/annotator-context/withAnnotatorContext.tsx‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,20 @@ import { AnnotatorState } from './types';
44

55
export interface WithAnnotatorContextProps {
66
annotatorState?: AnnotatorState;
7+
emitAnnotatorActiveChangeEvent?: (id: string) => void;
78
}
89

910
export default function withAnnotatorContext<P extends {}>(WrappedComponent: React.ComponentType<P>) {
1011
return React.forwardRef<React.RefForwardingComponent<React.ComponentType<P>>, P>((props, ref) => (
1112
<AnnotatorContext.Consumer>
12-
{annotatorState => <WrappedComponent ref={ref} {...props} annotatorState={annotatorState} />}
13+
{({ emitActiveChangeEvent, state }) => (
14+
<WrappedComponent
15+
ref={ref}
16+
{...props}
17+
annotatorState={state}
18+
emitAnnotatorActiveChangeEvent={emitActiveChangeEvent}
19+
/>
20+
)}
1321
</AnnotatorContext.Consumer>
1422
));
1523
}

‎src/elements/content-preview/ContentPreview.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ type Props = {
8383
logoUrl?: string,
8484
measureRef: Function,
8585
messages?: StringMap,
86+
onAnnotator: Function,
87+
onAnnotatorEvent: Function,
8688
onClose?: Function,
8789
onDownload: Function,
8890
onLoad: Function,
@@ -200,6 +202,7 @@ class ContentPreview extends React.PureComponent<Props, State> {
200202
enableThumbnailsSidebar: false,
201203
hasHeader: false,
202204
language: DEFAULT_LOCALE,
205+
onAnnotator: noop,
203206
onAnnotatorEvent: noop,
204207
onDownload: noop,
205208
onError: noop,
@@ -714,6 +717,7 @@ class ContentPreview extends React.PureComponent<Props, State> {
714717
enableThumbnailsSidebar,
715718
fileOptions,
716719
onAnnotatorEvent,
720+
onAnnotator,
717721
showAnnotationsControls,
718722
token: tokenOrTokenFunction,
719723
...rest
@@ -760,6 +764,7 @@ class ContentPreview extends React.PureComponent<Props, State> {
760764

761765
if (showAnnotationsControls) {
762766
this.preview.addListener('annotatorevent', onAnnotatorEvent);
767+
this.preview.addListener('annotator', onAnnotator);
763768
}
764769

765770
this.preview.updateFileCache([file]);

‎src/elements/content-sidebar/ActivitySidebar.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import AddTaskButton from './AddTaskButton';
1414
import API from '../../api';
1515
import messages from '../common/messages';
1616
import SidebarContent from './SidebarContent';
17+
import { WithAnnotatorContextProps, withAnnotatorContext } from '../common/annotator-context';
1718
import { EVENT_JS_READY } from '../common/logger/constants';
1819
import { getBadUserError, getBadItemError } from '../../utils/error';
1920
import { mark } from '../../utils/performance';
@@ -56,7 +57,8 @@ type ExternalProps = {
5657
onTaskDelete: (id: string) => any,
5758
onTaskUpdate: () => any,
5859
onTaskView: (id: string, isCreator: boolean) => any,
59-
} & ErrorContextProps;
60+
} & ErrorContextProps &
61+
WithAnnotatorContextProps;
6062

6163
type PropsWithoutContext = {
6264
elementId: string,
@@ -94,6 +96,7 @@ mark(MARK_NAME_JS_READY);
9496

9597
class ActivitySidebar extends React.PureComponent<Props, State> {
9698
static defaultProps = {
99+
emitAnnotatorActiveChangeEvent: noop,
97100
isDisabled: false,
98101
onCommentCreate: noop,
99102
onCommentDelete: noop,
@@ -594,6 +597,7 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
594597
activeFeedEntryId,
595598
activeFeedEntryType,
596599
onTaskView,
600+
emitAnnotatorActiveChangeEvent,
597601
} = this.props;
598602
const {
599603
currentUser,
@@ -619,6 +623,7 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
619623
mentionSelectorContacts={mentionSelectorContacts}
620624
currentUser={currentUser}
621625
isDisabled={isDisabled}
626+
onAnnotationSelect={emitAnnotatorActiveChangeEvent}
622627
onAppActivityDelete={this.deleteAppActivity}
623628
onCommentCreate={this.createComment}
624629
onCommentDelete={this.deleteComment}
@@ -651,4 +656,5 @@ export default flow([
651656
withErrorBoundary(ORIGIN_ACTIVITY_SIDEBAR),
652657
withAPIContext,
653658
withFeatureConsumer,
659+
withAnnotatorContext,
654660
])(ActivitySidebar);

0 commit comments

Comments
 (0)