fix: show Today/Yesterday labels in Livechat message separator#40592
Open
Varun789-mx wants to merge 3 commits into
Open
fix: show Today/Yesterday labels in Livechat message separator#40592Varun789-mx wants to merge 3 commits into
Varun789-mx wants to merge 3 commits into
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: b62edfc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
WalkthroughAdds a getDateLabel helper to produce localized "Today"/"Yesterday" or an uppercased formatted date; MessageSeparator now uses this helper for its date prop rendering and shows ChangesMessage separator date display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Varun789-mx
force-pushed
the
fix/message-separator-today-label
branch
from
eff476c to
32be188
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Varun789-mx
changed the title
coderabbitai
Bot
added
the
type: feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/meteor/app/custom-oauth/server/custom_oauth_server.js`:
- Around line 144-150: The new fetch call that supplies allowList:
settings.get('SSRF_Allowlist') (used when posting to this.tokenPath) enforces a
safe-by-default SSRF whitelist which will block OAuth providers unless admins
configure SSRF_Allowlist; update the project migration/config documentation and
any deployment runbooks to state that OAuth integrations require explicit
configuration of SSRF_Allowlist per provider (or provide examples), and add a
clear admin-facing note where OAuth provider settings are documented
(referencing this.tokenPath and the allowList setting) so operators know to
populate SSRF_Allowlist to restore existing OAuth flows.
In `@packages/livechat/src/components/Messages/MessageSeparator/index.tsx`:
- Line 19: The code assigns messageDate = new Date(date) without validation,
which can produce an Invalid Date and break downstream comparisons/formatting;
update the MessageSeparator component to validate the incoming date string
before creating the Date object (e.g., use Date.parse(date) or construct and
then check isNaN(messageDate.getTime())), and if invalid handle
gracefully—either skip rendering the separator, fall back to a safe default
date, or log/return null so the i18next formatter never receives an Invalid
Date; apply this check around the messageDate creation and any comparisons that
use messageDate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92dd90e7-309b-4992-9980-87778c3e7840
📒 Files selected for processing (2)
apps/meteor/app/custom-oauth/server/custom_oauth_server.jspackages/livechat/src/components/Messages/MessageSeparator/index.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/custom-oauth/server/custom_oauth_server.jspackages/livechat/src/components/Messages/MessageSeparator/index.tsx
🧠 Learnings (2)
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
packages/livechat/src/components/Messages/MessageSeparator/index.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/livechat/src/components/Messages/MessageSeparator/index.tsx
🔇 Additional comments (3)
apps/meteor/app/custom-oauth/server/custom_oauth_server.js (2)
184-185: LGTM!
291-292: LGTM!packages/livechat/src/components/Messages/MessageSeparator/index.tsx (1)
57-59: LGTM!
Varun789-mx
force-pushed
the
fix/message-separator-today-label
branch
3 times, most recently
from
1c757b1 to
39a341e
Compare
coderabbitai
Bot
removed
the
type: feature
psshrijith
reviewed
May 22, 2026
| use?: any; | ||
| }; | ||
|
|
||
| const getDateLabel = (date: string, t: TFunction): string => { |
There was a problem hiding this comment.
this version looks more readable. @Varun789-mx
const isSameDay = (a: Date, b: Date) =>
a.getFullYear() === b.getFullYear() &&
a.getMonth() === b.getMonth() &&
a.getDate() === b.getDate();
const getDateLabel = (date: string, t: TFunction): string => {
const messageDate = new Date(date);
if (isNaN(messageDate.getTime())) return '';
const today = new Date();
if (isSameDay(messageDate, today)) {
return t('today');
}
const yesterday = new Date(today);
yesterday.setDate(today.getDate() - 1);
if (isSameDay(messageDate, yesterday)) {
return t('yesterday');
}
return t('message_separator_date', {
val: messageDate,
formatParams: {
val: {
month: 'short',
day: '2-digit',
year: 'numeric',
},
},
}).toUpperCase();
};
Author
There was a problem hiding this comment.
Yeah this LGTM and i place the
const isSameDay = (a: Date, b: Date) =>
a.getFullYear() === b.getFullYear() &&
a.getMonth() === b.getMonth() &&
a.getDate() === b.getDate();
outside the main which will prevent multiple function calls make it more efficient
Varun789-mx
force-pushed
the
fix/message-separator-today-label
branch
from
0e56657 to
7a37084
Compare
coderabbitai
Bot
added
the
type: feature
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/livechat/src/components/Messages/MessageSeparator/index.tsx (1)
63-64: ⚡ Quick winConsider more explicit handling of the date label fallback.
The current logic
(!!date && getDateLabel(date, t)) || (unread && t('unread_messages'))works correctly, but the fallback behavior whengetDateLabelreturns an empty string (invalid date) is implicit. Whiledateandunreadare mutually exclusive in practice, a more explicit conditional would improve clarity.♻️ Optional refactor for clarity
- {(!!date && getDateLabel(date, t)) || - (unread && t('unread_messages'))} + {date ? getDateLabel(date, t) : unread ? t('unread_messages') : null}Or if you prefer to handle the empty string case explicitly:
- {(!!date && getDateLabel(date, t)) || - (unread && t('unread_messages'))} + {date ? getDateLabel(date, t) || null : unread ? t('unread_messages') : null}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/livechat/src/components/Messages/MessageSeparator/index.tsx` around lines 63 - 64, The current inline expression in MessageSeparator ((!!date && getDateLabel(date, t)) || (unread && t('unread_messages'))) can silently treat an empty string from getDateLabel as falsy; change to an explicit check: compute a dateLabel by calling getDateLabel only when date is present (e.g., const dateLabel = date ? getDateLabel(date, t) : ''), then render dateLabel if it is non-empty else render unread ? t('unread_messages') : null so that empty string from getDateLabel is handled explicitly; update the JSX in the MessageSeparator component to use these variables (dateLabel, unread, t).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/livechat/src/components/Messages/MessageSeparator/index.tsx`:
- Line 34: MessageSeparator calls t('today') when isSameDay(messageDate, today)
but there is no "today" key in the i18n files, so add a "today" translation
entry to the English locale used by the app (e.g., in
packages/i18n/src/locales/en.i18n.json or the livechat package i18n file) with
the appropriate string (and mirror it in other locale files as needed), or
update MessageSeparator to use an existing translation key instead of
t('today'); ensure the change targets the MessageSeparator component where
t('today') is invoked so the label localizes correctly.
---
Nitpick comments:
In `@packages/livechat/src/components/Messages/MessageSeparator/index.tsx`:
- Around line 63-64: The current inline expression in MessageSeparator ((!!date
&& getDateLabel(date, t)) || (unread && t('unread_messages'))) can silently
treat an empty string from getDateLabel as falsy; change to an explicit check:
compute a dateLabel by calling getDateLabel only when date is present (e.g.,
const dateLabel = date ? getDateLabel(date, t) : ''), then render dateLabel if
it is non-empty else render unread ? t('unread_messages') : null so that empty
string from getDateLabel is handled explicitly; update the JSX in the
MessageSeparator component to use these variables (dateLabel, unread, t).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ddcede5-84bf-4d35-9354-989941436cb0
📒 Files selected for processing (1)
packages/livechat/src/components/Messages/MessageSeparator/index.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/livechat/src/components/Messages/MessageSeparator/index.tsx
🧠 Learnings (2)
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
packages/livechat/src/components/Messages/MessageSeparator/index.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/livechat/src/components/Messages/MessageSeparator/index.tsx
🔇 Additional comments (1)
packages/livechat/src/components/Messages/MessageSeparator/index.tsx (1)
18-21: LGTM!
Varun789-mx
force-pushed
the
fix/message-separator-today-label
branch
from
7a37084 to
c45be93
Compare
Varun789-mx
force-pushed
the
fix/message-separator-today-label
branch
from
9895ac5 to
f287989
Compare
coderabbitai
Bot
removed
the
type: feature
Varun789-mx
changed the title
psshrijith
approved these changes
Jun 4, 2026
Author
|
this still required QA ASSURED and milestone for this pr @psshrijith can you please add it so that it can be merged |
|
let me check @Varun789-mx , i am not sure about this |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Updated the
MessageSeparatorcomponent in the Livechat widget to display user-friendly labels instead of the full formatted date.TodayYesterdayMAY 15, 2026) is still displayedThis was achieved by adding a
getDateLabelhelper function that compares the message date against today and yesterday using year/month/day matching before falling back to the defaulti18ndate formatter .Issue(s)
Closes #40588
Steps to test or reproduce
MAY 17, 2026)Further comments
The fix is minimal and self-contained within
packages/livechat/src/components/Messages/MessageSeparator/index.tsx. TheisSameDayutility is defined inline; if the team prefers, it can be extracted to a shared helper or replaced withisToday/isYesterdayfromdate-fnsif that is already a project dependency.Summary by CodeRabbit