Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Communication: Remember last scroll position when switching conversations #9614

Merged
merged 23 commits into from
Nov 9, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0ad2e22
Add scroll position memorization
cremertim Oct 28, 2024
28a0dc6
Reduced timeout
cremertim Oct 28, 2024
a76e349
performance
cremertim Oct 28, 2024
9726517
Default scroll to bottom of the messages if you create a new message
cremertim Oct 29, 2024
9979342
Added test cases
cremertim Oct 29, 2024
d9b1024
First implementation via ID
cremertim Oct 29, 2024
21c0c1b
Merge branch 'develop' into bugfix/communication/adjust-channel-jumping
cremertim Oct 29, 2024
870d20f
optimization from coderabit
cremertim Oct 29, 2024
82aff7f
Merge remote-tracking branch 'origin/bugfix/communication/adjust-chan…
cremertim Oct 29, 2024
1b5de0b
Added integration Test for search
cremertim Oct 30, 2024
4ad6553
variable readonly
cremertim Oct 30, 2024
c98aef2
Added tests
cremertim Oct 30, 2024
7398ef2
Merge branch 'develop' into bugfix/communication/adjust-channel-jumping
cremertim Oct 30, 2024
674e3b5
Merge branch 'develop' into bugfix/communication/adjust-channel-jumping
cremertim Oct 30, 2024
3fb2349
Merge remote-tracking branch 'origin/develop' into bugfix/communicati…
Pablosanqt Oct 31, 2024
7d7ad2c
Small bugfix
Pablosanqt Oct 31, 2024
94e18ab
Fix for change between modules
Pablosanqt Oct 31, 2024
355a91c
Fix tests
Pablosanqt Oct 31, 2024
41d9509
Merge remote-tracking branch 'origin/develop' into bugfix/communicati…
PaRangger Nov 3, 2024
095f321
Fix for proper scroll position on load and on new message
PaRangger Nov 4, 2024
8fe911b
Merge remote-tracking branch 'origin/develop' into bugfix/communicati…
PaRangger Nov 8, 2024
b4da15b
Fix merge conflict issues
PaRangger Nov 8, 2024
0e18443
Merge branch 'develop' into bugfix/communication/adjust-channel-jumping
krusche Nov 9, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@
'hide-input': isHiddenInputWithCallToAction,
}"
infinite-scroll
class="conversation-messages-message-list"
class="conversation-messages-message-list position-relative"
[scrollWindow]="false"
(scrolledUp)="fetchNextPage()"
>
<!-- list of all top level posts -->
<!-- answers are opened in the thread sidebar -->
@for (group of groupedPosts; track postsTrackByFn($index, group)) {
@for (group of groupedPosts; track postsGroupTrackByFn($index, group)) {
<div class="message-group">
@for (post of group.posts; track postsTrackByFn($index, post)) {
<div class="post-item">
Expand All @@ -107,15 +107,10 @@
@if (getAsChannel(_activeConversation)?.isAnnouncementChannel) {
<div class="pt-2">
<button class="btn btn-md btn-primary" (click)="createEditModal.open()" jhiTranslate="artemisApp.metis.newAnnouncement"></button>
<jhi-post-create-edit-modal
#createEditModal
[posting]="newPost!"
[isCommunicationPage]="true"
(onCreate)="createEmptyPost(); scrollToBottomOfMessages()"
/>
<jhi-post-create-edit-modal #createEditModal [posting]="newPost!" [isCommunicationPage]="true" (onCreate)="handleNewMessageCreated()" />
</div>
} @else {
<jhi-message-inline-input class="message-input" [posting]="newPost!" (onCreate)="createEmptyPost(); scrollToBottomOfMessages()" />
<jhi-message-inline-input class="message-input" [posting]="newPost!" (onCreate)="handleNewMessageCreated()" />
}
</div>
}
Expand All @@ -125,15 +120,10 @@
@if (getAsChannel(_activeConversation)?.isAnnouncementChannel) {
<div class="pt-2">
<button class="btn btn-md btn-primary" (click)="createEditModal.open()" jhiTranslate="artemisApp.metis.newAnnouncement"></button>
<jhi-post-create-edit-modal
#createEditModal
[posting]="newPost!"
[isCommunicationPage]="true"
(onCreate)="createEmptyPost(); scrollToBottomOfMessages()"
/>
<jhi-post-create-edit-modal #createEditModal [posting]="newPost!" [isCommunicationPage]="true" (onCreate)="handleNewMessageCreated()" />
</div>
} @else {
<jhi-message-inline-input class="message-input" [posting]="newPost!" (onCreate)="createEmptyPost(); scrollToBottomOfMessages()" />
<jhi-message-inline-input class="message-input" [posting]="newPost!" (onCreate)="handleNewMessageCreated()" />
}
</div>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
OnInit,
Output,
QueryList,
Renderer2,
ViewChild,
ViewChildren,
ViewEncapsulation,
Expand All @@ -32,6 +33,7 @@ import { LayoutService } from 'app/shared/breakpoints/layout.service';
import { CustomBreakpointNames } from 'app/shared/breakpoints/breakpoints.service';
import dayjs from 'dayjs/esm';
import { User } from 'app/core/user/user.model';
import { PostingThreadComponent } from 'app/shared/metis/posting-thread/posting-thread.component';

interface PostGroup {
author: User | undefined;
Expand All @@ -46,16 +48,23 @@ interface PostGroup {
})
export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnDestroy {
private ngUnsubscribe = new Subject<void>();
readonly sessionStorageKey = 'conversationId.scrollPosition.';

readonly PageType = PageType;
readonly ButtonType = ButtonType;

private scrollDebounceTime = 100; // ms
scrollSubject = new Subject<number>();
canStartSaving = false;
createdNewMessage = false;

@Output() openThread = new EventEmitter<Post>();

@ViewChild('searchInput')
searchInput: ElementRef;

@ViewChildren('postingThread')
messages: QueryList<any>;
messages: QueryList<PostingThreadComponent>;
@ViewChild('container')
content: ElementRef;
@Input()
Expand All @@ -77,6 +86,7 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD
searchText = '';
_activeConversation?: ConversationDTO;

elementsAtScrollPosition: PostingThreadComponent[];
newPost?: Post;
posts: Post[] = [];
groupedPosts: PostGroup[] = [];
Expand All @@ -93,6 +103,7 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD
isHiddenInputFull = false;

private layoutService: LayoutService = inject(LayoutService);
private renderer = inject(Renderer2);

constructor(
public metisService: MetisService, // instance from course-conversations.component
Expand All @@ -104,6 +115,7 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD
this.subscribeToSearch();
this.subscribeToMetis();
this.subscribeToActiveConversation();
this.setupScrollDebounce();
this.isMobile = this.layoutService.isBreakpointActive(CustomBreakpointNames.extraSmall);

this.layoutService
Expand Down Expand Up @@ -147,11 +159,28 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD

ngAfterViewInit() {
this.messages.changes.pipe(takeUntil(this.ngUnsubscribe)).subscribe(this.handleScrollOnNewMessage);
this.messages.changes.pipe(takeUntil(this.ngUnsubscribe)).subscribe(() => {
if (!this.createdNewMessage && this.posts.length > 0) {
this.scrollToStoredId();
} else {
this.createdNewMessage = false;
}
});
this.content.nativeElement.addEventListener('scroll', () => {
this.findElementsAtScrollPosition();
});
cremertim marked this conversation as resolved.
Show resolved Hide resolved
}

ngOnDestroy(): void {
this.ngUnsubscribe.next();
this.ngUnsubscribe.complete();
this.scrollSubject.complete();
this.content?.nativeElement.removeEventListener('scroll', this.saveScrollPosition);
}

private scrollToStoredId() {
const savedScrollId = sessionStorage.getItem(this.sessionStorageKey + this._activeConversation?.id) ?? '';
requestAnimationFrame(() => this.goToLastSelectedElement(parseInt(savedScrollId, 10)));
}

private onActiveConversationChange() {
Expand All @@ -168,6 +197,7 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD
this.searchInput.nativeElement.value = '';
this.searchText = '';
}
this.canStartSaving = false;
this.onSearch();
this.createEmptyPost();
}
Expand Down Expand Up @@ -260,11 +290,15 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD

fetchNextPage() {
const morePostsAvailable = this.posts.length < this.totalNumberOfPosts;
let addBuffer = 0;
if (morePostsAvailable) {
this.page += 1;
this.commandMetisToFetchPosts();
addBuffer = 50;
} else if (!this.canStartSaving) {
this.canStartSaving = true;
}
this.content.nativeElement.scrollTop = this.content.nativeElement.scrollTop + 50;
this.content.nativeElement.scrollTop = this.content.nativeElement.scrollTop + addBuffer;
}

public commandMetisToFetchPosts(forceUpdate = false) {
Expand Down Expand Up @@ -305,7 +339,9 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD
return this.metisService.createEmptyPostForContext(conversation);
}

postsTrackByFn = (index: number, post: Post): number => post.id!;
postsGroupTrackByFn = (index: number, post: PostGroup): string => 'grp_' + post.posts.map((p) => p.id?.toString()).join('_');

postsTrackByFn = (index: number, post: Post): string => 'post_' + post.id!;

setPostForThread(post: Post) {
this.openThread.emit(post);
Expand All @@ -318,9 +354,9 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD

scrollToBottomOfMessages() {
// Use setTimeout to ensure the scroll happens after the new message is rendered
setTimeout(() => {
requestAnimationFrame(() => {
this.content.nativeElement.scrollTop = this.content.nativeElement.scrollHeight;
}, 0);
});
}

onSearchQueryInput($event: Event) {
Expand All @@ -334,4 +370,58 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD
this.searchInput.nativeElement.dispatchEvent(new Event('input'));
}
}

private setupScrollDebounce(): void {
this.scrollSubject.pipe(debounceTime(this.scrollDebounceTime), takeUntil(this.ngUnsubscribe)).subscribe((postId) => {
if (this._activeConversation?.id) {
sessionStorage.setItem(this.sessionStorageKey + this._activeConversation.id, postId.toString());
}
});
}
cremertim marked this conversation as resolved.
Show resolved Hide resolved

saveScrollPosition = (postId: number) => {
this.scrollSubject.next(postId);
};

handleNewMessageCreated() {
this.createdNewMessage = true;
this.createEmptyPost();
this.scrollToBottomOfMessages();
}

async goToLastSelectedElement(lastScrollPosition: number) {
if (!lastScrollPosition) {
this.scrollToBottomOfMessages();
this.canStartSaving = true;
return;
}
const messageArray = this.messages.toArray();
const element = messageArray.find((message) => message.post.id === lastScrollPosition); // Suchen nach dem Post

if (!element) {
this.fetchNextPage();
} else {
// We scroll to the element with a slight buffer to ensure its fully visible (-10)
this.content.nativeElement.scrollTop = Math.max(0, element.elementRef.nativeElement.offsetTop - 10);
this.canStartSaving = true;
}
}

findElementsAtScrollPosition() {
const messageArray = this.messages.toArray();
const containerRect = this.content.nativeElement.getBoundingClientRect();
const visibleMessages = [];
for (const message of messageArray) {
if (!message.elementRef?.nativeElement || !message.post?.id) continue;
const rect = message.elementRef.nativeElement.getBoundingClientRect();
if (rect.top >= containerRect.top && rect.bottom <= containerRect.bottom) {
visibleMessages.push(message);
break; // Only need the first visible message
}
}
this.elementsAtScrollPosition = visibleMessages;
if (this.elementsAtScrollPosition && this.canStartSaving) {
this.saveScrollPosition(this.elementsAtScrollPosition[0].post.id!);
}
}
PaRangger marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ChangeDetectionStrategy, Component, EventEmitter, Input, Output } from '@angular/core';
import { ChangeDetectionStrategy, Component, ElementRef, EventEmitter, Input, Output, inject } from '@angular/core';
import { Post } from 'app/entities/metis/post.model';
import dayjs from 'dayjs/esm';

Expand All @@ -18,4 +18,6 @@ export class PostingThreadComponent {
@Input() hasChannelModerationRights = false;
@Output() openThread = new EventEmitter<Post>();
@Input() isConsecutive: boolean | undefined = false;

elementRef = inject(ElementRef);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { Post } from 'app/entities/metis/post.model';
import { BehaviorSubject } from 'rxjs';
import { ConversationDTO } from 'app/entities/metis/conversation/conversation.model';
import { generateExampleChannelDTO, generateExampleGroupChatDTO, generateOneToOneChatDTO } from '../../helpers/conversationExampleModels';
import { Directive, EventEmitter, Input, Output } from '@angular/core';
import { Directive, EventEmitter, Input, Output, QueryList } from '@angular/core';
import { By } from '@angular/platform-browser';
import { Course } from 'app/entities/course.model';
import { getAsChannelDTO } from 'app/entities/metis/conversation/channel.model';
Expand Down Expand Up @@ -87,6 +87,15 @@ examples.forEach((activeConversation) => {
fixture = TestBed.createComponent(ConversationMessagesComponent);
component = fixture.componentInstance;
component.course = course;
component.messages = {
toArray: jest.fn().mockReturnValue([]),
} as any;
component.content = {
nativeElement: {
getBoundingClientRect: jest.fn().mockReturnValue({ top: 0, bottom: 100 }),
},
} as any;
component.canStartSaving = true;
fixture.detectChanges();
});

Expand All @@ -96,8 +105,6 @@ examples.forEach((activeConversation) => {

it('should create', fakeAsync(() => {
expect(component).toBeTruthy();
component.handleScrollOnNewMessage();
tick();
}));

it('should set initial values correctly', fakeAsync(() => {
Expand Down Expand Up @@ -132,6 +139,83 @@ examples.forEach((activeConversation) => {
expect(getFilteredPostSpy).toHaveBeenCalledOnce();
}));

it('should save the scroll position in sessionStorage', fakeAsync(() => {
const setItemSpy = jest.spyOn(sessionStorage, 'setItem');
component.ngOnInit();
component.saveScrollPosition(15);
tick(100);
const expectedKey = `${component.sessionStorageKey}${component._activeConversation?.id}`;
const expectedValue = '15';
expect(setItemSpy).toHaveBeenCalledWith(expectedKey, expectedValue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we be more specific here. This can be very helpful in debugging if there is a bug somewhere.

Suggested change
expect(setItemSpy).toHaveBeenCalledWith(expectedKey, expectedValue);
expect(setItemSpy).toHaveBeenCalledExactlyOnceWith(expectedKey, expectedValue);

expect(setItemSpy).toHaveBeenCalledTimes(2);
}));
cremertim marked this conversation as resolved.
Show resolved Hide resolved

it('should scroll to the last selected element or fetch next page if not found', fakeAsync(() => {
const mockMessages = [
{ post: { id: 1 }, elementRef: { nativeElement: { scrollIntoView: jest.fn() } } },
{ post: { id: 2 }, elementRef: { nativeElement: { scrollIntoView: jest.fn() } } },
] as unknown as PostingThreadComponent[];
component.messages = {
toArray: () => mockMessages,
} as QueryList<PostingThreadComponent>;

const fetchNextPageSpy = jest.spyOn(component, 'fetchNextPage').mockImplementation(() => {});
const existingScrollPosition = 1;

component.goToLastSelectedElement(existingScrollPosition);
tick();
expect(fetchNextPageSpy).not.toHaveBeenCalled();

const nonExistingScrollPosition = 999;
component.goToLastSelectedElement(nonExistingScrollPosition);
tick();

expect(fetchNextPageSpy).toHaveBeenCalled();
}));

it('should find visible elements at the scroll position and save scroll position', () => {
// Mock des Containers
component.content.nativeElement = {
getBoundingClientRect: jest.fn().mockReturnValue({ top: 0, bottom: 100 }),
scrollTop: 0,
scrollHeight: 200,
removeEventListener: jest.fn(),
};
const mockMessages = [
{ post: { id: 1 }, elementRef: { nativeElement: { getBoundingClientRect: jest.fn().mockReturnValue({ top: 10, bottom: 90 }) } } },
{ post: { id: 2 }, elementRef: { nativeElement: { getBoundingClientRect: jest.fn().mockReturnValue({ top: 100, bottom: 200 }) } } },
] as unknown as PostingThreadComponent[];
component.messages.toArray = jest.fn().mockReturnValue(mockMessages);
component.canStartSaving = true;
const nextSpy = jest.spyOn(component.scrollSubject, 'next');
component.findElementsAtScrollPosition();
expect(component.elementsAtScrollPosition).toEqual([mockMessages[0]]);
expect(nextSpy).toHaveBeenCalledWith(1);
});

it('should not save scroll position if no elements are visible', () => {
const mockMessages = [
{
post: { id: 1 },
elementRef: { nativeElement: { getBoundingClientRect: jest.fn().mockReturnValue({ top: 200, bottom: 300 }) } },
},
] as unknown as PostingThreadComponent[];

component.messages.toArray = jest.fn().mockReturnValue(mockMessages);
const nextSpy = jest.spyOn(component.scrollSubject, 'next');
component.findElementsAtScrollPosition();
expect(component.elementsAtScrollPosition).toEqual([]);
expect(nextSpy).not.toHaveBeenCalled();
});

it('should scroll to the bottom when a new message is created', fakeAsync(() => {
component.content.nativeElement.scrollTop = 100;
fixture.detectChanges();
component.handleNewMessageCreated();
tick(300);
expect(component.content.nativeElement.scrollTop).toBe(component.content.nativeElement.scrollHeight);
}));

it('should create empty post with the correct conversation type', fakeAsync(() => {
const createEmptyPostForContextSpy = jest.spyOn(metisService, 'createEmptyPostForContext').mockReturnValue(new Post());
component.createEmptyPost();
Expand Down
Loading