-
Notifications
You must be signed in to change notification settings - Fork 297
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
: Group consecutive messages
#9456
Changes from 11 commits
b09c320
fedc900
1113a4b
df6a1fc
8a48723
613e8d8
72fefe2
313dfef
252cedf
8f264b4
a53c7ba
f152661
e3c5384
9725c07
6794ca1
c9e7e6e
8726cee
f6d149c
bfec85d
0cc49d9
29a25c5
2f73014
c9d276a
7792212
21e97e1
67ce6f0
eec93d9
9287c3a
d355ce0
64bae5f
f81ba93
c34b89f
daa405a
47c5b04
73da6d8
3d6beb7
95850f1
ba071a1
5bf7f97
12937ee
fe6537f
ae47652
7dba8e8
830a8ea
5610443
fcb0525
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ export class Post extends Posting { | |
public conversation?: Conversation; | ||
public displayPriority?: DisplayPriority; | ||
public resolved?: boolean; | ||
public isConsecutive?: boolean = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) LGTM! Consider making the property non-optional for type safety. The addition of the For improved type safety, consider making the property non-optional: public isConsecutive: boolean = false; This change would eliminate the need for null checks when using this property elsewhere in the codebase. |
||
|
||
constructor() { | ||
super(); | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -74,19 +74,24 @@ | |||||
> | ||||||
<!-- list of all top level posts --> | ||||||
<!-- answers are opened in the thread sidebar --> | ||||||
@for (post of posts; track postsTrackByFn($index, post)) { | ||||||
<div> | ||||||
<jhi-posting-thread | ||||||
#postingThread | ||||||
[lastReadDate]="_activeConversation?.lastReadDate" | ||||||
[hasChannelModerationRights]="!!getAsChannel(_activeConversation)?.hasChannelModerationRights" | ||||||
[id]="'item-' + post.id" | ||||||
[post]="post" | ||||||
[showAnswers]="false" | ||||||
[readOnlyMode]="!!getAsChannel(_activeConversation)?.isArchived" | ||||||
[isCommunicationPage]="true" | ||||||
(openThread)="setPostForThread($event)" | ||||||
/> | ||||||
@for (group of groupedPosts; track postsTrackByFn($index, group)) { | ||||||
<div class="message-group"> | ||||||
@for (post of group.posts; track postsTrackByFn($index, post)) { | ||||||
<div class="post-item"> | ||||||
<jhi-posting-thread | ||||||
#postingThread | ||||||
[lastReadDate]="_activeConversation?.lastReadDate" | ||||||
[hasChannelModerationRights]="!!getAsChannel(_activeConversation)?.hasChannelModerationRights" | ||||||
[id]="'item-' + post.id" | ||||||
[post]="post" | ||||||
[showAnswers]="false" | ||||||
[readOnlyMode]="!!getAsChannel(_activeConversation)?.isArchived" | ||||||
[isCommunicationPage]="true" | ||||||
(openThread)="setPostForThread($event)" | ||||||
[isConsecutive]="post.isConsecutive || false" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider using the nullish coalescing operator for default values Instead of using the logical OR -[isConsecutive]="post.isConsecutive || false"
+[isConsecutive]="post.isConsecutive ?? false" This ensures that if 📝 Committable suggestion
Suggested change
|
||||||
></jhi-posting-thread> | ||||||
</div> | ||||||
} | ||||||
</div> | ||||||
} | ||||||
</div> | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,3 +60,25 @@ | |
padding-bottom: 100px; | ||
} | ||
} | ||
|
||
.message-group { | ||
display: flex; | ||
flex-direction: column; | ||
margin-bottom: 10px; | ||
} | ||
|
||
.grouped-posts { | ||
margin-left: 30px; | ||
padding-left: 10px; | ||
} | ||
Comment on lines
+84
to
+87
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) LGTM: Good indentation for grouped posts The Consider using CSS variables for the margin and padding values to maintain consistency and ease future adjustments. For example: :root {
--grouped-posts-left-margin: 30px;
--grouped-posts-left-padding: 10px;
}
.grouped-posts {
margin-left: var(--grouped-posts-left-margin);
padding-left: var(--grouped-posts-left-padding);
} |
||
|
||
.grouped-posts, | ||
.grouped-post { | ||
margin-top: 0; | ||
margin-bottom: 0; | ||
padding: 0; | ||
} | ||
|
||
jhi-posting-thread { | ||
margin-bottom: 5px; | ||
} | ||
Comment on lines
+96
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) LGTM: Appropriate spacing for posting threads The addition of a 5px bottom margin to the Consider using a class selector instead of an element selector to be more consistent with BEM naming conventions and to improve the modularity of your styles. For example: .posting-thread {
margin-bottom: 5px;
} Then, apply this class to your |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,22 @@ import { MetisConversationService } from 'app/shared/metis/metis-conversation.se | |
import { OneToOneChat, isOneToOneChatDTO } from 'app/entities/metis/conversation/one-to-one-chat.model'; | ||
import { canCreateNewMessageInConversation } from 'app/shared/metis/conversations/conversation-permissions.utils'; | ||
import { debounceTime, distinctUntilChanged } from 'rxjs/operators'; | ||
import dayjs from 'dayjs/esm'; | ||
import { User } from 'app/core/user/user.model'; | ||
|
||
import { Pipe, PipeTransform } from '@angular/core'; | ||
|
||
@Pipe({ standalone: true, name: 'dayjsToDate' }) | ||
export class DayjsToDatePipe implements PipeTransform { | ||
transform(value: dayjs.Dayjs | undefined): Date | undefined { | ||
return value ? value.toDate() : undefined; | ||
} | ||
} | ||
|
||
interface PostGroup { | ||
author: User | undefined; | ||
posts: Post[]; | ||
} | ||
|
||
@Component({ | ||
selector: 'jhi-conversation-messages', | ||
|
@@ -69,6 +85,7 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD | |
|
||
newPost?: Post; | ||
posts: Post[] = []; | ||
groupedPosts: PostGroup[] = []; | ||
totalNumberOfPosts = 0; | ||
page = 1; | ||
public isFetchingPosts = true; | ||
|
@@ -159,11 +176,70 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD | |
}; | ||
} | ||
|
||
toDate(dayjsDate: dayjs.Dayjs | undefined): Date | undefined { | ||
return dayjsDate ? dayjsDate.toDate() : undefined; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Eliminate duplicate code by removing the The |
||
|
||
private groupPosts(): void { | ||
if (!this.posts || this.posts.length === 0) { | ||
this.groupedPosts = []; | ||
return; | ||
} | ||
|
||
const sortedPosts = this.posts.sort((a, b) => { | ||
const aDate = (a as any).creationDateAsDate; | ||
const bDate = (b as any).creationDateAsDate; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Replace Using Also applies to: 205-206 |
||
return (aDate?.getTime() || 0) - (bDate?.getTime() || 0); | ||
}); | ||
|
||
const groups: PostGroup[] = []; | ||
let currentGroup: PostGroup = { | ||
author: sortedPosts[0].author, | ||
posts: [{ ...sortedPosts[0], isConsecutive: false }], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid adding Adding Also applies to: 214-214 |
||
}; | ||
|
||
for (let i = 1; i < sortedPosts.length; i++) { | ||
const currentPost = sortedPosts[i]; | ||
const lastPostInGroup = currentGroup.posts[currentGroup.posts.length - 1]; | ||
|
||
const currentDate = (currentPost as any).creationDateAsDate; | ||
const lastDate = (lastPostInGroup as any).creationDateAsDate; | ||
|
||
let timeDiff = Number.MAX_SAFE_INTEGER; | ||
if (currentDate && lastDate) { | ||
timeDiff = Math.abs(currentDate.getTime() - lastDate.getTime()) / 60000; | ||
} | ||
|
||
if (currentPost.author?.id === currentGroup.author?.id && timeDiff <= 60) { | ||
currentGroup.posts.push({ ...currentPost, isConsecutive: true }); // consecutive post | ||
} else { | ||
groups.push(currentGroup); | ||
currentGroup = { | ||
author: currentPost.author, | ||
posts: [{ ...currentPost, isConsecutive: false }], | ||
}; | ||
} | ||
} | ||
|
||
groups.push(currentGroup); | ||
this.groupedPosts = groups; | ||
this.cdr.detectChanges(); | ||
} | ||
|
||
setPosts(posts: Post[]): void { | ||
if (this.content) { | ||
this.previousScrollDistanceFromTop = this.content.nativeElement.scrollHeight - this.content.nativeElement.scrollTop; | ||
} | ||
this.posts = posts.slice().reverse(); | ||
|
||
this.posts = posts | ||
.slice() | ||
.reverse() | ||
.map((post) => { | ||
(post as any).creationDateAsDate = post.creationDate ? dayjs(post.creationDate).toDate() : undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid mutating Adding |
||
return post; | ||
}); | ||
|
||
this.groupPosts(); | ||
} | ||
|
||
fetchNextPage() { | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,36 +1,51 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div [id]="'item-' + posting.id" class="row" [ngClass]="isCommunicationPage ? 'module-bg mt-2 rounded-2' : 'answer-post m-1'"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<jhi-answer-post-header | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[posting]="posting" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isReadOnlyMode]="isReadOnlyMode" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(openPostingCreateEditModal)="createAnswerPostModal.open()" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isCommunicationPage]="isCommunicationPage" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[lastReadDate]="lastReadDate" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[hasChannelModerationRights]="hasChannelModerationRights" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@if (!isConsecutive) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<jhi-answer-post-header | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[posting]="posting" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isReadOnlyMode]="isReadOnlyMode" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isCommunicationPage]="isCommunicationPage" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[lastReadDate]="lastReadDate" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[hasChannelModerationRights]="hasChannelModerationRights" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@if (!createAnswerPostModal.isInputOpen) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div class="answer-post-content-margin"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<jhi-posting-content | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[content]="posting.content" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isEdited]="!!posting.updatedDate" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[author]="posting.author" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[posting]="posting" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isReply]="true" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(userReferenceClicked)="userReferenceClicked.emit($event)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(channelReferenceClicked)="channelReferenceClicked.emit($event)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div class="answer-post-content-margin message-container"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div class="message-content"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<jhi-posting-content | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[content]="posting.content" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isEdited]="!!posting.updatedDate" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[author]="posting.author" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[posting]="posting" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isReply]="true" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(userReferenceClicked)="userReferenceClicked.emit($event)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(channelReferenceClicked)="channelReferenceClicked.emit($event)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div class="answer-post-content-margin hover-actions"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<jhi-answer-post-reactions-bar | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isReadOnlyMode]="isReadOnlyMode" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[posting]="posting" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isLastAnswer]="isLastAnswer" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isThreadSidebar]="isThreadSidebar" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(openPostingCreateEditModal)="createAnswerPostModal.open()" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(reactionsUpdated)="onReactionsUpdated($event)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) LGTM: New structure aligns with PR objectives The new structure with the hover-actions div and the placement of Consider adding an - <div class="answer-post-content-margin hover-actions">
+ <div class="answer-post-content-margin hover-actions" aria-label="Message actions"> 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div class="answer-post-content-margin"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<ng-container #createEditAnswerPostContainer /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div class="answer-post-content-margin"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<jhi-answer-post-footer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<jhi-answer-post-reactions-bar | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isReadOnlyMode]="isReadOnlyMode" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[posting]="posting" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isLastAnswer]="isLastAnswer" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isThreadSidebar]="isThreadSidebar" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(openPostingCreateEditModal)="openPostingCreateEditModal.emit()" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(openPostingCreateEditModal)="createAnswerPostModal.open()" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(reactionsUpdated)="onReactionsUpdated($event)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[isEmojiCount]="true" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) LGTM: Standalone reactions bar for emoji counts The addition of a standalone For consistency, consider moving the <jhi-answer-post-reactions-bar
[isReadOnlyMode]="isReadOnlyMode"
[posting]="posting"
[isLastAnswer]="isLastAnswer"
[isThreadSidebar]="isThreadSidebar"
+ [isEmojiCount]="true"
(openPostingCreateEditModal)="createAnswerPostModal.open()"
(reactionsUpdated)="onReactionsUpdated($event)"
- [isEmojiCount]="true"
/> 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<jhi-answer-post-create-edit-modal #createAnswerPostModal [posting]="posting" [createEditAnswerPostContainerRef]="containerRef" /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<jhi-answer-post-create-edit-modal #createAnswerPostModal [posting]="posting" (postingUpdated)="onPostingUpdated($event)" [createEditAnswerPostContainerRef]="containerRef" /> |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -6,3 +6,42 @@ | |||||||||||||||||||||||
.answer-post-content-margin { | ||||||||||||||||||||||||
padding-left: 3.2rem; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
.message-container { | ||||||||||||||||||||||||
position: relative; | ||||||||||||||||||||||||
border-radius: 5px; | ||||||||||||||||||||||||
transition: background-color 0.3s ease; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) LGTM! Consider adding box-shadow for depth. The To further improve the visual hierarchy, consider adding a subtle .message-container {
position: relative;
border-radius: 5px;
transition: background-color 0.3s ease;
+ box-shadow: 0 1px 3px rgba(0, 0, 0, 0.1);
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
.message-content { | ||||||||||||||||||||||||
padding-left: 0.3rem; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
.message-content:hover { | ||||||||||||||||||||||||
background: var(--metis-selection-option-hover-background); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) LGTM! Consider adding keyboard focus styles for accessibility. The hover effect for -.message-content:hover {
+.message-content:hover,
+.message-content:focus-within {
background: var(--metis-selection-option-hover-background);
} This ensures that keyboard users can also access the hover state. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
.hover-actions { | ||||||||||||||||||||||||
position: absolute; | ||||||||||||||||||||||||
top: -1.8rem; | ||||||||||||||||||||||||
right: 1%; | ||||||||||||||||||||||||
display: flex; | ||||||||||||||||||||||||
gap: 10px; | ||||||||||||||||||||||||
visibility: hidden; | ||||||||||||||||||||||||
transition: | ||||||||||||||||||||||||
opacity 0.2s ease-in-out, | ||||||||||||||||||||||||
visibility 0.2s ease-in-out; | ||||||||||||||||||||||||
background: var(--metis-selection-option-background); | ||||||||||||||||||||||||
padding: 5px; | ||||||||||||||||||||||||
border-radius: 5px; | ||||||||||||||||||||||||
border: 0.01rem solid var(--metis-gray); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
.message-container:hover .hover-actions { | ||||||||||||||||||||||||
opacity: 1; | ||||||||||||||||||||||||
visibility: visible; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
Comment on lines
+61
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) LGTM! Consider adding keyboard focus styles for accessibility. The implementation for showing hover actions when the message container is hovered over is well done. To improve accessibility, consider adding styles for keyboard focus: -.message-container:hover .hover-actions {
+.message-container:hover .hover-actions,
+.message-container:focus-within .hover-actions {
opacity: 1;
visibility: visible;
} This ensures that keyboard users can also access the hover actions. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
.clickable { | ||||||||||||||||||||||||
cursor: pointer; | ||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,6 +2,8 @@ import { ChangeDetectionStrategy, Component, EventEmitter, Input, Output, ViewCh | |||||
import { AnswerPost } from 'app/entities/metis/answer-post.model'; | ||||||
import { PostingDirective } from 'app/shared/metis/posting.directive'; | ||||||
import dayjs from 'dayjs/esm'; | ||||||
import { Posting } from 'app/entities/metis/posting.model'; | ||||||
import { Reaction } from 'app/entities/metis/reaction.model'; | ||||||
|
||||||
@Component({ | ||||||
selector: 'jhi-answer-post', | ||||||
|
@@ -20,4 +22,13 @@ export class AnswerPostComponent extends PostingDirective<AnswerPost> { | |||||
isReadOnlyMode = false; | ||||||
// ng-container to render answerPostCreateEditModalComponent | ||||||
@ViewChild('createEditAnswerPostContainer', { read: ViewContainerRef }) containerRef: ViewContainerRef; | ||||||
@Input() isConsecutive: boolean = false; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Remove unnecessary type annotation The type of Apply this diff to remove the redundant type annotation: - @Input() isConsecutive: boolean = false;
+ @Input() isConsecutive = false; 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome
|
||||||
|
||||||
onPostingUpdated(updatedPosting: Posting) { | ||||||
this.posting = updatedPosting; | ||||||
} | ||||||
Comment on lines
+70
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Add unit tests for To ensure the correctness and maintainability of the I can assist in creating unit tests or open a GitHub issue to track this task. |
||||||
|
||||||
onReactionsUpdated(updatedReactions: Reaction[]) { | ||||||
this.posting = { ...this.posting, reactions: updatedReactions }; | ||||||
} | ||||||
Comment on lines
+74
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Add unit tests for To ensure the correctness and maintainability of the I can assist in creating unit tests or open a GitHub issue to track this task. |
||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
LGTM! Consider adding a comment for clarity.
The addition of the
isConsecutive
property aligns well with the PR objectives for enhancing message grouping. The implementation follows the coding guidelines, using camelCase for property naming.Consider adding a brief comment explaining the purpose of the
isConsecutive
property for better code documentation. For example: