feat: Grouped messages based on date and added time for the messages #260
Conversation
|
@balaraju-nerati is attempting to deploy a commit to the samarthkadam's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@balaraju-nerati great work. But make a little change that box appears a liltle big decrease its font a little bit and also decrease the padding as it looks big |
|
@SamarthKadam sure! I'll update it |
|
Hey, @SamarthKadam , I modified the code regarding the changes u mentioned. |
|
@balaraju-nerati why am I not able to see timestamps in some messages |
|
Hey, @SamarthKadam sry for that bug. Didn't see that. Now it's been solved. Please check now and let me know if there are any issues. |
SamarthKadam
requested review from
SamarthKadam,
Spiral-Memory and
souravbhunia07
|
Hey @SamarthKadam
|
|
@Spiral-Memory now is it visible? |
Nope Edit: Yes it's visible now |
|
@Spiral-Memory @souravbhunia07 Are you reviewing the code? |
|
Yes, I've started the review and have suggested some changes.. Waiting for the contributor to reply ! |
|
Hey, @Spiral-Memory I don't know what changes you requested to me regarding this Pull request. Can you please explain? |
frontend/package.json
Outdated
| "emoji-mart": "^5.6.0", | ||
| "gender-detection-from-name": "^1.8.0", | ||
| "javascript-time-ago": "^2.5.10", |
There was a problem hiding this comment.
As far as I can see, you are using date-fns in the code. So, consider removing another library that you have tried.
frontend/package.json
Outdated
| @@ -28,6 +30,7 @@ | |||
| "react-router-dom": "^6.15.0", | |||
| "react-scripts": "5.0.1", | |||
| "react-speech-recognition": "^3.10.0", | |||
| "react-time-ago": "^7.3.3", | |||
| @@ -110,8 +112,33 @@ export default function ChatMessages() { | |||
| }; | |||
| }, [data]); | |||
|
|
|||
|
|
|||
| const groupedMessagesByDate = (data) => { | |||
There was a problem hiding this comment.
Is grouping messages explicitly necessary in this case? Can't we have a simple function that tests whether the previous message has a difference of 24 hours or if prev message doesn't exist ? If yes, then simply append the date label like 'Today' or 'Yesterday' or the exact date. I think that will be a more efficient approach. Adding a code sample for your reference
import { isSameDay } from 'date-fns';
const isMessageNewDay = (current, previous) =>
!previous || !isSameDay(new Date(current.ts), new Date(previous.ts));
| ></RecieverMessage> | ||
| ); | ||
| })} | ||
| {Object.keys(groupedMessages).map(date => ( |
There was a problem hiding this comment.
Once you utilize that approach, you will be then iterating with data as it was previously and checks if is this Message a message of new day, if it is, add the message divider
Sorry ! Haven't submitted the review.. check now ! |
|
@Spiral-Memory @souravbhunia07 done reviewing. As this pr was made 5 days ago. Please make this fast |
I have finished reviewing @SamarthKadam . I am waiting for the contributor to make the suggested changes. Aren't my review comments available? |
|
If my review comments are not available for any reason, i am attaching the screenshot of the same.
|
|
@balaraju-nerati Please make the required changes. |
|
Hey, @Spiral-Memory I'm working on the changes you suggested. I'll update by the end of tomorrow. Sry about the delay. |
Okay @balaraju-nerati Sure. Thanks |
|
Hey, @Spiral-Memory Please check now....I have optimized the code as per your suggestions. |
|
LGTM @balaraju-nerati, I am approving this PR, please make sure to remove the console logs after that it can be merged. Thanks! |
I have added time for the messages and Organized the messages based on date.
Screenshot of the feature
Closes #242