Skip to content

Commit 1123db9

Browse files
author
Conrad Chan
authored
feat(annotation): scroll into view on activeEntryId change (#2105)
1 parent db2b0da commit 1123db9

File tree

3 files changed

+124
-72
lines changed

3 files changed

+124
-72
lines changed

‎src/elements/content-sidebar/activity-feed/activity-feed/ActiveState.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ const ActiveState = ({
160160
'bcs-is-focused': isFocused,
161161
})}
162162
data-testid="annotation-activity"
163+
ref={refValue}
163164
>
164165
<AnnotationActivity
165166
currentUser={currentUser}

‎src/elements/content-sidebar/activity-feed/activity-feed/ActivityFeed.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,11 @@ class ActivityFeed extends React.Component<Props, State> {
7070
}
7171

7272
componentDidUpdate(prevProps: Props, prevState: State) {
73-
const { currentUser: prevCurrentUser, feedItems: prevFeedItems } = prevProps;
73+
const {
74+
activeFeedEntryId: prevActiveFeedEntryId,
75+
currentUser: prevCurrentUser,
76+
feedItems: prevFeedItems,
77+
} = prevProps;
7478
const { feedItems: currFeedItems, activeFeedEntryId } = this.props;
7579
const { isInputOpen: prevIsInputOpen } = prevState;
7680
const { isInputOpen: currIsInputOpen } = this.state;
@@ -79,13 +83,13 @@ class ActivityFeed extends React.Component<Props, State> {
7983
const hasMoreItems = prevFeedItems && currFeedItems && prevFeedItems.length < currFeedItems.length;
8084
const didLoadFeedItems = prevFeedItems === undefined && currFeedItems !== undefined;
8185
const hasInputOpened = currIsInputOpen !== prevIsInputOpen;
86+
const hasActiveFeedEntryIdChanged = activeFeedEntryId !== prevActiveFeedEntryId;
8287

8388
if ((hasLoaded || hasMoreItems || didLoadFeedItems || hasInputOpened) && activeFeedEntryId === undefined) {
8489
this.resetFeedScroll();
8590
}
8691

87-
// do the scroll only once after first fetch of feed items
88-
if (didLoadFeedItems) {
92+
if (didLoadFeedItems || hasActiveFeedEntryIdChanged) {
8993
this.scrollToActiveFeedItemOrErrorMessage();
9094
}
9195
}

‎src/elements/content-sidebar/activity-feed/activity-feed/__tests__/ActivityFeed.test.js‎

Lines changed: 116 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -173,92 +173,139 @@ describe('elements/content-sidebar/ActivityFeed/activity-feed/ActivityFeed', ()
173173
expect(instance.feedContainer.scrollTop).toEqual(100);
174174
});
175175

176-
test('should set scrollTop to be the scrollHeight if feedContainer exists and prevProps feedItems is undefined and this.props.feedItems is defined', () => {
177-
const wrapper = getWrapper({
178-
feedItems: [{ type: 'comment' }],
176+
describe('componentDidUpdate()', () => {
177+
test('should set scrollTop to be the scrollHeight if feedContainer exists and prevProps feedItems is undefined and this.props.feedItems is defined', () => {
178+
const wrapper = getWrapper({
179+
feedItems: [{ type: 'comment' }],
180+
});
181+
const instance = wrapper.instance();
182+
instance.feedContainer = {
183+
scrollTop: 0,
184+
scrollHeight: 100,
185+
};
186+
187+
instance.componentDidUpdate(
188+
{
189+
feedItems: undefined,
190+
currentUser,
191+
},
192+
{ isInputOpen: false },
193+
);
194+
195+
expect(instance.feedContainer.scrollTop).toEqual(100);
179196
});
180-
const instance = wrapper.instance();
181-
instance.feedContainer = {
182-
scrollTop: 0,
183-
scrollHeight: 100,
184-
};
185197

186-
instance.componentDidUpdate(
187-
{
188-
feedItems: undefined,
189-
currentUser,
190-
},
191-
{ isInputOpen: false },
192-
);
198+
test('should set scrollTop to be the scrollHeight if more feedItems are added', () => {
199+
const wrapper = getWrapper({
200+
feedItems: [{ type: 'comment' }, { type: 'comment' }],
201+
});
193202

194-
expect(instance.feedContainer.scrollTop).toEqual(100);
195-
});
203+
const instance = wrapper.instance();
204+
instance.feedContainer = {
205+
scrollTop: 0,
206+
scrollHeight: 100,
207+
};
208+
209+
instance.componentDidUpdate(
210+
{
211+
feedItems: [{ type: 'comment' }],
212+
currentUser,
213+
},
214+
{ isInputOpen: false },
215+
);
196216

197-
test('should set scrollTop to be the scrollHeight if more feedItems are added', () => {
198-
const wrapper = getWrapper({
199-
feedItems: [{ type: 'comment' }, { type: 'comment' }],
217+
expect(instance.feedContainer.scrollTop).toEqual(100);
200218
});
201219

202-
const instance = wrapper.instance();
203-
instance.feedContainer = {
204-
scrollTop: 0,
205-
scrollHeight: 100,
206-
};
207-
208-
instance.componentDidUpdate(
209-
{
220+
test('should set scrollTop to be the scrollHeight if the user becomes defined', () => {
221+
const wrapper = getWrapper({
210222
feedItems: [{ type: 'comment' }],
211-
currentUser,
212-
},
213-
{ isInputOpen: false },
214-
);
215-
216-
expect(instance.feedContainer.scrollTop).toEqual(100);
217-
});
223+
});
224+
const instance = wrapper.instance();
225+
instance.feedContainer = {
226+
scrollTop: 0,
227+
scrollHeight: 100,
228+
};
229+
230+
instance.componentDidUpdate(
231+
{
232+
feedItems: [{ type: 'comment' }],
233+
currentUser: undefined,
234+
},
235+
{ isInputOpen: false },
236+
);
218237

219-
test('should set scrollTop to be the scrollHeight if the user becomes defined', () => {
220-
const wrapper = getWrapper({
221-
feedItems: [{ type: 'comment' }],
238+
expect(instance.feedContainer.scrollTop).toEqual(100);
222239
});
223-
const instance = wrapper.instance();
224-
instance.feedContainer = {
225-
scrollTop: 0,
226-
scrollHeight: 100,
227-
};
228240

229-
instance.componentDidUpdate(
230-
{
241+
test('should set scrollTop to be the scrollHeight if input opens', () => {
242+
const wrapper = getWrapper({
231243
feedItems: [{ type: 'comment' }],
232-
currentUser: undefined,
233-
},
234-
{ isInputOpen: false },
235-
);
244+
});
245+
wrapper.setState({
246+
isInputOpen: true,
247+
});
248+
const instance = wrapper.instance();
249+
instance.feedContainer = {
250+
scrollTop: 0,
251+
scrollHeight: 100,
252+
};
253+
254+
instance.componentDidUpdate(
255+
{
256+
feedItems: [{ type: 'comment' }],
257+
currentUser,
258+
},
259+
{ isInputOpen: false },
260+
);
236261

237-
expect(instance.feedContainer.scrollTop).toEqual(100);
238-
});
262+
expect(instance.feedContainer.scrollTop).toEqual(100);
263+
});
239264

240-
test('should set scrollTop to be the scrollHeight if input opens', () => {
241-
const wrapper = getWrapper({
242-
feedItems: [{ type: 'comment' }],
265+
test('should call scrollToActiveFeedItemOrErrorMessage if feed items loaded', () => {
266+
const wrapper = getWrapper({ feedItems: [{ type: 'comment' }] });
267+
const instance = wrapper.instance();
268+
instance.scrollToActiveFeedItemOrErrorMessage = jest.fn();
269+
270+
instance.componentDidUpdate(
271+
{
272+
feedItems: undefined,
273+
},
274+
{ isInputOpen: false },
275+
);
276+
277+
expect(instance.scrollToActiveFeedItemOrErrorMessage).toHaveBeenCalled();
243278
});
244-
wrapper.setState({
245-
isInputOpen: true,
279+
280+
test('should call scrollToActiveFeedItemOrErrorMessage if activeFeedEntryId changed', () => {
281+
const wrapper = getWrapper({ activeFeedEntryId: '123' });
282+
const instance = wrapper.instance();
283+
instance.scrollToActiveFeedItemOrErrorMessage = jest.fn();
284+
285+
instance.componentDidUpdate(
286+
{
287+
activeFeedEntryId: '456',
288+
},
289+
{ isInputOpen: false },
290+
);
291+
292+
expect(instance.scrollToActiveFeedItemOrErrorMessage).toHaveBeenCalled();
246293
});
247-
const instance = wrapper.instance();
248-
instance.feedContainer = {
249-
scrollTop: 0,
250-
scrollHeight: 100,
251-
};
252294

253-
instance.componentDidUpdate(
254-
{
255-
feedItems: [{ type: 'comment' }],
256-
currentUser,
257-
},
258-
{ isInputOpen: false },
259-
);
295+
test('should not call scrollToActiveFeedItemOrErrorMessage if activeFeedEntryId changed', () => {
296+
const wrapper = getWrapper({ activeFeedEntryId: '456' });
297+
const instance = wrapper.instance();
298+
instance.scrollToActiveFeedItemOrErrorMessage = jest.fn();
260299

261-
expect(instance.feedContainer.scrollTop).toEqual(100);
300+
instance.componentDidUpdate(
301+
{
302+
activeFeedEntryId: '456',
303+
},
304+
{ isInputOpen: false },
305+
);
306+
307+
expect(instance.scrollToActiveFeedItemOrErrorMessage).not.toHaveBeenCalled();
308+
});
262309
});
263310

264311
test('should pass activeFeedItemRef to the ActiveState', () => {

0 commit comments

Comments
 (0)