-
Notifications
You must be signed in to change notification settings - Fork 291
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
: Refactor consecutive message view
#9456
base: develop
Are you sure you want to change the base?
Changes from all 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
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 | ||||
---|---|---|---|---|---|---|
|
@@ -75,19 +75,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> | ||||||
} | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,3 +74,25 @@ | |
display: none; | ||
} | ||
} | ||
|
||
.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 |
---|---|---|
|
@@ -30,6 +30,13 @@ import { canCreateNewMessageInConversation } from 'app/shared/metis/conversation | |
import { debounceTime, distinctUntilChanged } from 'rxjs/operators'; | ||
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'; | ||
|
||
interface PostGroup { | ||
author: User | undefined; | ||
posts: Post[]; | ||
} | ||
|
||
@Component({ | ||
selector: 'jhi-conversation-messages', | ||
|
@@ -72,6 +79,7 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD | |
|
||
newPost?: Post; | ||
posts: Post[] = []; | ||
groupedPosts: PostGroup[] = []; | ||
totalNumberOfPosts = 0; | ||
page = 1; | ||
public isFetchingPosts = true; | ||
|
@@ -178,11 +186,66 @@ export class ConversationMessagesComponent implements OnInit, AfterViewInit, OnD | |
}; | ||
} | ||
|
||
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).creationDateDayjs; | ||
const bDate = (b as any).creationDateDayjs; | ||
return aDate?.valueOf() - bDate?.valueOf(); | ||
}); | ||
|
||
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).creationDateDayjs; | ||
const lastDate = (lastPostInGroup as any).creationDateDayjs; | ||
|
||
let timeDiff = Number.MAX_SAFE_INTEGER; | ||
if (currentDate && lastDate) { | ||
timeDiff = currentDate.diff(lastDate, 'minute'); | ||
} | ||
|
||
if (currentPost.author?.id === currentGroup.author?.id && timeDiff < 5 && timeDiff >= 0) { | ||
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).creationDateDayjs = post.creationDate ? dayjs(post.creationDate) : undefined; | ||
return post; | ||
}); | ||
|
||
this.groupPosts(); | ||
} | ||
|
||
fetchNextPage() { | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,36 +1,80 @@ | ||||||||||||||||||||||||||||||||||||||||
<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" (contextmenu)="onRightClick($event)"> | ||||||||||||||||||||||||||||||||||||||||
<div class="message-content" [class.force-hover]="showDropdown"> | ||||||||||||||||||||||||||||||||||||||||
<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> | ||||||||||||||||||||||||||||||||||||||||
</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" | ||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||
Comment on lines
+40
to
48
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" /> | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
<!-- Right-Click Dropdown --> | ||||||||||||||||||||||||||||||||||||||||
<div *ngIf="showDropdown" [ngStyle]="{ position: 'fixed', 'top.px': dropdownPosition.y, 'left.px': dropdownPosition.x }" class="dropdown-menu show"> | ||||||||||||||||||||||||||||||||||||||||
<button class="dropdown-item d-flex" (click)="addReaction($event)"> | ||||||||||||||||||||||||||||||||||||||||
<fa-icon [icon]="faSmile" class="item-icon"></fa-icon> | ||||||||||||||||||||||||||||||||||||||||
<span jhiTranslate="artemisApp.metis.post.addReaction"></span> | ||||||||||||||||||||||||||||||||||||||||
</button> | ||||||||||||||||||||||||||||||||||||||||
<button class="dropdown-item d-flex" (click)="editPosting()"> | ||||||||||||||||||||||||||||||||||||||||
<fa-icon [icon]="faPencilAlt" class="item-icon"></fa-icon> | ||||||||||||||||||||||||||||||||||||||||
<span jhiTranslate="artemisApp.metis.post.editMessage"></span> | ||||||||||||||||||||||||||||||||||||||||
</button> | ||||||||||||||||||||||||||||||||||||||||
<button class="dropdown-item d-flex" (click)="deletePost()"> | ||||||||||||||||||||||||||||||||||||||||
<fa-icon [icon]="faTrash" class="item-icon"></fa-icon> | ||||||||||||||||||||||||||||||||||||||||
<span jhiTranslate="artemisApp.metis.post.deleteMessage"></span> | ||||||||||||||||||||||||||||||||||||||||
</button> | ||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
<div #emojiPickerTrigger="cdkOverlayOrigin" cdkOverlayOrigin [ngStyle]="{ position: 'fixed', 'top.px': clickPosition.y, 'left.px': clickPosition.x }"></div> | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
<ng-template | ||||||||||||||||||||||||||||||||||||||||
cdkConnectedOverlay | ||||||||||||||||||||||||||||||||||||||||
[cdkConnectedOverlayHasBackdrop]="true" | ||||||||||||||||||||||||||||||||||||||||
[cdkConnectedOverlayBackdropClass]="'cdk-overlay-transparent-backdrop'" | ||||||||||||||||||||||||||||||||||||||||
[cdkConnectedOverlayOrigin]="emojiPickerTrigger" | ||||||||||||||||||||||||||||||||||||||||
[cdkConnectedOverlayOpen]="showReactionSelector" | ||||||||||||||||||||||||||||||||||||||||
(backdropClick)="toggleEmojiSelect()" | ||||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||||
<jhi-emoji-picker (emojiSelect)="selectReaction($event)"></jhi-emoji-picker> | ||||||||||||||||||||||||||||||||||||||||
</ng-template> |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -16,3 +16,57 @@ | |||||||||||||||||||||||
padding-left: 0.5rem; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
.message-container { | ||||||||||||||||||||||||
position: relative; | ||||||||||||||||||||||||
border-radius: 5px; | ||||||||||||||||||||||||
transition: background-color 0.3s ease; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
Comment on lines
+20
to
+24
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; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
&.force-hover { | ||||||||||||||||||||||||
background: var(--metis-selection-option-hover-background); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
.hover-actions { | ||||||||||||||||||||||||
opacity: 1; | ||||||||||||||||||||||||
visibility: visible; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
.message-content:hover { | ||||||||||||||||||||||||
background: var(--metis-selection-option-hover-background); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
Comment on lines
+39
to
+41
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 { | ||||||||||||||||||||||||
Check warning on line 43 in src/main/webapp/app/shared/metis/answer-post/answer-post.component.scss Codacy Production / Codacy Static Code Analysissrc/main/webapp/app/shared/metis/answer-post/answer-post.component.scss#L43
|
||||||||||||||||||||||||
position: absolute; | ||||||||||||||||||||||||
top: -1.8rem; | ||||||||||||||||||||||||
right: 3%; | ||||||||||||||||||||||||
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
+59
to
+62
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; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
.item-icon { | ||||||||||||||||||||||||
width: 20px; | ||||||||||||||||||||||||
height: 20px; | ||||||||||||||||||||||||
margin-right: 0.2rem; | ||||||||||||||||||||||||
} |
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: